Resolve function lifetime elision on the AST by cjgillot · Pull Request #97313 · rust-lang/rust

cjgillot · 2022-05-23T12:31:18Z

~~Based on #97720~~

Lifetime elision for functions is purely syntactic in nature, so can be resolved on the AST.
This PR replicates the elision logic and diagnostics on the AST, and replaces HIR-based resolution by a delay_span_bug.

This refactor allows for more consistent diagnostics, which don't have to guess the original code from HIR.

r? @petrochenkov

rust-highfive · 2022-05-23T12:31:21Z

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

petrochenkov · 2022-05-23T18:26:53Z

Blocked on #96296.

cjgillot · 2022-06-19T15:56:48Z

@bors try @rust-timer queue

rust-timer · 2022-06-19T15:56:50Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-06-19T15:57:02Z

⌛ Trying commit 5d4053639c6a30859f6001324667a3ec32e32f33 with merge a5ac7517e73160a094c58d00ac8f79b4935456d5...

rust-timer · 2022-07-26T00:13:41Z

Finished benchmarking commit (6dbae3a): comparison url.

Instruction count

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.3%	0.3%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	3.1%	3.1%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-3.0%	-3.0%	1
Improvements 🎉 (secondary)	-2.7%	-2.7%	1
All 😿🎉 (primary)	0.1%	3.1%	2

Cycles

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.6%	2.6%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

Our nightly CI job [found](https://github.com/Enselic/cargo-public-api/actions/runs/2736941493) detected an upstream change which I think is caused by rust-lang/rust#97313. I tried to git bisect it but it didn't want to cooperate, so I have up. In either case, the change looks reasonable, so let's just adopt it.

…r=petrochenkov Clean up HIR-based lifetime resolution Based on rust-lang#97313. Fixes rust-lang#98932. r? `@petrochenkov`

…enkov Clean up HIR-based lifetime resolution Based on rust-lang/rust#97313. Fixes #98932. r? `@petrochenkov`

…henkov Resolve function lifetime elision on the AST ~Based on rust-lang#97720 Lifetime elision for functions is purely syntactic in nature, so can be resolved on the AST. This PR replicates the elision logic and diagnostics on the AST, and replaces HIR-based resolution by a `delay_span_bug`. This refactor allows for more consistent diagnostics, which don't have to guess the original code from HIR. r? `@petrochenkov`

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

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

rust-highfive assigned petrochenkov May 23, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 23, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2022

petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2022

cjgillot mentioned this pull request May 29, 2022

Compute is_late_bound_map query separately from lifetime resolution #97415

Merged

cjgillot force-pushed the ast-lifetimes-anon branch from 1fdcea8 to 9d246eb Compare May 29, 2022 18:43

This comment has been minimized.

Sign in to view

cjgillot marked this pull request as draft May 29, 2022 20:04

This comment was marked as resolved.

Sign in to view

cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 3, 2022

cjgillot force-pushed the ast-lifetimes-anon branch from 9d246eb to 8123954 Compare June 6, 2022 18:35

cjgillot force-pushed the ast-lifetimes-anon branch from 8123954 to b9c8ef3 Compare June 19, 2022 12:01

This comment has been minimized.

Sign in to view

cjgillot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2022

cjgillot marked this pull request as ready for review June 19, 2022 15:55

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 19, 2022

This comment was marked as resolved.

Sign in to view

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 19, 2022

petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 19, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 25, 2022

bors merged commit 6dbae3a into rust-lang:master Jul 25, 2022

rustbot added this to the 1.64.0 milestone Jul 25, 2022

This was referenced Jul 25, 2022

Stabilize let else #93628

Merged

Move object lifetime default computation #97839

Closed

Convert a hard-warning about named static lifetimes into a lint #98079

Closed

Do not re-parse function signatures to suggest generics #99249

Merged

Enselic mentioned this pull request Jul 26, 2022

Run scripts/bless-expected-output-for-tests.sh with nightly-2022-07-26 cargo-public-api/cargo-public-api#84

Merged

cjgillot deleted the ast-lifetimes-anon branch July 26, 2022 17:07

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jul 28, 2022

Rollup merge of #99728 - cjgillot:ast-lifetimes-anon-clean, r=petroch…

c31637e

…enkov Clean up HIR-based lifetime resolution Based on rust-lang/rust#97313. Fixes #98932. r? `@petrochenkov`

BoxyUwU mentioned this pull request Apr 28, 2023

const arguments have '_ resolved to 'static instead of infer vars #110931

Closed

matthiaskrgr mentioned this pull request Apr 28, 2023

ICE: ast lowering: no entry for node id: NodeId(53) #110899

Closed

compiler-errors mentioned this pull request Aug 10, 2023

Explicitly deny elided lifetimes in associated consts #114716

Closed

compiler-errors mentioned this pull request Aug 11, 2023

Fix elided lifetime in associated const rust-lang/cargo#12475

Merged

compiler-errors mentioned this pull request Aug 19, 2023

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

Merged

compiler-errors mentioned this pull request Aug 24, 2023

Tracking issue for future-incompatibility lint elided_lifetimes_in_associated_constant #115010

Open

4 tasks

matthiaskrgr mentioned this pull request Mar 22, 2024

ICE: no entry for node id: 'NodeId(53)' #122903

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Resolve function lifetime elision on the AST#97313

Resolve function lifetime elision on the AST#97313
bors merged 4 commits intorust-lang:masterfrom
cjgillot:ast-lifetimes-anon

cjgillot commented May 23, 2022 •

edited

Loading

Uh oh!

rust-highfive commented May 23, 2022

Uh oh!

petrochenkov commented May 23, 2022

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

This comment has been minimized.

cjgillot commented Jun 19, 2022

Uh oh!

rust-timer commented Jun 19, 2022

Uh oh!

bors commented Jun 19, 2022

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

rust-timer commented Jul 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

cjgillot commented May 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented May 23, 2022

Uh oh!

petrochenkov commented May 23, 2022

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

This comment has been minimized.

cjgillot commented Jun 19, 2022

Uh oh!

rust-timer commented Jun 19, 2022

Uh oh!

bors commented Jun 19, 2022

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

rust-timer commented Jul 26, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cjgillot commented May 23, 2022 •

edited

Loading