-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Better Diagnostic for Trait Object Capture #54848
Conversation
// statements after the reserve location in the current block. We expect the reserve | ||
// location to be a statement assigning to a local. We follow that local in the subsequent | ||
// statements, checking for an assignment of our local (or something intermediate that | ||
// it was assigned into) that results in a trait object. |
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.
Meta-thought: All this custom logic feels a lot like a "dataflow" sort of thing. Once we move to polonius, we might be able to move all of this into datalog (but lazilly executed datalog queries) very nicely.
"borrow captured here by closure, in later iteration of loop" | ||
}, | ||
LaterUseKind::TraitCapture => | ||
"borrow later captured here by trait object, in later iteration of loop", |
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.
Pre-existing but: there... has to be a better way than having two copies of each message... :)
Why, for some messages, do we have not say "in later iteration of loop", anyway? (e.g., FakeLetRead
)
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 think the two copies of each message is kind-of necessary, we can't just have one base message and append the "in later iteration of loop" because we remove the "later" from the earlier part of the message. We could do that, but it'd be a lot of stiching together strings, so I think it's probably cleaner to not do that.
debug!("was_captured_by_trait_object: stmt={:?}", stmt); | ||
// Simple case where our target is assigned into another local, and we start | ||
// watching that local instead. | ||
if let StatementKind::Assign( |
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.
Maybe we should be looking for whether the value flows into a Cast
rvalue instead? That is, in order to assign something into a trait object, you first cast it into a trait object using an Unsize
cast.
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.
Pushed a commit that fixes this.
This comment has been minimized.
This comment has been minimized.
This commit enhances `LaterUseKind` detection to identify when a borrow is captured by a trait object which helps explain why there is a borrow error.
This commit updates the captured trait object search logic to look for unsized casts to boxed types rather than for functions that returned trait objects.
ad06f1c
to
72911fb
Compare
@bors r+ I'm not sure on the wording, but the spans seem correct. |
📌 Commit 72911fb has been approved by |
💡 This pull request was already approved, no need to approve it again.
|
📌 Commit 72911fb has been approved by |
… r=nikomatsakis Better Diagnostic for Trait Object Capture Part of rust-lang#52663. This commit enhances `LaterUseKind` detection to identify when a borrow is captured by a trait object which helps explain why there is a borrow error. r? @nikomatsakis cc @pnkfelix
… r=nikomatsakis Fixes rust-lang#47311. r? @nrc
Rollup of 9 pull requests Successful merges: - #54747 (codegen_llvm: verify that inline assembly operands are scalars) - #54848 (Better Diagnostic for Trait Object Capture) - #54850 (Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls) - #54858 (second round of refactorings for universes) - #54862 (Implement RFC 2539: cfg_attr with multiple attributes) - #54869 (Fix mobile docs) - #54870 (Stabilize tool lints) - #54893 (Fix internal compiler error on malformed match arm pattern.) - #54904 (Stabilize the `Option::replace` method) Failed merges: - #54909 ( Add chalk rules related to associated type defs) r? @ghost
…akis Better Diagnostic for Trait Object Capture Part of #52663. This commit enhances `LaterUseKind` detection to identify when a borrow is captured by a trait object which helps explain why there is a borrow error. r? @nikomatsakis cc @pnkfelix
☀️ Test successful - status-appveyor, status-travis |
Rollup of 9 pull requests Successful merges: - #54747 (codegen_llvm: verify that inline assembly operands are scalars) - #54848 (Better Diagnostic for Trait Object Capture) - #54850 (Fix #54707 - parse_trait_item_ now handles interpolated blocks as function body decls) - #54858 (second round of refactorings for universes) - #54862 (Implement RFC 2539: cfg_attr with multiple attributes) - #54869 (Fix mobile docs) - #54870 (Stabilize tool lints) - #54893 (Fix internal compiler error on malformed match arm pattern.) - #54904 (Stabilize the `Option::replace` method) Failed merges: - #54909 ( Add chalk rules related to associated type defs) r? @ghost
Part of #52663.
This commit enhances
LaterUseKind
detection to identify when a borrowis captured by a trait object which helps explain why there is a borrow
error.
r? @nikomatsakis
cc @pnkfelix