-
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
Try fix incorrect "explicit lifetime name needed" #65307
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
r? @varkor maybe? |
Can you add a test with the new diagnostic? |
Sure yes, there ought to be a test correspond to this issue. |
It seems enough to solve this issue, please review 😃 |
src/librustc/hir/lowering.rs
Outdated
/// Refer to issue #65285 | ||
/// return a "error lifetime". | ||
fn new_missing_lifetime(&mut self, id: Option<NodeId>, span: Span) -> hir::Lifetime { | ||
match id { |
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.
This could be if id.is_some() { … }
.
src/librustc/hir/lowering.rs
Outdated
@@ -3284,7 +3295,7 @@ impl<'a> LoweringContext<'a> { | |||
// This is the normal case. | |||
AnonymousLifetimeMode::PassThrough => self.new_implicit_lifetime(span), | |||
|
|||
AnonymousLifetimeMode::ReportError => self.new_error_lifetime(None, span), | |||
AnonymousLifetimeMode::ReportError => self.new_missing_lifetime(None, span), |
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.
This is always called with None
, so you might as well just use new_implicit_lifetime
here, no?
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.
Well, shame of me. Thanks for pointing out XD
src/librustc/hir/lowering.rs
Outdated
@@ -3283,8 +3283,10 @@ impl<'a> LoweringContext<'a> { | |||
} | |||
// This is the normal case. | |||
AnonymousLifetimeMode::PassThrough => self.new_implicit_lifetime(span), | |||
|
|||
AnonymousLifetimeMode::ReportError => self.new_error_lifetime(None, span), | |||
// elide the lifetime parameter (if possible) for the lifetime resolver to check |
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.
It's not clear to me from this comment why this change works. Could you try to elaborate what this does in your comment?
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r=me with comment suggestion and squashing the commits. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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 remove the full stop here?
There were something wrong with CI, and I just tried to trigger the re-run by a commit, I'll revert this change |
In the future, you can trigger the CI to restart by amending the last commit and force-pushing. If you could squash your commits into a single commit, I'll approve the PR :) |
add test to for the fix add descriptive text for the fix simplified code logics update descriptive comments update to cope with the tidyness requirement merged commit suggestions Co-Authored-By: varkor <github@varkor.com> truncated redundant comments update to cope with tidy-check
Thanks a lot 🎁 , I'm somewhat new to the rust community and you've helped me greatly. |
Thanks for your work on this issue! @bors r+ rollup |
📌 Commit 53187c5 has been approved by |
Try fix incorrect "explicit lifetime name needed" This pr is trying to fixes rust-lang#65285 .
Try fix incorrect "explicit lifetime name needed" This pr is trying to fixes rust-lang#65285 .
Try fix incorrect "explicit lifetime name needed" This pr is trying to fixes rust-lang#65285 .
Rollup of 14 pull requests Successful merges: - #64603 (Reducing spurious unused lifetime warnings.) - #64623 (Remove last uses of gensyms) - #65235 (don't assume we can *always* find a return type hint in async fn) - #65242 (Fix suggestion to constrain trait for method to be found) - #65265 (Cleanup librustc mir err codes) - #65293 (Optimize `try_expand_impl_trait_type`) - #65307 (Try fix incorrect "explicit lifetime name needed") - #65308 (Add long error explanation for E0574) - #65353 (save-analysis: Don't ICE when resolving qualified type paths in struct members) - #65389 (Return `false` from `needs_drop` for all zero-sized arrays.) - #65402 (Add troubleshooting section to PGO chapter in rustc book.) - #65425 (Optimize `BitIter`) - #65438 (Organize `never_type` tests) - #65444 (Implement AsRef<[T]> for List<T>) Failed merges: - #65390 (Add long error explanation for E0576) r? @ghost
This pr is trying to fixes #65285 .