Explicitly deny elided lifetimes in associated consts #114716
Conversation
rustbot
added
S-waiting-on-review
|
@bors try |
|
⌛ Trying commit 7ed899bcea6314bd5cdef21721e2c3c1ab5999fb with merge 2b3cefe2517d0228c272ea52322148a40cabc88f... |
| | ^^ expected named lifetime parameter | ||
| | | ||
| help: consider introducing a named lifetime parameter | ||
| | | ||
| LL | const C<'a>: &'a str = ""; | ||
| | ++++ ~~ |
There was a problem hiding this comment.
I guess I could probably recover this suggestion by inverting the order of the lifetime ribs... Doesn't seem particularly useful, though.
There was a problem hiding this comment.
If we end up deciding on dropping these suggestion diagnostics (personally I'm fine either way, I also don't know how useful they are, mainly they're consistent with other diagnostics), then you can get rid of the revisions of this test as well as LifetimeBinderKind::ConstItem (replace it with Item) in late/diagnostics.rs and error handling code related to it since it will become unreachable.
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-author
Fix elided lifetime in associated const
Fix an unelided lifetime in an associated const.
The old code was equivalent to:
```rust
impl<'a> RegistryConfig {
/// File name of [`RegistryConfig`].
const NAME: &'a str = "config.json";
}
```
and not `&'static str`, as it might be in a regular `const` item.
This "regressed" in rust-lang/rust#97313, which started allowing this behavior (inadvertently, as far as I can tell). It's not necessarily clear to me that this is sound (or at least, it's not something we intended to be able to express), but it's also preventing me from doing crater runs to investigate fallout of this issue (rust-lang/rust#114713 and rust-lang/rust#114716).
|
Blocked on a cargo sync for now. @rustbot blocked |
rustbot
added
S-blocked
rust-cloud-vms
bot
force-pushed
the
deny-ct-elided-lt
branch
from
7ed899b
to
f575aa5
Compare
|
@bors try |
|
⌛ Trying commit f575aa5 with merge 19359abcd1be36c9ba1922a244cf9abf02af7a22... |
|
☀️ Try build successful - checks-actions |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
Well of course people have begun to rely on this faulty behavior :/ |
|
Closing in favor of a future-compat lint: #115011 |
…oc-ct-lt, r=cjgillot Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since rust-lang#97313. This is not correct behavior (see rust-lang#38831). I originally opened rust-lang#114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang#114716 (comment) This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
…oc-ct-lt, r=cjgillot Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since rust-lang#97313. This is not correct behavior (see rust-lang#38831). I originally opened rust-lang#114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang#114716 (comment) This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
… r=cjgillot Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since #97313. This is not correct behavior (see #38831). I originally opened #114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang/rust#114716 (comment) This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
… r=cjgillot Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since #97313. This is not correct behavior (see #38831). I originally opened #114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang/rust#114716 (comment) This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
… r=cjgillot Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since #97313. This is not correct behavior (see #38831). I originally opened #114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang/rust#114716 (comment) This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
This seems to have regressed in #97313, after which we have begun resolving elided lifetimes in impls to anonymous early-bound lifetimes.
This PR applies an
AnonymousReportErrorlifetime binder to both impl and trait const items to avoid this behavior. Only the former needs it, but also doing the latter for diagnostic consistency.r? @cjgillot or @petrochenkov
Please review with whitespace changes disabled, the real diff is like 4 lines.
This probably requires a crater run anyways, but this is clearly a bug. I personally think we should use an
Elided(LifetimeRes::Static)rib here, but that would require a T-lang FCP (or an RFC? idk).cc #114706