-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Rust: Improve type inference for for
loops and range expressions
#19971
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
base: main
Are you sure you want to change the base?
Conversation
2e9addd
to
2c8f0b8
Compare
2c8f0b8
to
a36c9ec
Compare
a36c9ec
to
7647bc3
Compare
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
A few questions, but otherwise looks great, as does the DCA run.
this.getOperatorName() = ".." and | ||
not this.hasStart() | ||
} | ||
} |
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.
There is also a RangeFullExpr
which has neither a start or end. At present it would match both RangeFromExpr
and RangeToExpr
.
override TypeParameter getATypeParameter() { | ||
result.(TypeParamTypeParameter).getTypeParam() = this.getGenericParamList().getATypeParam() | ||
or | ||
result.(AssociatedTypeTypeParameter).getTrait() = this |
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.
What does this change permit?
s instanceof RangeInclusiveStruct | ||
or | ||
n1 instanceof RangeToInclusiveExpr and | ||
s instanceof RangeToInclusiveStruct |
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.
Can we use inferRangeExprType
here?
let path = e.path(); // $ Alert[rust/summary/taint-sources] | ||
let file_name = e.file_name(); // $ Alert[rust/summary/taint-sources] | ||
sink(path); // $ hasTaintFlow | ||
sink(file_name); // $ hasTaintFlow |
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.
Really happy to see these! 🎉
This PR generalizes the existing logic for inferring types of
for
loops to be based on theIntoIterator
trait, cf. howfor
loops are desugared. Additionally, this PR also adds type inference for range expressions like1..10
.DCA looks good; more resolved calls, no significant slowdown.