-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Implement UnusedValue.ql
#17726
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
Conversation
82094c5 to
e622991
Compare
|
|
||
| /** Same as `lastRefRedef`, but skips uncertain reads. */ | ||
| pragma[nomagic] | ||
| private predicate lastRefSkipUncertainReadsExt(DefinitionExt def, BasicBlock bb, int i) { |
Check warning
Code scanning / CodeQL
Missing QLDoc for parameter
e622991 to
d50d1b0
Compare
1165f83 to
548521b
Compare
| } | ||
|
|
||
| use YesOrNo::{Yes, No}; // allows `Yes`, `No` to be accessed without qualifiers. | ||
| use YesOrNo::{No, Yes}; // allows `Yes`, `No` to be accessed without qualifiers. |
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.
Why this change?
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.
Hmm, I think it's the auto-formatter.
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.
In general I believe we should not autoformat test code in the target language. Autoformatting adds a regularity that could (theoretically) mask issues in the extractor.
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.
Not that I strongly care about this particular case (reverting not necessary).
548521b to
b6a26b5
Compare
b6a26b5 to
118bbb1
Compare
geoffw0
left a comment
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.
LGTM once #17713 is merged.
|
Oh, we should have a (successful) DCA run to check this query isn't too noisy (like |
|
Superseded by #17773. |
|
Thanks for the work you did on this (all merged now). |
This PR implements an initial version of
UnusedValue.ql, based on SSA.