-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Disambiguate some method calls based on argument types #19749
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
Merged
paldepind
merged 3 commits into
github:main
from
paldepind:rust/impl-parameter-resolution
Jun 13, 2025
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1231,11 +1231,133 @@ private Function getTypeParameterMethod(TypeParameter tp, string name) { | |
| result = getMethodSuccessor(tp.(ImplTraitTypeTypeParameter).getImplTraitTypeRepr(), name) | ||
| } | ||
|
|
||
| bindingset[t1, t2] | ||
| private predicate typeMentionEqual(TypeMention t1, TypeMention t2) { | ||
| forex(TypePath path, Type type | t1.resolveTypeAt(path) = type | t2.resolveTypeAt(path) = type) | ||
| } | ||
|
|
||
| pragma[nomagic] | ||
| private predicate implSiblingCandidate( | ||
| Impl impl, TraitItemNode trait, Type rootType, TypeMention selfTy | ||
| ) { | ||
| trait = impl.(ImplItemNode).resolveTraitTy() and | ||
| // If `impl` has an expansion from a macro attribute, then it's been | ||
| // superseded by the output of the expansion (and usually the expansion | ||
| // contains the same `impl` block so considering both would give spurious | ||
| // siblings). | ||
| not exists(impl.getAttributeMacroExpansion()) and | ||
| // We use this for resolving methods, so exclude traits that do not have methods. | ||
| exists(Function f | f = trait.getASuccessor(_) and f.getParamList().hasSelfParam()) and | ||
| selfTy = impl.getSelfTy() and | ||
| rootType = selfTy.resolveType() | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `impl1` and `impl2` are a sibling implementations of `trait`. We | ||
| * consider implementations to be siblings if they implement the same trait for | ||
| * the same type. In that case `Self` is the same type in both implementations, | ||
| * and method calls to the implementations cannot be resolved unambiguously | ||
| * based only on the receiver type. | ||
| */ | ||
| pragma[inline] | ||
| private predicate implSiblings(TraitItemNode trait, Impl impl1, Impl impl2) { | ||
| exists(Type rootType, TypeMention selfTy1, TypeMention selfTy2 | | ||
| impl1 != impl2 and | ||
| implSiblingCandidate(impl1, trait, rootType, selfTy1) and | ||
| implSiblingCandidate(impl2, trait, rootType, selfTy2) and | ||
| // In principle the second conjunct below should be superflous, but we still | ||
| // have ill-formed type mentions for types that we don't understand. For | ||
| // those checking both directions restricts further. Note also that we check | ||
| // syntactic equality, whereas equality up to renaming would be more | ||
| // correct. | ||
| typeMentionEqual(selfTy1, selfTy2) and | ||
| typeMentionEqual(selfTy2, selfTy1) | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `impl` is an implementation of `trait` and if another implementation | ||
| * exists for the same type. | ||
| */ | ||
| pragma[nomagic] | ||
| private predicate implHasSibling(Impl impl, Trait trait) { implSiblings(trait, impl, _) } | ||
|
|
||
| /** | ||
| * Holds if a type parameter of `trait` occurs in the method with the name | ||
| * `methodName` at the `pos`th parameter at `path`. | ||
| */ | ||
| bindingset[trait] | ||
| pragma[inline_late] | ||
| private predicate traitTypeParameterOccurrence( | ||
| TraitItemNode trait, string methodName, int pos, TypePath path | ||
| ) { | ||
| exists(Function f | f = trait.getASuccessor(methodName) | | ||
| f.getParam(pos).getTypeRepr().(TypeMention).resolveTypeAt(path) = | ||
| trait.(TraitTypeAbstraction).getATypeParameter() | ||
| ) | ||
| } | ||
|
|
||
| bindingset[f, pos, path] | ||
| pragma[inline_late] | ||
| private predicate methodTypeAtPath(Function f, int pos, TypePath path, Type type) { | ||
| f.getParam(pos).getTypeRepr().(TypeMention).resolveTypeAt(path) = type | ||
| } | ||
|
|
||
| /** | ||
| * Holds if resolving the method `f` in `impl` with the name `methodName` | ||
| * requires inspecting the types of applied _arguments_ in order to determine | ||
| * whether it is the correct resolution. | ||
| */ | ||
| pragma[nomagic] | ||
| private predicate methodResolutionDependsOnArgument( | ||
| Impl impl, string methodName, Function f, int pos, TypePath path, Type type | ||
| ) { | ||
| /* | ||
| * As seen in the example below, when an implementation has a sibling for a | ||
| * trait we find occurrences of a type parameter of the trait in a method | ||
| * signature in the trait. We then find the type given in the implementation | ||
| * at the same position, which is a position that might disambiguate the | ||
| * method from its siblings. | ||
| * | ||
| * ```rust | ||
| * trait MyTrait<T> { | ||
| * fn method(&self, value: Foo<T>) -> Self; | ||
| * // ^^^^^^^^^^^^^ `pos` = 0 | ||
| * // ^ `path` = "T" | ||
| * } | ||
| * impl MyAdd<i64> for i64 { | ||
| * fn method(&self, value: Foo<i64>) -> Self { ... } | ||
| * // ^^^ `type` = i64 | ||
| * } | ||
| * ``` | ||
| * | ||
| * Note that we only check the root type symbol at the position. If the type | ||
| * at that position is a type constructor (for instance `Vec<..>`) then | ||
| * inspecting the entire type tree could be necessary to disambiguate the | ||
| * method. In that case we will still resolve several methods. | ||
| */ | ||
|
|
||
| exists(TraitItemNode trait | | ||
| implHasSibling(impl, trait) and | ||
| traitTypeParameterOccurrence(trait, methodName, pos, path) and | ||
| methodTypeAtPath(getMethodSuccessor(impl, methodName), pos, path, type) and | ||
| f = getMethodSuccessor(impl, methodName) | ||
| ) | ||
| } | ||
|
|
||
| /** Gets a method from an `impl` block that matches the method call `mc`. */ | ||
| private Function getMethodFromImpl(MethodCall mc) { | ||
| exists(Impl impl | | ||
| IsInstantiationOf<MethodCall, IsInstantiationOfInput>::isInstantiationOf(mc, impl, _) and | ||
| result = getMethodSuccessor(impl, mc.getMethodName()) | ||
| | | ||
|
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. I would suggest the following diff, just to be sure that the method being restricted by diff --git a/rust/ql/lib/codeql/rust/internal/TypeInference.qll b/rust/ql/lib/codeql/rust/internal/TypeInference.qll
index 6ea2df6550c..bc80a1a14f3 100644
--- a/rust/ql/lib/codeql/rust/internal/TypeInference.qll
+++ b/rust/ql/lib/codeql/rust/internal/TypeInference.qll
@@ -1275,12 +1275,13 @@ private predicate methodTypeAtPath(Function f, int pos, TypePath path, Type type
}
/**
- * Holds if resolving the method in `impl` with the name `methodName` requires
+ * Holds if resolving the method `f` in `impl` with the name `methodName` requires
* inspecting the types of applied _arguments_ in order to determine whether it
* is the correct resolution.
*/
+pragma[nomagic]
private predicate methodResolutionDependsOnArgument(
- Impl impl, string methodName, int pos, TypePath path, Type type
+ Impl impl, Function f, string methodName, int pos, TypePath path, Type type
) {
/*
* As seen in the example below, when an implementation has a sibling for a
@@ -1310,20 +1311,21 @@ private predicate methodResolutionDependsOnArgument(
exists(TraitItemNode trait |
implHasSibling(impl, trait) and
traitTypeParameterOccurrence(trait, methodName, pos, path) and
- methodTypeAtPath(getMethodSuccessor(impl, methodName), pos, path, type)
+ methodTypeAtPath(f, pos, path, type) and
+ f = getMethodSuccessor(impl, methodName)
)
}
/** Gets a method from an `impl` block that matches the method call `mc`. */
private Function getMethodFromImpl(MethodCall mc) {
exists(Impl impl |
- IsInstantiationOf<MethodCall, IsInstantiationOfInput>::isInstantiationOf(mc, impl, _) and
- result = getMethodSuccessor(impl, mc.getMethodName())
+ IsInstantiationOf<MethodCall, IsInstantiationOfInput>::isInstantiationOf(mc, impl, _)
|
- not methodResolutionDependsOnArgument(impl, _, _, _, _)
+ not methodResolutionDependsOnArgument(impl, _, _, _, _, _) and
+ result = getMethodSuccessor(impl, mc.getMethodName())
or
exists(int pos, TypePath path, Type type |
- methodResolutionDependsOnArgument(impl, mc.getMethodName(), pos, path, type) and
+ methodResolutionDependsOnArgument(impl, result, mc.getMethodName(), pos, path, type) and
inferType(mc.getArgument(pos), path) = type
)
)
|
||
| not methodResolutionDependsOnArgument(impl, _, _, _, _, _) and | ||
| result = getMethodSuccessor(impl, mc.getMethodName()) | ||
| or | ||
| exists(int pos, TypePath path, Type type | | ||
| methodResolutionDependsOnArgument(impl, mc.getMethodName(), result, pos, path, type) and | ||
| inferType(mc.getArgument(pos), path) = type | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Perhaps a comment why this restriction is needed?