Skip to content
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

Always print '_, even for erased lifetimes. #102068

Merged
merged 6 commits into from
Sep 24, 2022

Conversation

cjgillot
Copy link
Contributor

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021. This PR applies this discipline to rustc itself.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 20, 2022
@rust-highfive
Copy link
Collaborator

r? @eholk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2022
@eholk
Copy link
Contributor

eholk commented Sep 20, 2022

@bors r+

@eholk
Copy link
Contributor

eholk commented Sep 20, 2022

Ah, looks like a few more tests still need blessed, but you can r=me after they're passing.

@rust-log-analyzer

This comment has been minimized.

@cjgillot cjgillot force-pushed the erased-lifetime-print branch from 20da4f0 to eee64a0 Compare September 21, 2022 18:15
@cjgillot
Copy link
Contributor Author

@bors r=eholk

@bors
Copy link
Contributor

bors commented Sep 21, 2022

📌 Commit eee64a0 has been approved by eholk

It is now in the queue for this repository.

@bors 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 Sep 21, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 22, 2022
…eholk

Always print '_, even for erased lifetimes.

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.  This PR applies this discipline to rustc itself.
@Dylan-DPC
Copy link
Member

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 22, 2022
@Dylan-DPC
Copy link
Member

failed in rollup

@rustbot
Copy link
Collaborator

rustbot commented Sep 22, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@cjgillot
Copy link
Contributor Author

@bors r=eholk

@bors
Copy link
Contributor

bors commented Sep 22, 2022

📌 Commit e01a3bd has been approved by eholk

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 22, 2022
@aliemjay
Copy link
Member

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.

I wonder if this if this is true in the ValueNS as well? I guess the convention there is to not . I mean I probably see some_fn::<T>() more than some_fn::<'_, T>("").

@@ -38,10 +38,10 @@
StorageLive(_4); // scope 0 at $DIR/funky_arms.rs:+4:9: +4:19
StorageLive(_5); // scope 0 at $DIR/funky_arms.rs:+4:22: +4:37
_5 = &(*_1); // scope 0 at $DIR/funky_arms.rs:+4:22: +4:37
_4 = Formatter::sign_plus(move _5) -> bb1; // scope 0 at $DIR/funky_arms.rs:+4:22: +4:37
_4 = Formatter::<'_>::sign_plus(move _5) -> bb1; // scope 0 at $DIR/funky_arms.rs:+4:22: +4:37
Copy link
Member

@aliemjay aliemjay Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, see this one! I don't think Formatter::<'_>::sign_plus is more prefereable to Formatter::sign_plus

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is an issue. Diagnostics only use this in TypeNs position, so that's good. MIR dump is an internal debugging tool, so having non-beginner-friendly printing should be ok.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 23, 2022
…eholk

Always print '_, even for erased lifetimes.

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.  This PR applies this discipline to rustc itself.
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 23, 2022
@ChrisDenton
Copy link
Member

It just needs the src/tools/miri test to be updated:

--- tests/fail\concurrency\windows_join_detached.stderr
+++ <stderr output>
... 8 lines skipped ...
    = note: BACKTRACE:
    = note: inside `std::sys::PLATFORM::thread::Thread::join` at RUSTLIB/std/src/sys/PLATFORM/thread.rs:LL:CC
~   = note: inside `std::thread::JoinInner::<'_, ()>::join` at RUSTLIB/std/src/thread/mod.rs:LL:CC
    = note: inside `std::thread::JoinHandle::<()>::join` at RUSTLIB/std/src/thread/mod.rs:LL:CC
 note: inside `main` at $DIR/windows_join_detached.rs:LL:CC
... 10 lines skipped ...

@saethlin
Copy link
Member

I think you just need to run ./miri bless then check in the diff.

@cjgillot
Copy link
Contributor Author

@saethlin How do I run ./miri bless from x.py? Are there docs for this with the subtree?

@saethlin
Copy link
Member

That would probably be a question for @oli-obk, I've never operated the Miri tests from x.py.

@RalfJung
Copy link
Member

How do I run ./miri bless from x.py? Are there docs for this with the subtree?

./x.py test src/tools/miri --stage 0 --bless -- except currently you have to use --stage 1 due to a cargo bug (rust-lang/cargo#11138).

I don't think we have those docs yet, where should they usually go?

@cjgillot cjgillot force-pushed the erased-lifetime-print branch from e01a3bd to eb1ddd2 Compare September 23, 2022 22:10
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2022

The Miri subtree was changed

cc @rust-lang/miri

@cjgillot
Copy link
Contributor Author

@bors r=eholk rollup=iffy

@bors
Copy link
Contributor

bors commented Sep 23, 2022

📌 Commit eb1ddd2 has been approved by eholk

It is now in the queue for this repository.

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

bors commented Sep 24, 2022

⌛ Testing commit eb1ddd2 with merge e1c28e0...

@bors
Copy link
Contributor

bors commented Sep 24, 2022

☀️ Test successful - checks-actions
Approved by: eholk
Pushing e1c28e0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 24, 2022
@bors bors merged commit e1c28e0 into rust-lang:master Sep 24, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 24, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e1c28e0): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.3% [-1.4%, -1.1%] 6
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
3.4% [3.4%, 3.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.4% [3.4%, 3.4%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean1 range count2
Regressions ❌
(primary)
2.4% [2.2%, 2.6%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) 2.4% [2.2%, 2.6%] 2

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@cjgillot cjgillot deleted the erased-lifetime-print branch September 24, 2022 08:30
oli-obk pushed a commit to oli-obk/rust that referenced this pull request Sep 28, 2022
Always print '_, even for erased lifetimes.

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.  This PR applies this discipline to rustc itself.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
Always print '_, even for erased lifetimes.

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.  This PR applies this discipline to rustc itself.
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
Always print '_, even for erased lifetimes.

Explicit lifetime arguments are now the recommended syntax in rust 2018 and rust 2021.  This PR applies this discipline to rustc itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.