-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Replace locals in debuginfo records during ref_prop #147525
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: master
Are you sure you want to change the base?
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
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.
I'm not certain this is the best approach. Maintaining this invariant during optimizations will be error-prone. Should SimplifyLocals
drop dbg statements whose LHS is dead?
location: Location, | ||
) { | ||
if let StmtDebugInfo::AssignRef(local, _) = stmt_debuginfo | ||
&& let Value::Pointer(target, _) = self.targets[*local] |
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.
If this mirrors visit_var_debug_info
, we need an extra condition on target.projection
, don't we?
- StorageDead(_3); | ||
- StorageDead(_1); | ||
+ // DBG: _1 = &(*_5); | ||
+ // DBG: _5 = &(*_5); |
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.
Should we keep this statement, or drop it as a tautology?
This doesn't fix the case we saw internally in netlink-proto. Could you also give it a shot against the non-minimized reproducer in #147485? |
Had a few minutes, so I checked: with this PR applied the reproducer in https://github.com/krasimirgg/netlink-proto branch |
Fixes #147485.
r? cjgillot