resolve: Preserve ambiguous glob reexports in crate metadata

[petrochenkov]

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]

r? @jdonszelmann

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]

@bors try

[petrochenkov]

cc @LorrensP-2158466 @bvanjoi

[rust-bors]

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

[petrochenkov]

@craterbot check

[craterbot]

👌 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]

🚧 Experiment pr-147984 is now running

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

[craterbot]

🎉 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

Footnotes

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

[petrochenkov]

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.

[rustbot]

This PR was rebased onto a different master 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]

@bors try

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

[rust-bors]

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

[traviscross]

We talked about this and ended up liking it. This preserves what I might call "ambiguity monotonicity" -- adding ambiguity should never reduce ambiguity warnings.

We'd also like to see a def-site warning about these cases paired with this -- in this PR or otherwise concurrently -- if possible.

@rfcbot fcp merge

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[nikomatsakis]

@rfcbot reviewed

[joshtriplett]

FWIW I'd personally like to see that proposed additional warning be in a separate PR, not mixed into this one. I do want to see that warning too, though.

[tmandry]

I think a def-site warning is important to "shift left" failures like this.

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.

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[petrochenkov]

We'd also like to see a def-site warning about these cases paired with this

I think a def-site warning is important to "shift left" failures like this.

We already have a lint for this - https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#ambiguous-glob-reexports

[petrochenkov]

@traviscross

74 regressions are just due to ambiguous_glob_imports being deny-by-default.
We could potentially introduce a separate warn-by-default lint extern_ambiguous_glob_imports to soften the transition.

Is there some decision on this ^^^ ?

[traviscross]

The FCP, as proposed, is to accept that soft breakage.

(I.e., "soft breakage" due to this being from a deny-by-default lint, that could be allowed and is affected by cap-lints, rather than due to a hard error.)

[traviscross]

We already have a lint for this - https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#ambiguous-glob-reexports

Thanks. Yes, that is the lint we were looking for.

[bors]

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

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.