fix(resolve): update the ambiguity glob binding as warning recursively by bvanjoi · Pull Request #113099 · rust-lang/rust

bvanjoi · 2023-06-27T14:03:29Z

Fixes #47525
Fixes #56593, but issue-56593-2.rs is not fixed to ensure backward compatibility.
Fixes #98467
Fixes #105235
Fixes #112713

This PR had added a field called warn_ambiguous in NameBinding which is only for back compatibly reason and used for lint.

More details: #112743

r? @petrochenkov

rustbot · 2023-06-27T14:03:38Z

r? @TaKO8Ki

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

tests/ui/resolve/issue-56593-2.rs

tests/ui/resolve/issue-112743.stderr

petrochenkov · 2023-06-29T08:09:43Z

Meta: not sure why new PR, we could continue in #112743, but it's fine too.

compiler/rustc_resolve/src/lib.rs

tests/ui/resolve/issue-56593-2.rs

bvanjoi · 2023-06-29T08:35:51Z

not sure why new PR

Ahh, just because I feel there are some differences in the way they are implemented, perhaps the new PR can provide a clearer message. Of course, this matter is not important, and we don't need to dwell on it.

compiler/rustc_lint_defs/src/builtin.rs

compiler/rustc_resolve/src/diagnostics.rs

compiler/rustc_lint_defs/src/lib.rs

compiler/rustc_lint/src/context.rs

compiler/rustc_resolve/src/diagnostics.rs

compiler/rustc_resolve/src/imports.rs

petrochenkov · 2023-06-29T11:47:36Z

Could you also move all the new tests from ui/resolve to ui/imports?

compiler/rustc_resolve/src/imports.rs

compiler/rustc_resolve/src/diagnostics.rs

compiler/rustc_resolve/src/imports.rs

petrochenkov · 2023-07-26T14:44:19Z

r=me after cleaning up git history.

petrochenkov · 2023-07-26T14:44:48Z

@bors delegate+

bors · 2023-07-26T14:44:51Z

✌️ @bvanjoi, you can now approve this pull request!

If @petrochenkov told you to "r=me" after making some further change, please make that change, then do @bors r=@petrochenkov

bvanjoi · 2023-07-26T15:35:40Z

CI is green

@bors r=@petrochenkov

bors · 2023-07-26T15:35:41Z

📌 Commit 379891902a412b13b73d1151986564c55b987e70 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2023-07-26T17:23:54Z

⌛ Testing commit 379891902a412b13b73d1151986564c55b987e70 with merge 1c3ef3cb3606e31bf36d695e5d30ccd686a25046...

petrochenkov · 2023-07-26T17:42:04Z

It's better to report this to stdarch, but in meantime ambiguous_glob_reexports can be allowed on mod core_arch.
@rustbot author

bvanjoi · 2023-07-28T15:55:40Z

I have added #[allow(ambiguous_glob_reexports)] f mod core_arch.

@rustbot ready

petrochenkov · 2023-07-28T19:10:50Z

@bors r+

bors · 2023-07-28T19:10:52Z

📌 Commit 771c832 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2023-07-29T06:04:44Z

⌛ Testing commit 771c832 with merge 2dc6610...

bors · 2023-07-29T07:48:37Z

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

bvanjoi · 2023-07-29T08:05:25Z

As the next step, I will attempt to address the issue at #113834.

rust-timer · 2023-07-29T18:04:36Z

Finished benchmarking commit (2dc6610): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)	1.5%	[1.2%, 2.6%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
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.

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.1%, 2.3%]	6
Regressions ❌ (secondary)	7.8%	[6.2%, 9.7%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-4.1%, -2.4%]	3
All ❌✅ (primary)	1.7%	[1.1%, 2.3%]	6

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)	2.6%	[2.1%, 3.5%]	8
Improvements ✅ (primary)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.3%, -2.3%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 650.158s -> 652.195s (0.31%)

nnethercote · 2023-07-30T00:38:30Z

Minor regressions for unused-warnings, but that is an extreme stress tests that exists mostly to detect catastrophic regressions in error handling, so it's no big deal.

@rustbot label: +perf-regression-triaged

RalfJung · 2023-09-22T06:27:23Z

compiler/rustc_lint_defs/src/builtin.rs

+    Warn,
+    "detects certain glob imports that require reporting an ambiguity error",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseError,


FWIW this FutureIncompatibilityReason will make the warning only show up in locally built crates, but not in cargo's future breakage report that also shows warnings from dependencies. Just making sure that this is a deliberate choice and not an accident. :)

I just assumed all future incompatibility lints are reported for dependencies during review.

No, that's not the case. #116049 should help clarify (but doesn't change the behavior of any existing lint).

petrochenkov · 2025-11-21T17:16:14Z

compiler/rustc_resolve/src/imports.rs

+                            // However we currently using `Some(old_binding)` for back compact
+                            // purposes.
+                            // This case can be removed after once `Undetermined` is prepared
+                            // for glob-imports.


