-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Restrict type propagation into receivers #21333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -778,13 +778,6 @@ private predicate typeEquality(AstNode n1, TypePath prefix1, AstNode n2, TypePat | |||||||||
| prefix1 = TypePath::singleton(getArrayTypeParameter()) and | ||||||||||
| prefix2.isEmpty() | ||||||||||
| or | ||||||||||
| exists(Struct s | | ||||||||||
| n2 = [n1.(RangeExpr).getStart(), n1.(RangeExpr).getEnd()] and | ||||||||||
| prefix1 = TypePath::singleton(TTypeParamTypeParameter(s.getGenericParamList().getATypeParam())) and | ||||||||||
| prefix2.isEmpty() and | ||||||||||
| s = getRangeType(n1) | ||||||||||
| ) | ||||||||||
| or | ||||||||||
| exists(ClosureExpr ce, int index | | ||||||||||
| n1 = ce and | ||||||||||
| n2 = ce.getParam(index).getPat() and | ||||||||||
|
|
@@ -829,6 +822,12 @@ private predicate lubCoercion(AstNode parent, AstNode child, TypePath prefix) { | |||||||||
| bodyReturns(parent, child) and | ||||||||||
| strictcount(Expr e | bodyReturns(parent, e)) > 1 and | ||||||||||
| prefix.isEmpty() | ||||||||||
| or | ||||||||||
| exists(Struct s | | ||||||||||
| child = [parent.(RangeExpr).getStart(), parent.(RangeExpr).getEnd()] and | ||||||||||
| prefix = TypePath::singleton(TTypeParamTypeParameter(s.getGenericParamList().getATypeParam())) and | ||||||||||
| s = getRangeType(parent) | ||||||||||
| ) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -1031,10 +1030,10 @@ private module StructExprMatchingInput implements MatchingInputSig { | |||||||||
| private module StructExprMatching = Matching<StructExprMatchingInput>; | ||||||||||
|
|
||||||||||
| pragma[nomagic] | ||||||||||
| private Type inferStructExprType0(AstNode n, boolean isReturn, TypePath path) { | ||||||||||
| private Type inferStructExprType0(AstNode n, FunctionPosition pos, TypePath path) { | ||||||||||
| exists(StructExprMatchingInput::Access a, StructExprMatchingInput::AccessPosition apos | | ||||||||||
| n = a.getNodeAt(apos) and | ||||||||||
| if apos.isStructPos() then isReturn = true else isReturn = false | ||||||||||
| if apos.isStructPos() then pos.isReturn() else pos.asPosition() = 0 // the acutal position doesn't matter, as long as it is positional | ||||||||||
hvitved marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| | | ||||||||||
| result = StructExprMatching::inferAccessType(a, apos, path) | ||||||||||
| or | ||||||||||
|
|
@@ -1113,6 +1112,25 @@ private Trait getCallExprTraitQualifier(CallExpr ce) { | |||||||||
| * Provides functionality related to context-based typing of calls. | ||||||||||
| */ | ||||||||||
| private module ContextTyping { | ||||||||||
| /** | ||||||||||
| * Holds if `f` mentions type parameter `tp` at some non-return position, | ||||||||||
| * possibly via a constraint on another mentioned type parameter. | ||||||||||
| */ | ||||||||||
| pragma[nomagic] | ||||||||||
| private predicate assocFunctionMentionsTypeParameterAtNonRetPos( | ||||||||||
| ImplOrTraitItemNode i, Function f, TypeParameter tp | ||||||||||
| ) { | ||||||||||
| exists(FunctionPosition nonRetPos | | ||||||||||
| not nonRetPos.isReturn() and | ||||||||||
| tp = getAssocFunctionTypeAt(f, i, nonRetPos, _) | ||||||||||
| ) | ||||||||||
| or | ||||||||||
| exists(TypeParameter mid | | ||||||||||
| assocFunctionMentionsTypeParameterAtNonRetPos(i, f, mid) and | ||||||||||
| tp = getATypeParameterConstraint(mid, _) | ||||||||||
| ) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Holds if the return type of the function `f` inside `i` at `path` is type | ||||||||||
| * parameter `tp`, and `tp` does not appear in the type of any parameter of | ||||||||||
|
|
@@ -1129,12 +1147,7 @@ private module ContextTyping { | |||||||||
| ) { | ||||||||||
| pos.isReturn() and | ||||||||||
| tp = getAssocFunctionTypeAt(f, i, pos, path) and | ||||||||||
| not exists(FunctionPosition nonResPos | not nonResPos.isReturn() | | ||||||||||
| tp = getAssocFunctionTypeAt(f, i, nonResPos, _) | ||||||||||
| or | ||||||||||
| // `Self` types in traits implicitly mention all type parameters of the trait | ||||||||||
| getAssocFunctionTypeAt(f, i, nonResPos, _) = TSelfTypeParameter(i) | ||||||||||
| ) | ||||||||||
| not assocFunctionMentionsTypeParameterAtNonRetPos(i, f, tp) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -1184,7 +1197,7 @@ private module ContextTyping { | |||||||||
| pragma[nomagic] | ||||||||||
| private predicate hasUnknownType(AstNode n) { hasUnknownTypeAt(n, _) } | ||||||||||
|
|
||||||||||
| signature Type inferCallTypeSig(AstNode n, boolean isReturn, TypePath path); | ||||||||||
| signature Type inferCallTypeSig(AstNode n, FunctionPosition pos, TypePath path); | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Given a predicate `inferCallType` for inferring the type of a call at a given | ||||||||||
|
|
@@ -1194,19 +1207,34 @@ private module ContextTyping { | |||||||||
| */ | ||||||||||
| module CheckContextTyping<inferCallTypeSig/3 inferCallType> { | ||||||||||
| pragma[nomagic] | ||||||||||
| private Type inferCallTypeFromContextCand(AstNode n, TypePath prefix, TypePath path) { | ||||||||||
| result = inferCallType(n, false, path) and | ||||||||||
| private Type inferCallNonReturnType(AstNode n, FunctionPosition pos, TypePath path) { | ||||||||||
| result = inferCallType(n, pos, path) and | ||||||||||
| not pos.isReturn() | ||||||||||
| } | ||||||||||
|
|
||||||||||
| pragma[nomagic] | ||||||||||
| private Type inferCallNonReturnType( | ||||||||||
| AstNode n, FunctionPosition pos, TypePath prefix, TypePath path | ||||||||||
| ) { | ||||||||||
| result = inferCallNonReturnType(n, pos, path) and | ||||||||||
| hasUnknownType(n) and | ||||||||||
| prefix = path.getAPrefix() | ||||||||||
| } | ||||||||||
|
|
||||||||||
| pragma[nomagic] | ||||||||||
| Type check(AstNode n, TypePath path) { | ||||||||||
| result = inferCallType(n, true, path) | ||||||||||
| result = inferCallType(n, any(FunctionPosition pos | pos.isReturn()), path) | ||||||||||
| or | ||||||||||
| exists(TypePath prefix | | ||||||||||
| result = inferCallTypeFromContextCand(n, prefix, path) and | ||||||||||
| exists(FunctionPosition pos, TypePath prefix | | ||||||||||
| result = inferCallNonReturnType(n, pos, prefix, path) and | ||||||||||
| hasUnknownTypeAt(n, prefix) | ||||||||||
| | | ||||||||||
| pos.isPosition() | ||||||||||
| or | ||||||||||
| // Never propagate type information directly into the receiver, since its type | ||||||||||
| // must already have been known in order to resolve the call | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this always true? Can we not have cases where a non-self argument is used to find the receiver?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right, I will account for the case where a type qualifier is used. |
||||||||||
| pos.isSelf() and | ||||||||||
| not prefix.isEmpty() | ||||||||||
|
||||||||||
| not prefix.isEmpty() | |
| // Never propagate type information directly into the root of the receiver, since its type | |
| // must already have been known in order to resolve the call | |
| pos.isSelf() implies not prefix.isEmpty() |
hvitved marked this conversation as resolved.
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| mod regression1 { | ||
|
|
||
| pub struct S<T>(T); | ||
|
|
||
| pub enum E { | ||
| V { vec: Vec<E> }, | ||
| } | ||
|
|
||
| impl<T> From<S<T>> for Option<T> { | ||
| fn from(s: S<T>) -> Self { | ||
| Some(s.0) // $ fieldof=S | ||
| } | ||
| } | ||
|
|
||
| pub fn f() -> E { | ||
| let mut vec_e = Vec::new(); // $ target=new | ||
| let mut opt_e = None; | ||
|
|
||
| let e = E::V { vec: Vec::new() }; // $ target=new | ||
|
|
||
| if let Some(e) = opt_e { | ||
| vec_e.push(e); // $ target=push | ||
| } | ||
| opt_e = e.into(); // $ target=into | ||
|
|
||
| #[rustfmt::skip] | ||
| let _ = if let Some(last) = vec_e.pop() // $ target=pop | ||
| { | ||
| opt_e = last.into(); // $ target=into | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a purpose with the block here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It belongs to the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤦 Of course, the formatting threw me off. |
||
|
|
||
| opt_e.unwrap() // $ target=unwrap | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not what solves the timeout, but I saw cases where type information would incorrectly flow between limits in range expressions, so I decided to treat them as LUB conversions.