Simplify some of the logic in the `invalid_reference_casting` lint by Urgau · Pu...
source link: https://github.com/rust-lang/rust/pull/116199
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Contributor
This PR simplifies 2 areas of the logic for the invalid_reference_casting lint:
- The init detection: we now use the newly added
expr_or_initfunction instead of a manual detection - The ref-to-mut-ptr casting detection logic: I simplified this logic by caring less hardly about the order of the casting operations
Those two simplifications permits us to detect more cases, as can be seen in the test output changes.
Collaborator
|
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
Contributor
Author
|
I'm not so sure about the last commit, it fixes the ICEs in #116199 (comment) but at the cost a new scaled down function, just to avoid outside expr outside the current body. Maybe we should check that the result of |
compiler/rustc_lint/src/reference_casting.rs
Show resolved
tests/ui/lint/reference_casting.rs
Outdated Show resolved
/// dbg!(def); |
||
/// // ^^^ input |
||
/// ``` |
||
pub fn expr_or_init<'a>(&self, mut expr: &'a hir::Expr<'tcx>) -> &'a hir::Expr<'tcx> { |
Contributor
Isn't it a bit dangerous to change the other method's name and reusing it for this one?
Contributor
Author
Yes, a bit, but the function was added very recently, has only one user (the utf8 lints) and it is now better matching the similar function from clippy: clippy_utils::expr_or_init.
arg |
||
} else if had_at_least_one_cast { |
Contributor
IIUC, this flag is an optimization to avoid calling node_type if we haven't seed any cast. Is this worth it? node_type is a simple hashmap lookup.
This deserves at least a comment.
Contributor
Author
Since we are now checking the end type at the start, I agree keeping this flag doesn't seems compelling enough. Removed.
Contributor
|
@bors r+ |
added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
labels
Contributor
|
☀️ Test successful - checks-actions |
Collaborator
|
Finished benchmarking commit (1393ef1): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 631.024s -> 630.971s (-0.01%) |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK