Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) #115011

compiler-errors · 2023-08-19T23:52:33Z

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: #114716 (comment)

This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.

rustbot · 2023-08-19T23:52:38Z

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

cjgillot

r=me on the implementation
I wonder if we should notify t-lang somehow.

cjgillot · 2023-08-21T17:23:01Z

compiler/rustc_resolve/src/late.rs

+                    };
+                    self.r.lint_buffer.buffer_lint_with_diagnostic(
+                        lint::builtin::ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
+                        node_id,


Why not lifetime.id here?

Elided lifetimes are not present in the AST, so buffered lints on that node ID (lifetime.id) will not get processed later, since that walks through the AST. This results in ICEs.

compiler-errors · 2023-08-21T17:33:07Z

I consider this to be fully a compiler bug, so I'll cc @rust-lang/lang here, but I don't think there's a decision for them to make.

If we wanted to, for example, stabilize a correct behavior here (e.g. lower elided to 'static, or a fresh lifetime param(?!)) then that should be done starting from correct behavior, not "moving horizontally" here, i.e. from "accidentally works" to "stablilize".

compiler-errors · 2023-08-21T18:20:03Z

@bors r=cjgillot

bors · 2023-08-21T18:20:05Z

📌 Commit fad7d22 has been approved by cjgillot

It is now in the queue for this repository.

…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.

bors · 2023-08-21T20:00:22Z

⌛ Testing commit fad7d22 with merge dcf0c926f6e689a7e93b263785ee0f95d65bd097...

bors · 2023-08-21T20:28:10Z

💔 Test failed - checks-actions

rustbot · 2023-08-21T20:43:21Z

Some changes occurred in src/tools/rust-analyzer

cc @rust-lang/rust-analyzer

compiler-errors · 2023-08-21T20:44:04Z

Sorry for the in-tree fix rust-analyzer, but can't land if RA fails CI 😅

matthiaskrgr · 2023-08-21T21:08:59Z

Does that also fix #115068 (comment) ?

bors · 2023-08-21T22:24:02Z

⌛ Testing commit c5f9d42fac0c5a9b00a2391bce4750f27656b294 with merge 8b702df6d255c1f5992e312e5765517a6ceccc52...

bors · 2023-08-21T23:48:06Z

💔 Test failed - checks-actions

lnicola

RA change seems okay.

compiler-errors · 2023-08-22T14:39:28Z

whoops forgot to reapprove

@bors r=cjgillot

bors · 2023-08-22T14:39:31Z

📌 Commit b1c609e has been approved by cjgillot

It is now in the queue for this repository.

…mpiler-errors Rollup of 6 pull requests Successful merges: - rust-lang#114959 (fix rust-lang#113702 emit a proper diagnostic message for unstable lints passed from CLI) - rust-lang#115011 (Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`)) - rust-lang#115077 (Do not emit invalid suggestion in E0191 when spans overlap) - rust-lang#115087 (Add disclaimer on size assertion macro) - rust-lang#115090 (Always use `os-release` rather than `/lib` to detect `NixOS` (bootstrap)) - rust-lang#115101 (triagebot: add dependency licensing pings) r? `@ghost` `@rustbot` modify labels: rollup

Changes were required due to: - rust-lang/rust#115011 - rust-lang/rust#114993

…rough, r=cjgillot Fall through when resolving elided assoc const lifetimes `@QuineDot` makes a good point in rust-lang#115010 (comment) that we probably should not accept *more* code due to rust-lang#115011 even though that code will eventually become a forbid-warning in a few versions (rust-lang#115010 (comment)). Fall through when walking thru the `AnonymousWarnToStatic` (renamed to `AnonymousWarn`) rib so that we can resolve as a fresh lifetime like we did before.

…rough, r=cjgillot Fall through when resolving elided assoc const lifetimes ``@QuineDot`` makes a good point in rust-lang#115010 (comment) that we probably should not accept *more* code due to rust-lang#115011 even though that code will eventually become a forbid-warning in a few versions (rust-lang#115010 (comment)). Fall through when walking thru the `AnonymousWarnToStatic` (renamed to `AnonymousWarn`) rib so that we can resolve as a fresh lifetime like we did before.

…ugh, r=cjgillot Fall through when resolving elided assoc const lifetimes `@QuineDot` makes a good point in rust-lang#115010 (comment) that we probably should not accept *more* code due to rust-lang#115011 even though that code will eventually become a forbid-warning in a few versions (rust-lang#115010 (comment)). Fall through when walking thru the `AnonymousWarnToStatic` (renamed to `AnonymousWarn`) rib so that we can resolve as a fresh lifetime like we did before.

…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.

RalfJung · 2023-09-22T06:27:48Z

compiler/rustc_lint_defs/src/builtin.rs

+    Warn,
+    "elided lifetimes cannot be used in associated constants in impls",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseError,


FWIW this FutureIncompatibilityReason will make the warning only show up in locally built crates, but not in cargo's future breakage report that also shows warnings from dependencies. Just making sure that this is a deliberate choice and not an accident. :)

Not deliberate, but isn't it conventional to bump it later? Possibly at the same time that this lint gets bumped to Deny? Not sure if there's a definite process for how to gradually raise FCW severity 🫠

I don't think we have a standard process. :/

rustbot assigned TaKO8Ki Aug 19, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 19, 2023

compiler-errors mentioned this pull request Aug 19, 2023

Explicitly deny elided lifetimes in associated consts #114716

Closed

rust-cloud-vms bot force-pushed the warn-on-elided-assoc-ct-lt branch from c99b5dd to 2456323 Compare August 19, 2023 23:59

compiler-errors mentioned this pull request Aug 20, 2023

Tracking issue for future-incompatibility lint elided_lifetimes_in_associated_constant #115010

Open

3 tasks

This comment has been minimized.

Sign in to view

Warn on elided lifetimes in associated constants

fad7d22

rust-cloud-vms bot force-pushed the warn-on-elided-assoc-ct-lt branch from 2456323 to fad7d22 Compare August 20, 2023 00:22

cjgillot self-assigned this Aug 21, 2023

cjgillot reviewed Aug 21, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2023

matthiaskrgr mentioned this pull request Aug 21, 2023

Rollup of 5 pull requests #115068

Closed

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2023

Kriskras99 mentioned this pull request Aug 21, 2023

Fix table issues in platform support documentation (closes #115047) #115055

Merged

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 21, 2023

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2023

Fix elided lifetimes in rust-lang/rust

b1c609e

rust-cloud-vms bot force-pushed the warn-on-elided-assoc-ct-lt branch from c5f9d42 to b1c609e Compare August 21, 2023 23:54

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 21, 2023

lnicola approved these changes Aug 22, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 22, 2023

compiler-errors mentioned this pull request Aug 22, 2023

Rollup of 6 pull requests #115104

Merged

bors merged commit e9897c3 into rust-lang:master Aug 22, 2023
11 checks passed

rustbot added this to the 1.74.0 milestone Aug 22, 2023

lnicola mentioned this pull request Aug 23, 2023

proc-macro-test: Pass target to cargo invocation rust-lang/rust-analyzer#15385

Merged

celinval mentioned this pull request Aug 24, 2023

Update to today's toolchain (2023-08-24) model-checking/kani#2711

Merged

4 tasks

celinval added a commit to model-checking/kani that referenced this pull request Aug 24, 2023

Update toolchain to nightly-2023-08-24 (#2711)

8ccdea4

Changes were required due to: - rust-lang/rust#115011 - rust-lang/rust#114993

tautschnig pushed a commit to tautschnig/kani that referenced this pull request Aug 30, 2023

Update toolchain to nightly-2023-08-24 (model-checking#2711)

ed26d59

Changes were required due to: - rust-lang/rust#115011 - rust-lang/rust#114993

compiler-errors mentioned this pull request Sep 1, 2023

Fall through when resolving elided assoc const lifetimes #115429

Merged

RalfJung reviewed Sep 22, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) #115011

Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) #115011

compiler-errors commented Aug 19, 2023

rustbot commented Aug 19, 2023

This comment has been minimized.

cjgillot left a comment

cjgillot Aug 21, 2023

compiler-errors Aug 21, 2023 •

edited

Loading

compiler-errors commented Aug 21, 2023

compiler-errors commented Aug 21, 2023

bors commented Aug 21, 2023

bors commented Aug 21, 2023

This comment has been minimized.

bors commented Aug 21, 2023

rustbot commented Aug 21, 2023

compiler-errors commented Aug 21, 2023

matthiaskrgr commented Aug 21, 2023

bors commented Aug 21, 2023

This comment has been minimized.

bors commented Aug 21, 2023

lnicola left a comment

compiler-errors commented Aug 22, 2023

bors commented Aug 22, 2023

RalfJung Sep 22, 2023

compiler-errors Sep 22, 2023 •

edited

Loading

RalfJung Sep 22, 2023

Warn on elided lifetimes in associated constants (ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT) #115011

Warn on elided lifetimes in associated constants (ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT) #115011

Conversation

compiler-errors commented Aug 19, 2023

rustbot commented Aug 19, 2023

This comment has been minimized.

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot Aug 21, 2023

Choose a reason for hiding this comment

compiler-errors Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Aug 21, 2023

compiler-errors commented Aug 21, 2023

bors commented Aug 21, 2023

bors commented Aug 21, 2023

This comment has been minimized.

bors commented Aug 21, 2023

rustbot commented Aug 21, 2023

compiler-errors commented Aug 21, 2023

matthiaskrgr commented Aug 21, 2023

bors commented Aug 21, 2023

This comment has been minimized.

bors commented Aug 21, 2023

lnicola left a comment

Choose a reason for hiding this comment

compiler-errors commented Aug 22, 2023

bors commented Aug 22, 2023

RalfJung Sep 22, 2023

Choose a reason for hiding this comment

compiler-errors Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

RalfJung Sep 22, 2023

Choose a reason for hiding this comment

Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) #115011

Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`) #115011

compiler-errors Aug 21, 2023 •

edited

Loading

compiler-errors Sep 22, 2023 •

edited

Loading