Sigh, this case was supposed to produce ambiguous_glob_imports lint for years, but it never did, so we cannot remove it now.
I've noticed it just now...

rustbot assigned TaKO8Ki Jun 27, 2023

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. labels Jun 27, 2023

bvanjoi commented Jun 27, 2023

View reviewed changes

tests/ui/resolve/issue-56593-2.rs Outdated Show resolved Hide resolved

bvanjoi commented Jun 27, 2023

View reviewed changes

tests/ui/resolve/issue-112743.stderr Outdated Show resolved Hide resolved

rustbot assigned petrochenkov and unassigned TaKO8Ki Jun 27, 2023

This comment has been minimized.

Sign in to view

bvanjoi force-pushed the fix-112713-2 branch from 05b39a0 to d8ed2c8 Compare June 27, 2023 14:51

This comment has been minimized.

Sign in to view

bvanjoi force-pushed the fix-112713-2 branch from d8ed2c8 to c9b38ed Compare June 27, 2023 17:00

petrochenkov mentioned this pull request Jun 29, 2023

fix(resolve): update the ambiguity glob binding recursively #112743

Closed

petrochenkov reviewed Jun 29, 2023

View reviewed changes

compiler/rustc_resolve/src/lib.rs Outdated Show resolved Hide resolved

tests/ui/resolve/issue-56593-2.rs Outdated Show resolved Hide resolved

tests/ui/resolve/issue-56593-2.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Jun 29, 2023

View reviewed changes

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 Jun 29, 2023

This comment was marked as resolved.

Sign in to view

bvanjoi force-pushed the fix-112713-2 branch from c9b38ed to 6ff5bab Compare July 1, 2023 03:27

This comment has been minimized.

Sign in to view

petrochenkov 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 Jul 1, 2023

petrochenkov reviewed Jul 1, 2023

View reviewed changes

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

compiler/rustc_resolve/src/diagnostics.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 Jul 1, 2023

bvanjoi force-pushed the fix-112713-2 branch from 6ff5bab to 67a3e93 Compare July 1, 2023 16:11

petrochenkov mentioned this pull request Jul 1, 2023

resolve: Always update globs importing from a module when bindings in that module change #113242

Closed

This comment has been minimized.

Sign in to view

bvanjoi mentioned this pull request Jul 26, 2023

Tracking Issue for ambiguous_glob_imports lint #114095

Open

2 tasks

petrochenkov reviewed Jul 26, 2023

View reviewed changes

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

compiler/rustc_resolve/src/imports.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

bvanjoi mentioned this pull request Jul 27, 2023

ambiguous glob re-exports caused by vcvtq_s32_f32 rust-lang/stdarch#1446

Closed

This comment was marked as resolved.

Sign in to view

bvanjoi added 2 commits July 29, 2023 00:19

fix(resolve): update the ambiguity glob binding as warning recursively

cac0bd0

library: allow ambiguous_glob_reexports for core_arch

771c832

petrochenkov mentioned this pull request Aug 30, 2023

Adding #[derive()] changes blob import semantics #115377

Closed

tmiasko mentioned this pull request Aug 30, 2023

no MIR available ICE instead of an ambiguity error #115380

Closed

bvanjoi mentioned this pull request Sep 12, 2023

regression: import resolution ambiguity #115777

Open

RalfJung reviewed Sep 22, 2023

View reviewed changes

petrochenkov reviewed Nov 21, 2025

View reviewed changes

petrochenkov mentioned this pull request Nov 21, 2025

resolve: Partially convert ambiguous_glob_imports lint into a hard error #149195

Open

Uh oh!

Conversation

bvanjoi commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 27, 2023

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Jun 29, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bvanjoi commented Jun 29, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Jun 29, 2023

Uh oh!

This comment was marked as resolved.

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Jul 26, 2023

Uh oh!

petrochenkov commented Jul 26, 2023

Uh oh!

bors commented Jul 26, 2023

Uh oh!

bvanjoi commented Jul 26, 2023

Uh oh!

bors commented Jul 26, 2023

Uh oh!

bors commented Jul 26, 2023

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

petrochenkov commented Jul 26, 2023

Uh oh!

This comment was marked as resolved.

bvanjoi commented Jul 28, 2023

Uh oh!

petrochenkov commented Jul 28, 2023

Uh oh!

bors commented Jul 28, 2023

Uh oh!

bors commented Jul 29, 2023

Uh oh!

bors commented Jul 29, 2023

Uh oh!

bvanjoi commented Jul 29, 2023

Uh oh!

rust-timer commented Jul 29, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

nnethercote commented Jul 30, 2023

Uh oh!

RalfJung Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

petrochenkov Sep 22, 2023

Choose a reason for hiding this comment

bvanjoi commented Jun 27, 2023 •

edited

Loading

petrochenkov Nov 21, 2025 •

edited

Loading