-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Reducing spurious unused lifetime warnings. #64603
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @Centril |
I'm sadly not familiar with this code. r? @matthewjasper seems like a good choice actually :) |
I was wondering if that would be fine. Great, will do that. + add more
tests.
…On Thu, Sep 19, 2019 at 7:57 PM matthewjasper ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/librustc/middle/resolve_lifetime.rs
<#64603 (comment)>:
> @@ -1673,6 +1673,16 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
_ => None,
} {
+ if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
+ if !format!("{}", snippet).contains(&format!("{}", name)) {
+ // Avoid spurious warnings: If the named lifetime
+ // is not found in the snippit, then what would a person
+ // getting this warning be able to do about it?
+ debug!("skipping unused lifetime lint as not in snippit");
This should just check whether the second character of ident (the first
after ') is "_". This is consistent with other unused lints and should
fix this issue.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#64603?email_source=notifications&email_token=AAGEJCGY2JSH3TH2E5CKV73QKPDRNA5CNFSM4IYH7VY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFKOIVY#discussion_r326333207>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGEJCD3QHQSN5MATDW5LGLQKPDRNANCNFSM4IYH7VYQ>
.
|
80e7585
to
989d6e0
Compare
Anything more to do here or are we good @matthewjasper, @Mark-Simulacrum? |
57f967c
to
c4b58fd
Compare
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.
So, looking into this some more, the issue is that we are linting for the lifetimes on the opaque type generated by the async fn
lowering, which the user isn't able to control.
The lifetimes added here should have their IDs added to self.lifetime_uses
with LifetimeUseSet::Many
.
rust/src/librustc/middle/resolve_lifetime.rs
Lines 710 to 723 in 1ab5593
GenericParamKind::Lifetime { .. } => { | |
let (name, reg) = Region::early(&self.tcx.hir(), &mut index, ¶m); | |
if let hir::ParamName::Plain(param_name) = name { | |
if param_name.name == kw::UnderscoreLifetime { | |
// Pick the elided lifetime "definition" if one exists | |
// and use it to make an elision scope. | |
elision = Some(reg); | |
} else { | |
lifetimes.insert(name, reg); | |
} | |
} else { | |
lifetimes.insert(name, reg); | |
} | |
} |
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 |
190a1ef
to
9ba6368
Compare
@rustbot modify labels to +S-waiting-on-review |
@rustbot modify labels to -S-waiting-on-author |
Ping from triage |
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.
r=me with typo fixed
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 |
5842e99
to
d82c1c5
Compare
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 |
@bors retry |
@bors retry |
@gilescope: 🔑 Insufficient privileges: not in try users |
@Centril bors is still not playing ball - getting 503s. Could you gently kick bors one more time please? |
@bors r=matthewjasper |
📌 Commit d82c1c5 has been approved by |
…r=matthewjasper Reducing spurious unused lifetime warnings. Fixes rust-lang#61115, fixes rust-lang#64493.
…r=matthewjasper Reducing spurious unused lifetime warnings. Fixes rust-lang#61115, fixes rust-lang#64493.
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
Fixes #61115, fixes #64493.