Skip to content

Lint single-use lifetimes during AST resolution - #96833

Merged
bors merged 3 commits into
rust-lang:masterfrom
cjgillot:ast-lifetimes-single
May 20, 2022
Merged

Lint single-use lifetimes during AST resolution#96833
bors merged 3 commits into
rust-lang:masterfrom
cjgillot:ast-lifetimes-single

Conversation

@cjgillot

@cjgillotcjgillot commented May 8, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR rewrites single_use_lifetime and unused_lifetime lints to be based on the AST.
We have more information at our disposal, so we can reduce the amount of false positives.

Remaining false positive: single-use lifetimes in argument-position impl-trait.
I'm waiting for #96529 to be fixed to have a clean and proper solution here.

Closes#54079
Closes#55057
Closes#55058
Closes#60554
Closes#69952

r? @petrochenkov

@cjgillotcjgillot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label May 8, 2022
@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 8, 2022
@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2022
@petrochenkovpetrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2022
@cjgillot
cjgillotforce-pushed the ast-lifetimes-single branch from c4ce5f5 to 64c8663CompareMay 10, 2022 19:30
@cjgillotcjgillot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 10, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@cjgillot
cjgillotforce-pushed the ast-lifetimes-single branch from b4cbf5e to f629d7dCompareMay 15, 2022 15:41
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot
cjgillotforce-pushed the ast-lifetimes-single branch from 4799029 to 69b9166CompareMay 15, 2022 18:16
petrochenkov
petrochenkov reviewed May 19, 2022
View reviewed changes
Comment threadcompiler/rustc_lint/src/context.rs Outdated
Comment threadsrc/test/ui/single-use-lifetime/one-use-in-struct.rs Outdated
Comment threadsrc/test/ui/async-await/unused-lifetime.rs Outdated
Comment threadcompiler/rustc_lint/src/levels.rs Outdated
Comment threadcompiler/rustc_resolve/src/late.rs Outdated
Comment threadcompiler/rustc_resolve/src/late.rs Outdated
Comment threadcompiler/rustc_parse/src/parser/ty.rs Outdated
Comment threadcompiler/rustc_parse/src/parser/ty.rs Outdated
@petrochenkovpetrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 19, 2022
@petrochenkov

petrochenkov commented May 19, 2022

Copy link
Copy Markdown
Contributor

r=me after squashing review commits into main commits.
@rustbot author

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2022
@cjgillot
cjgillotforce-pushed the ast-lifetimes-single branch from a7e8836 to 563916dCompareMay 20, 2022 11:39
@cjgillot

cjgillot commented May 20, 2022

Copy link
Copy Markdown
ContributorAuthor

@bors r=petrochenkov

@bors

bors commented May 20, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 563916d has been approved by petrochenkov

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 20, 2022
@bors

bors commented May 20, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 563916d with merge b5caa5a...

@bors

bors commented May 20, 2022

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing b5caa5a to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label May 20, 2022
@bors
bors merged commit b5caa5a into rust-lang:masterMay 20, 2022
@rustbotrustbot added this to the 1.63.0 milestone May 20, 2022
This was referenced May 20, 2022
Merged
Merged
Merged
Closed
Closed
@rust-timer

rust-timer commented May 20, 2022

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (b5caa5a): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvement found
Regressions 😿
(primary)		Regressions 😿
(secondary)		Improvements 🎉
(primary)		Improvements 🎉
(secondary)		All 😿 🎉
(primary)
count100010
mean2N/AN/AN/A-0.6%N/A
maxN/AN/AN/A-0.6%N/A

Max RSS (memory usage)

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 😿 relevant regression found
Regressions 😿
(primary)		Regressions 😿
(secondary)		Improvements 🎉
(primary)		Improvements 🎉
(secondary)		All 😿 🎉
(primary)
count111203
mean22.4%3.6%-2.4%N/A-0.8%
max2.4%3.6%-2.5%N/A-2.5%

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		Regressions 😿
(secondary)		Improvements 🎉
(primary)		Improvements 🎉
(secondary)		All 😿 🎉
(primary)
count112223
mean23.4%1.8%-2.2%-1.9%-0.3%
max3.4%2.3%-2.6%-2.7%3.4%

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

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes↩↩2↩3

  2. the arithmetic mean of the percent change↩↩2↩3

@cjgillot
cjgillot deleted the ast-lifetimes-single branch May 22, 2022 08:56
@est31est31 mentioned this pull request May 29, 2022
Open
2 tasks
@oskgooskgo mentioned this pull request Jan 25, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkovpetrochenkovpetrochenkov left review comments

Assignees

@petrochenkovpetrochenkov

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.63.0

7 participants

@cjgillot@rust-log-analyzer@bors@petrochenkov@rust-timer@rust-highfive@rustbot