rustc_ast_lowering: Stop lowering imports into multiple items #104963

petrochenkov · 2022-11-26T23:09:24Z

Lower them into a single item with multiple resolutions instead.
This also allows to remove additional NodIds and DefIds related to those additional items.

rustbot · 2022-11-26T23:09:29Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-11-26T23:09:32Z

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

petrochenkov · 2022-11-26T23:09:52Z

@bors try @rust-timer queue

bors · 2022-11-26T23:10:01Z

⌛ Trying commit b93c4dca5cb7d1e000b67d3028c1e64d8a207ecf with merge 92fa2a1be7dd7730c2f2fe2f6c0ea4c1568bf834...

petrochenkov · 2022-11-27T12:07:59Z

@bors try @rust-timer queue

bors · 2022-11-27T12:08:08Z

⌛ Trying commit 04eabb0d387ea54e53e513ba96d2d2cd1fc1aacd with merge 95e6cf06dca55add53a600d4152451a00fb84128...

bors · 2022-11-27T14:48:26Z

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

rust-timer · 2022-11-27T17:13:08Z

Finished benchmarking commit (95e6cf06dca55add53a600d4152451a00fb84128): comparison URL.

Overall result: ✅ improvements - no action needed

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 may lead to changes in compiler perf.

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

Instruction count

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	[0.8%, 0.8%]	1
Improvements ✅ (primary)	-0.6%	[-1.2%, -0.2%]	46
Improvements ✅ (secondary)	-4.3%	[-5.5%, -2.1%]	13
All ❌✅ (primary)	-0.6%	[-1.2%, -0.2%]	46

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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.4%	[3.8%, 5.2%]	3
Improvements ✅ (primary)	-1.8%	[-4.0%, -0.9%]	32
Improvements ✅ (secondary)	-8.0%	[-12.1%, -0.9%]	15
All ❌✅ (primary)	-1.8%	[-4.0%, -0.9%]	32

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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.8%	[-2.1%, -1.6%]	3
Improvements ✅ (secondary)	-9.6%	[-13.2%, -5.2%]	13
All ❌✅ (primary)	-1.8%	[-2.1%, -1.6%]	3

petrochenkov · 2022-11-27T18:57:46Z

@rustbot ready

compiler/rustc_hir/src/hir.rs

…resolutions

Lower them into a single item with multiple resolutions instead. This also allows to remove additional `NodId`s and `DefId`s related to those additional items.

petrochenkov · 2022-12-01T16:13:50Z

@rustbot ready

cjgillot · 2022-12-01T17:57:22Z

@bors r+

bors · 2022-12-01T17:57:23Z

📌 Commit b32a4ed has been approved by cjgillot

It is now in the queue for this repository.

flip1995

Small review on the Clippy changes.

flip1995 · 2022-12-01T17:57:11Z

src/tools/clippy/clippy_lints/src/wildcard_imports.rs

@@ -176,7 +176,8 @@ impl LateLintPass<'_> for WildcardImports {
                    format!("{import_source_snippet}::{imports_string}")
                };

-                let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res {
+                // Glob imports always have a single resolution.
+                let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {


This could be when already matching anyway. A little bit safer.

Suggested change

let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {

let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.get(0) {

If glob import doesn't have a resolution (for its prefix) then something unexpected is happening, so it should be fine to leae this as indexing.

flip1995 · 2022-12-01T17:58:40Z

src/tools/clippy/tests/ui/macro_use_imports.stderr

@@ -1,8 +1,8 @@
 error: `macro_use` attributes are no longer needed in the Rust 2018 edition
-  --> $DIR/macro_use_imports.rs:23:5
+  --> $DIR/macro_use_imports.rs:25:5


Those two error messages got swapped around. Weird 🤔

They are reported in a different order now, due to a different order of DefIds assigned by def collector, most probably.

bors · 2022-12-02T04:25:00Z

⌛ Testing commit b32a4ed with merge 11663b1...

bors · 2022-12-02T07:22:36Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 11663b1 to master...

rust-timer · 2022-12-02T08:55:41Z

Finished benchmarking commit (11663b1): 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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.6%	[0.6%, 0.6%]	1
Improvements ✅ (primary)	-0.5%	[-1.2%, -0.3%]	54
Improvements ✅ (secondary)	-4.2%	[-5.4%, -2.0%]	13
All ❌✅ (primary)	-0.5%	[-1.2%, -0.3%]	54

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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.5%	[-3.6%, -0.7%]	25
Improvements ✅ (secondary)	-7.9%	[-11.4%, -3.0%]	14
All ❌✅ (primary)	-1.5%	[-3.6%, -0.7%]	25

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.

	mean	range	count
Regressions ❌ (primary)	1.5%	[1.5%, 1.5%]	1
Regressions ❌ (secondary)	3.6%	[3.2%, 4.3%]	3
Improvements ✅ (primary)	-1.5%	[-2.1%, -0.9%]	3
Improvements ✅ (secondary)	-8.8%	[-11.9%, -3.6%]	13
All ❌✅ (primary)	-0.7%	[-2.1%, 1.5%]	4

rustc_ast_lowering: Stop lowering imports into multiple items Lower them into a single item with multiple resolutions instead. This also allows to remove additional `NodId`s and `DefId`s related to those additional items.

rustbot assigned wesleywiser Nov 26, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Nov 26, 2022

This comment has been minimized.

Sign in to view

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

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 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 Nov 27, 2022

This comment was marked as resolved.

Sign in to view

petrochenkov force-pushed the noaddids2 branch from b93c4dc to 04eabb0 Compare November 27, 2022 12:07

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Nov 27, 2022

This comment has been minimized.

Sign in to view

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

cjgillot self-assigned this Nov 27, 2022

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 27, 2022

petrochenkov force-pushed the noaddids2 branch from 04eabb0 to 00ce328 Compare November 27, 2022 18:57

cjgillot reviewed Nov 27, 2022

View reviewed changes

compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved

compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved

petrochenkov 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 Nov 29, 2022

This comment was marked as resolved.

Sign in to view

petrochenkov added 3 commits December 1, 2022 17:04

rustc_hir: Relax lifetime requirements on Visitor::visit_path

6cd4dd3

rustc_hir: Change representation of import paths to support multiple …

1f259ae

…resolutions

rustc_ast_lowering: Stop lowering imports into multiple items

b32a4ed

Lower them into a single item with multiple resolutions instead. This also allows to remove additional `NodId`s and `DefId`s related to those additional items.

petrochenkov force-pushed the noaddids2 branch from 00ce328 to b32a4ed Compare December 1, 2022 16:12

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 1, 2022

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 Dec 1, 2022

flip1995 reviewed Dec 1, 2022

View reviewed changes

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 2, 2022

bors merged commit 11663b1 into rust-lang:master Dec 2, 2022

rustbot added this to the 1.67.0 milestone Dec 2, 2022

xFrednet mentioned this pull request Dec 2, 2022

Bump rust nightly -> 2022-12-15 rust-marker/marker#55

Closed

bors mentioned this pull request Dec 2, 2022

Fix impl block in const expr #104889

Merged

dtolnay mentioned this pull request Dec 15, 2022

Regression in rustdoc of re-export involving namespace overlap #105735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_ast_lowering: Stop lowering imports into multiple items #104963

rustc_ast_lowering: Stop lowering imports into multiple items #104963

petrochenkov commented Nov 26, 2022

rustbot commented Nov 26, 2022

rustbot commented Nov 26, 2022

petrochenkov commented Nov 26, 2022

This comment has been minimized.

bors commented Nov 26, 2022

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

petrochenkov commented Nov 27, 2022

This comment has been minimized.

bors commented Nov 27, 2022

bors commented Nov 27, 2022

This comment has been minimized.

rust-timer commented Nov 27, 2022

petrochenkov commented Nov 27, 2022

This comment was marked as resolved.

petrochenkov commented Dec 1, 2022

cjgillot commented Dec 1, 2022

bors commented Dec 1, 2022

flip1995 left a comment

flip1995 Dec 1, 2022

petrochenkov Dec 2, 2022

flip1995 Dec 1, 2022

petrochenkov Dec 2, 2022

bors commented Dec 2, 2022

bors commented Dec 2, 2022

rust-timer commented Dec 2, 2022

	let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {
	let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.get(0) {

rustc_ast_lowering: Stop lowering imports into multiple items #104963

rustc_ast_lowering: Stop lowering imports into multiple items #104963

Conversation

petrochenkov commented Nov 26, 2022

rustbot commented Nov 26, 2022

rustbot commented Nov 26, 2022

petrochenkov commented Nov 26, 2022

This comment has been minimized.

bors commented Nov 26, 2022

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

petrochenkov commented Nov 27, 2022

This comment has been minimized.

bors commented Nov 27, 2022

bors commented Nov 27, 2022

This comment has been minimized.

rust-timer commented Nov 27, 2022

Overall result: ✅ improvements - no action needed

petrochenkov commented Nov 27, 2022

This comment was marked as resolved.

petrochenkov commented Dec 1, 2022

cjgillot commented Dec 1, 2022

bors commented Dec 1, 2022

flip1995 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Dec 2, 2022

bors commented Dec 2, 2022

rust-timer commented Dec 2, 2022

Overall result: ✅ improvements - no action needed