resolve: Preserve ambiguous glob reexports in crate metadata #147984

petrochenkov · 2025-10-22T13:12:42Z

So in cross-crate scenarios they can work in the same way as in crate-local scenarios.

Resurrection of #114682.
One of unblocking steps for #145108.
Fixes #36837.

rustbot · 2025-10-22T13:12:47Z

rustbot has assigned @jdonszelmann.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

petrochenkov · 2025-10-22T13:13:01Z

@bors try

resolve: Preserve ambiguous glob reexports in crate metadata

petrochenkov · 2025-10-22T13:14:38Z

cc @LorrensP-2158466 @bvanjoi

rust-bors · 2025-10-22T15:32:03Z

☀️ Try build successful (CI)
Build commit: b4c5508 (b4c55082edd8dec08ce8af276d7054d9c4db20c4, parent: f5e2df741b4a9820a7579f0c8eccc951706a8782)

petrochenkov · 2025-10-22T15:33:22Z

@craterbot check

craterbot · 2025-10-22T15:33:47Z

👌 Experiment pr-147984 created and queued.
🤖 Automatically detected try build b4c5508
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-10-22T15:34:03Z

🚧 Experiment pr-147984 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-10-24T07:11:57Z

🎉 Experiment pr-147984 is completed!
📊 2312 regressed and 2 fixed (721923 total)
📊 1766 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-147984/retry-regressed-list.txt ↩

petrochenkov · 2025-10-24T12:28:34Z

That's a lot of breakage.
Mostly from dependencies, including various version of openssl that we've seen previously.
I'll demote this error to always be reported as a lint in cross-crate scenarios, and then rerun crater.

petrochenkov · 2025-10-24T12:58:17Z

@bors try

@craterbot run mode=check-only p=1 crates=https://crater-reports.s3.amazonaws.com/pr-147984/retry-regressed-list.txt

resolve: Preserve ambiguous glob reexports in crate metadata

rust-bors · 2025-10-24T15:17:26Z

☀️ Try build successful (CI)
Build commit: c6359cd (c6359cd3b4418e8472bae1a89c242796f2b86d56, parent: 75948c8bb3bd37f1e8ee20273a04edea4c1f84f8)

petrochenkov · 2025-10-24T15:24:38Z

@craterbot run mode=check-only p=1 crates=https://crater-reports.s3.amazonaws.com/pr-147984/retry-regressed-list.txt

craterbot · 2025-10-24T15:24:47Z

👌 Experiment pr-147984-1 created and queued.
🤖 Automatically detected try build c6359cd
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

compiler/rustc_resolve/src/build_reduced_graph.rs

compiler/rustc_resolve/src/diagnostics.rs

tests/ui/imports/glob-conflict-cross-crate-1.rs

yaahc · 2025-12-05T23:25:45Z

tests/ui/imports/glob-conflict-cross-crate-3.rs

+    //~^ ERROR `C` is ambiguous
+    //~| ERROR `C` is ambiguous
+    //~| WARN this was previously accepted
+    //~| WARN this was previously accepted


why does this emit so many duplicate warnings? It looks like theres two unique ones, the one for the local ambiguity and the one for the external ambiguity, but the stderr file looks like its emitting each of those twice for a total of four warnings. Why isn't this being prevented by the warning deduplication logic?

rust/compiler/rustc_resolve/src/lib.rs

Lines 2072 to 2075 in 743dc68

if !self.matches_previous_ambiguity_error(&ambiguity_error) {

// avoid duplicated span information to be emit out

self.ambiguity_errors.push(ambiguity_error);

}

