Skip to content

Conversation

@petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Nov 21, 2025

I'm tired of this logic interfering with any attempts to fix or refactor glob imports.

The lint was implemented 2.3 years ago, and made deny-by-default and report-in-dependencies 4 months ago.

The removal is a bit over-eager because of one piece that wasn't implemented correctly (#113099 (comment)), but I want to look at the crater results.

Part of #114095.

@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot 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 Nov 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2025

r? @nnethercote

rustbot has assigned @nnethercote.
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
Copy link
Contributor Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 21, 2025
resolve: Convert `ambiguous_glob_imports` lint into a hard error
@petrochenkov 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 Nov 21, 2025
@rust-bors
Copy link

rust-bors bot commented Nov 21, 2025

☀️ Try build successful (CI)
Build commit: 446cb60 (446cb600aa4837dd6c513f14fa0d25a909b177d7, parent: e22dab387f6b4f6a87dfc54ac2f6013dddb41e68)

@petrochenkov
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-149195 created and queued.
🤖 Automatically detected try build 446cb60
🔍 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

@bors

This comment was marked as resolved.

@craterbot
Copy link
Collaborator

🚧 Experiment pr-149195 is now running

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

@craterbot
Copy link
Collaborator

🎉 Experiment pr-149195 is completed!
📊 2676 regressed and 6 fixed (741888 total)
📊 2000 spurious results on the retry-regessed-list.txt, consider a retry1 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-149195/retry-regressed-list.txt

@craterbot 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 Dec 1, 2025
@nnethercote
Copy link
Contributor

@petrochenkov: seems like a lot of regressions :( Should I review this?

@petrochenkov
Copy link
Contributor Author

@petrochenkov: seems like a lot of regressions :( Should I review this?

Not yet.
@rustbot author

@rustbot 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 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2025

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

@petrochenkov
Copy link
Contributor Author

The largest part of the regressions are from openssl-0.10.*, it's a known issue that is addressed by #147984.
So this is blocked by #147984 now.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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, 2025
@yaahc
Copy link
Member

yaahc commented Dec 9, 2025

Correct me if I'm wrong on this one

@rustbot author

@rustbot 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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 9, 2025
@craterbot craterbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 13, 2025
@petrochenkov 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 Dec 14, 2025
@rustbot rustbot added the T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. label Dec 15, 2025
@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

These commits modify tests/rustdoc-json.
rustdoc-json is a public (but unstable) interface.

Please ensure that if you've changed the output:

  • It's intentional.
  • The FORMAT_VERSION in src/librustdoc-json-types is bumped if necessary.

cc @aDotInTheVoid, @obi1kenobi

@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Dec 15, 2025
@rustbot

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2025

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
Copy link
Contributor Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Dec 20, 2025
resolve: Partially convert `ambiguous_glob_imports` lint into a hard error
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

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

I have no opinion one way or the other on this PR, just noting that I found the change to rustdoc JSON surprising and felt it might be a correctness risk to downstream tools.

cc @aDotInTheVoid

View changes since this review


//@ set m1 = "$.index[?(@.name == 'm1' && @.inner.module)].id"
//@ is "$.index[?(@.name == 'm1')].inner.module.items" []
//@ is "$.index[?(@.name == 'm1')].inner.module.items" [0]
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a surprising change and possibly like a regression to me.

A few things I don't quite understand:

  • why m1 claims to be stripped but still returns the item
  • why m2 has different behavior, not returning its item despite looking identical to m1

Based on the current rustdoc here (and ignoring that we run with private and hidden items documented), cargo-semver-checks and similar tools would determine that m1::f is pub and importable without ambiguity, since m2::f is not shown and therefore there's nothing to indicate ambiguity.

Copy link
Contributor Author

@petrochenkov petrochenkov Dec 20, 2025

Choose a reason for hiding this comment

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

This is something that needed to go together with #147984 actually, independently from this PR.
Ambiguous glob imports can now be used from other crates (even if under a lint), so effective visibilities tables need to mark them as reachable (otherwise you get various kinds of issues, e.g. MIR for those items not being encoded).
Rustdoc uses the same effective visibility tables too, so the new items appear in rustdoc output as well.

@petrochenkov
Copy link
Contributor Author

@bors try

rust-bors bot added a commit that referenced this pull request Dec 20, 2025
resolve: Partially convert `ambiguous_glob_imports` lint into a hard error
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Dec 20, 2025

☀️ Try build successful (CI)
Build commit: aea72be (aea72be7e5474dab7f3e28cea3e2e6cfa3d1c3fa, parent: d0835adc4e114eac911150b3692b830b5583223a)

@petrochenkov
Copy link
Contributor Author

@craterbot
Copy link
Collaborator

👌 Experiment pr-149195-3 created and queued.
🤖 Automatically detected try build aea72be
🔍 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 craterbot 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 Dec 20, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-149195-3 is now running

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-resolve Area: Name/path resolution done by `rustc_resolve` specifically A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-crater Status: Waiting on a crater run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants