Rust: Handle self parameters in variables and SSA library
#18088
Conversation
This file contains 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
paldepind
force-pushed
the
rust-self-parameters
branch
from
f4ab6a3
to
d06b583
Compare
paldepind
changed the title
self parameters in variables and SSA library
hvitved
requested changes
Nov 25, 2024
|
PR now updated with the actual commit that it was supposed to contain 😅 |
hvitved
requested changes
Nov 26, 2024
| // exclude parameters from function pointer types (e.g. `x` in `fn(x: i32) -> i32`) | ||
| not exists(FnPtrType fp | fp.getParamList().getParam(_).getPat() = p) | ||
| private predicate variableDecl(AstNode definingNode, AstNode p, string name) { | ||
| exists(SelfParam sp | sp = p | |
There was a problem hiding this comment.
For patterns like this, I prefer p = any(SelfParam sp |
There was a problem hiding this comment.
That is definitely nicer to read!
| not exists(Function f | not f.hasBody() and f.getParamList().getSelfParam() = sp) | ||
| ) | ||
| or | ||
| exists(IdentPat pat | pat = p | |
| /** | ||
| * Gets the pattern that declares this variable. | ||
| * Gets the pattern that declares this variable, if one exists. |
There was a problem hiding this comment.
Nit: The standard phrasing tends to be "if any".
| @@ -135,7 +147,9 @@ module Impl { | |||
| predicate isCaptured() { this.getAnAccess().isCapture() } | |||
|
|
|||
| /** Gets the parameter that introduces this variable, if any. */ | |||
| Param getParameter() { parameterDeclInScope(result, this, _) } | |||
| ParamBase getParameter() { | |||
| result = this.getSelfParam() or result = getAVariablePatAncestor(this).getParentNode() | |||
There was a problem hiding this comment.
Relying on getParentNode() is generally not good, as it is basically an untyped interface. So I would prefer if we could revert it back to using parameterDeclInScope.
There was a problem hiding this comment.
I've now removed getParentNode on the rhs. and added getPat on the lhs.
hvitved
approved these changes
Nov 26, 2024
|
DCA looks fine. Thanks for starting the run. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes in this PR:
selfparameters as variables.