fn record_use_inner walks the import chain and reports an ambiguity at every step.
I think we could only report the first one (if it's an error).
I look at the PR introducing the deduplication logic, and I don't think the author really understood what they were doing, not sure why it doesn't deduplicate this specific case.
In any case, probably off-topic for this PR.

Upd: also record_use can run twice on the same "use", at least for imports (for other diagnostic reasons).

yaahc · 2025-12-05T23:32:37Z

tests/ui/imports/issue-114682-2.rs

-// First, should this ambiguous item be omitted considering the maximum visibility
-// of `issue_114682_2_extern::m::max` in the type namespace is only within the extern crate.
-// Second, if we retain the ambiguous item of the extern crate, should it be treated
-// as an ambiguous item within the local crate for the same reasoning?


Can you link to where the decision on these questions was made?

From my understanding of the PR it's looking like you went with

no

no

Is that right?

Correct, seems to be the simplest and conservative choice.

I don't quite understand the sentence, but it seems like the answer is "yes", because issue_114682_2_extern::max is indeed treated as ambiguous in this test (= local crate).

yaahc · 2025-12-05T23:38:28Z

tests/ui/imports/issue-114682-3.rs

    a.ext();
-    //^ FIXME: it should report `ext` not found because `SettingsExt`
-    // is an ambiguous item in `issue-114682-3-extern.rs`.
+    //~^ ERROR no method named `ext` found for type `u8` in the current scope


I've not yet looked into how ambiguous imports interact with which traits are in scope so the logic for this is unfamiliar to me but I find this behavior surprising. I would have expected this to report an ambiguity error while selecting the innermost¹ candidate and continuing, not for it to block the selection of all three candidates in scope with no mention of ambiguity.

This doesn't seem like an ideal diagnostic experience. Is there some structural issue that makes it hard to identify and report that this error was caused by an interaction with ambiguious imports?

Footnotes

I know this is a globvsglob/globvsexpanded issue where they're ambiguous within a single scope so I'm not sure innermost is accurate, but I don't remember off hand how the candidates for this kind of ambiguity are stored and its nearing the end of the day so I wanna get my first review pass posted rather than diving in more deeply. ↩

This is a separate issue (#147992), there are several possible ways to address it, and the latest development can be found in #149058.

I think this PR and that PR combined will result in .ext() producing a compatibility warning about the method call being ambiguous.

tests/ui/imports/issue-114682-4.rs

compiler/rustc_middle/src/metadata.rs

yaahc · 2025-12-06T00:13:55Z

We won't be catching cases where the ambiguity comes from a dependency adding an item in a subsequent version. @nikomatsakis made a suggestion to lint anytime we glob export from two different crates, which I liked. Otherwise, we can at least catch cases where there is a known conflict at the time of writing the code.

@petrochenkov am I correct in understanding that https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#ambiguous-glob-reexports represents the def-site lint but not the second suggestion from niko quoted above?

@tmandry am I correct in assuming that lang doesn't consider adding the second lint you mentioned to be a blocker for this PR?

tmandry · 2025-12-06T00:19:00Z

@yaahc Just checking you mean this one. Correct, I don't consider it a blocker.

nikomatsakis made a suggestion to lint anytime we glob export from two different crates, which I liked.

yaahc · 2025-12-06T00:19:51Z

@rustbot author

rustbot · 2025-12-06T00:19:56Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

petrochenkov · 2025-12-06T09:34:27Z

@rustbot ready

yaahc · 2025-12-08T19:26:48Z

tests/ui/imports/ambiguous-glob-vs-expanded-extern.stderr

+   = note: ambiguous because of multiple glob imports of a name in the same module
+note: `mac` could refer to the macro defined here
+  --> $DIR/auxiliary/glob-vs-expanded.rs:18:9
+   |
+LL | pub use m::*;
+   |         ^
+note: `mac` could also refer to the macro defined here
+  --> $DIR/auxiliary/glob-vs-expanded.rs:18:9
+   |
+LL | pub use m::*;
+   |         ^


This seems to be emitting the diagnostic as though it were a globvsglob rather than a globvsexpanded ambiguity.

I would expect it to say:

= note: ambiguous because of a conflict between a name from a glob import and a macro-expanded name in the same module during import or macro resolution note: `mac` could refer to the macro imported here

Removed the unnecessary glob reexport from the test for now.
The root issue here is sort of hard to fix, for that you need https://github.com/petrochenkov/rust/tree/neverwrite, but it is mostly blocked on #149195.

yaahc · 2025-12-08T19:32:53Z

go ahead and r=me once the issue with the diagnostic on the globvsexpanded test is corrected.

yaahc · 2025-12-08T19:33:04Z

@rustbot author

So in cross-crate scenarios they can work in the same way as in crate-local scenarios.

rustbot · 2025-12-09T10:52:14Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

petrochenkov · 2025-12-09T10:53:12Z

@bors r=yaahc

bors · 2025-12-09T10:53:15Z

📌 Commit 51780a5 has been approved by yaahc

It is now in the queue for this repository.

resolve: Preserve ambiguous glob reexports in crate metadata So in cross-crate scenarios they can work in the same way as in crate-local scenarios. Change Description: #147984 (comment) Resurrection of #114682. One of unblocking steps for #145108. Fixes #36837.

bors · 2025-12-09T16:22:58Z

⌛ Testing commit 51780a5 with merge c61a3a4...

rustbot assigned jdonszelmann Oct 22, 2025

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 Oct 22, 2025

rust-bors bot added a commit that referenced this pull request Oct 22, 2025

Auto merge of #147984 - petrochenkov:expambig, r=<try>

b4c5508

resolve: Preserve ambiguous glob reexports in crate metadata

This comment has been minimized.

Sign in to view

petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 22, 2025

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Oct 24, 2025

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 Oct 24, 2025

petrochenkov force-pushed the expambig branch from aa2fb6e to e388d62 Compare October 24, 2025 12:56

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 24, 2025

Auto merge of #147984 - petrochenkov:expambig, r=<try>

c6359cd

resolve: Preserve ambiguous glob reexports in crate metadata

petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2025

petrochenkov mentioned this pull request Dec 3, 2025

Resolver: Batched Import Resolution #145108

Open

yaahc reviewed Dec 6, 2025

View reviewed changes

rustbot 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 Dec 6, 2025

petrochenkov force-pushed the expambig branch from 743dc68 to 255f6c3 Compare December 6, 2025 09:33

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 6, 2025

This comment was marked as resolved.

Sign in to view

yaahc reviewed Dec 8, 2025

View reviewed changes

rustbot 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 Dec 8, 2025

resolve: Preserve ambiguous glob reexports in crate metadata

51780a5

So in cross-crate scenarios they can work in the same way as in crate-local scenarios.

petrochenkov force-pushed the expambig branch from 255f6c3 to 51780a5 Compare December 9, 2025 10:52

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 Dec 9, 2025

petrochenkov mentioned this pull request Dec 9, 2025

Support importing path-segment keyword with renaming #146972

Open

bors mentioned this pull request Dec 9, 2025

Make typo in field and name suggestions verbose #149816

Open

	if !self.matches_previous_ambiguity_error(&ambiguity_error) {
	// avoid duplicated span information to be emit out
	self.ambiguity_errors.push(ambiguity_error);
	}

resolve: Preserve ambiguous glob reexports in crate metadata #147984

Are you sure you want to change the base?

resolve: Preserve ambiguous glob reexports in crate metadata #147984

Conversation

petrochenkov commented Oct 22, 2025 • edited by yaahc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 22, 2025

Uh oh!

petrochenkov commented Oct 22, 2025

Uh oh!

This comment has been minimized.

petrochenkov commented Oct 22, 2025

Uh oh!

rust-bors bot commented Oct 22, 2025

Uh oh!

petrochenkov commented Oct 22, 2025

Uh oh!

craterbot commented Oct 22, 2025

Uh oh!

craterbot commented Oct 22, 2025

Uh oh!

craterbot commented Oct 24, 2025

Footnotes

Uh oh!

petrochenkov commented Oct 24, 2025

Uh oh!

This comment has been minimized.

petrochenkov commented Oct 24, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Oct 24, 2025

Uh oh!

petrochenkov commented Oct 24, 2025

Uh oh!

craterbot commented Oct 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yaahc Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yaahc Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

yaahc Dec 5, 2025

Choose a reason for hiding this comment

Footnotes

Uh oh!

petrochenkov Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yaahc commented Dec 6, 2025

Uh oh!

tmandry commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yaahc commented Dec 6, 2025

Uh oh!

rustbot commented Dec 6, 2025

Uh oh!

This comment has been minimized.

petrochenkov commented Dec 6, 2025

Uh oh!

This comment was marked as resolved.

yaahc Dec 8, 2025

petrochenkov commented Oct 22, 2025 •

edited by yaahc

Loading

petrochenkov Dec 6, 2025 •

edited

Loading

tmandry commented Dec 6, 2025 •

edited

Loading

petrochenkov Dec 9, 2025 •

edited

Loading