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.

Sign up for GitHub

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

Jump to bottom

Make def names and HIR names consistent. #95435

Merged
merged 3 commits into from
Apr 10, 2022
Merged

Make def names and HIR names consistent. #95435

merged 3 commits into from
Apr 10, 2022

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Mar 29, 2022 •
edited
Loading

The name in the DefKey is interned to create the DefId, so it does not
require any query to access. This can be leveraged to avoid a few useless
HIR accesses for names.

In order to achieve that, generic parameters created from universal
impl-trait are given the pretty-printed ast as a name, instead of
{{opaque}}.

Drive-by: the TyCtxt::opt_item_name used a dummy span for non-local
definitions. We have access to def_ident_span, so we use it.

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

r? @estebank

(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 Mar 29, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 29, 2022
@bors
Copy link
Contributor

bors commented Mar 29, 2022

⌛ Trying commit 79010104f47e9a435ef49a7f4b40f0f84a56dda5 with merge 570bb0da8027dcda69858fc2b200800fd580563c...

@bors
Copy link
Contributor

bors commented Mar 29, 2022

☀️ Try build successful - checks-actions
Build commit: 570bb0da8027dcda69858fc2b200800fd580563c (570bb0da8027dcda69858fc2b200800fd580563c)

@rust-timer
Copy link
Collaborator

Queued 570bb0da8027dcda69858fc2b200800fd580563c with parent 11909e3, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (570bb0da8027dcda69858fc2b200800fd580563c): comparison url.

Summary: This benchmark run did not return any relevant results. 4 results were found to be statistically significant but too small to be relevant.

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 29, 2022
@cjgillot cjgillot mentioned this pull request Mar 30, 2022
Merged
@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 30, 2022
@bors
Copy link
Contributor

bors commented Mar 30, 2022

⌛ Trying commit e71dc40236621ccbdb1ba6cea4aab5a6ccb3d2c8 with merge b3ca67599d9fad163470e4584416fc676a8fb6a9...

@bors
Copy link
Contributor

bors commented Mar 30, 2022

☀️ Try build successful - checks-actions
Build commit: b3ca67599d9fad163470e4584416fc676a8fb6a9 (b3ca67599d9fad163470e4584416fc676a8fb6a9)

@rust-timer
Copy link
Collaborator

Queued b3ca67599d9fad163470e4584416fc676a8fb6a9 with parent 3e75146, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b3ca67599d9fad163470e4584416fc676a8fb6a9): comparison url.

Summary: This benchmark run did not return any relevant results. 3 results were found to be statistically significant but too small to be relevant.

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

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 30, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Apr 3, 2022

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned estebank Apr 3, 2022
oli-obk
oli-obk requested changes Apr 3, 2022
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

I'll look into compiletest changes next week. I wanted to add a way to add defaults for the comment-configs of ui tests anyway, and we could use that to insert a filter for all crates.io crate paths

compiler/rustc_resolve/src/def_collector.rs Outdated Show resolved Hide resolved
src/test/ui/associated-type-bounds/assoc-type-eq-with-dyn-atb-fail.stderr Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Apr 5, 2022

☔ The latest upstream changes (presumably #95667) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 9, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 9, 2022

📌 Commit 672ce15 has been approved by oli-obk

@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 Apr 9, 2022
@bors
Copy link
Contributor

bors commented Apr 9, 2022

⌛ Testing commit 672ce15 with merge 559c019...

@bors
Copy link
Contributor

bors commented Apr 10, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 559c019 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 10, 2022
@bors bors merged commit 559c019 into rust-lang:master Apr 10, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 10, 2022
@rust-timer
Copy link
Collaborator

This benchmark run did not return any relevant results.

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

@klensy klensy mentioned this pull request Apr 10, 2022
Closed
@cjgillot cjgillot deleted the one-name branch April 10, 2022 11:07
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2022
@bors
Auto merge of rust-lang#95487 - cjgillot:menhir, r=oli-obk
027a232 
Avoid accessing HIR from MIR passes

`hir_owner_nodes` contains a lot of information, and the query result is typically dirty. This forces dependent queries to be re-executed needlessly.

This PR refactors some accesses to HIR to go through more targeted queries that yield the same result.

Based on rust-lang#95435 and rust-lang#95436
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

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
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.
8 participants
@cjgillot @rust-highfive @rust-log-analyzer @rust-timer @bors @oli-obk @estebank @rustbot