don't elide lifetimes in paths in librustc/ #53816
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
src/librustc/dep_graph/dep_node.rs
Outdated
| @@ -331,7 +331,7 @@ macro_rules! define_dep_nodes { | |||
| /// refers to something from the previous compilation session that | |||
| /// has been removed. | |||
| #[inline] | |||
| pub fn extract_def_id(&self, tcx: TyCtxt) -> Option<DefId> { | |||
| pub fn extract_def_id(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Option<DefId> { | |||
There was a problem hiding this comment.
I think I would prefer this as '_ since they're not used.
There was a problem hiding this comment.
I too would prefer to use '_ — I like to know when the names are important. I also just don't like our convention of using name 'a, though I guess that's neither here nor there =)
There was a problem hiding this comment.
Are we using in-band lifetimes here, or are these names in scope?
nikomatsakis
approved these changes
Aug 30, 2018
|
@bors r+ I think I'm going to r+ this as is because "bitrot" |
|
📌 Commit c9a67858ecb9c686bfbbd7e2929e3e86e62bd571 has been approved by |
bors
added
S-waiting-on-bors
src/librustc/mir/cache.rs
Outdated
| @@ -55,7 +55,8 @@ impl Cache { | |||
| *self.predecessors.borrow_mut() = None; | |||
| } | |||
|
|
|||
| pub fn predecessors(&self, mir: &Mir) -> ReadGuard<IndexVec<BasicBlock, Vec<BasicBlock>>> { | |||
| pub fn predecessors(&self, mir: &Mir<'_>) | |||
| -> ReadGuard<'_, IndexVec<BasicBlock, Vec<BasicBlock>>> { | |||
There was a problem hiding this comment.
This exact function confuses me every time — I always think "how come there is no lifetime here?!"
|
⌛ Testing commit c9a67858ecb9c686bfbbd7e2929e3e86e62bd571 with merge 0c4c20043bfe7c2069c8cd5f4969449feb9cfc66... |
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Missed one |
kennytm
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #53884) made this pull request unmergeable. Please resolve the merge conflicts. |
Closed
|
Ping from triage, @zackmdavis! Any update on fixing the missing lifetime? |
|
@BatmanAoD yes, thanks, I have a note on my to-do list to rebase this tonight. |
|
or at least "soon" if not literally tonight (I hope I may be forgiven for the phenomenon where a task that only takes 10 minutes of wall time costs disproportionately more in subjective "activiation energy") |
zackmdavis
force-pushed
the
elided_lifetimes_in_paths_field_day
branch
from
c9a6785
to
54cc45c
Compare
zackmdavis
removed
the
S-waiting-on-author
|
☔ The latest upstream changes (presumably #54032) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
the
S-waiting-on-author
zackmdavis
force-pushed
the
elided_lifetimes_in_paths_field_day
branch
from
54cc45c
to
91a3cb1
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☔ The latest upstream changes (presumably #54151) made this pull request unmergeable. Please resolve the merge conflicts. |
|
📌 Commit aab59892b4fa4357230610dd8c6525bf29b16b1a has been approved by |
bors
added
S-waiting-on-bors
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
bors
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #54601) made this pull request unmergeable. Please resolve the merge conflicts. |
This seemed like a good way to kick the tires on the elided-lifetimes-in-paths lint (rust-lang#52069)—seems to work! This was also pretty tedious—it sure would be nice if `cargo fix` worked on this codebase (rust-lang#53896)!
zackmdavis
force-pushed
the
elided_lifetimes_in_paths_field_day
branch
from
aab5989
to
5b22d9b
Compare
|
@bors r=nikomatsakis |
|
📌 Commit 5b22d9b has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 5b22d9b with merge 2bafac082a2573db4df4917533e419e54d7e63b9... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Sep 30, 2018
…, r=nikomatsakis don't elide lifetimes in paths in librustc/ In light of the "Apply to rustc" checkbox on #44524 and @nikomatsakis's [recent comment about regularly wanting visual indication of elided lifetimes in types](#44524 (comment)), I was curious to see what it would look like if we turned the `elided_lifetimes_in_path` lint on in at least one crate in the codebase (I chose librustc). Given that I couldn't figure out how to get `cargo fix` work with the build system, this arguably wasn't a very efficient use of my time, but once I started, the conjunction of moral law and the sunk cost fallacy forced me to continue. This is mostly applying the `<'_>` suggestions issued by the lint, but there were a few places where I named the lifetimes (_e.g._, `<'a, 'gcx, 'tcx>` on `TyCtxt`) in order to match style with surrounding code. r? @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
This was referenced Sep 30, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In light of the "Apply to rustc" checkbox on #44524 and @nikomatsakis's recent comment about regularly wanting visual indication of elided lifetimes in types, I was curious to see what it would look like if we turned the
elided_lifetimes_in_pathlint on in at least one crate in the codebase (I chose librustc). Given that I couldn't figure out how to getcargo fixwork with the build system, this arguably wasn't a very efficient use of my time, but once I started, the conjunction of moral law and the sunk cost fallacy forced me to continue.This is mostly applying the
<'_>suggestions issued by the lint, but there were a few places where I named the lifetimes (e.g.,<'a, 'gcx, 'tcx>onTyCtxt) in order to match style with surrounding code.r? @nikomatsakis