Skip to content

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented May 15, 2025

Fixes #140780 and #140765

r? jieyouxu

@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 May 15, 2025
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 15, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. A few nits.

Comment on lines 7 to 10
mod foo {
pub use d::*; // this imports d::Foo
pub use m::Foo; // this should shadow d::Foo
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: a few things:

  1. Move this test to tests/ui/resolve/, and
  2. Rename it to sth more meaningful, like non-glob-vs-glob-reexport-precedence
  3. Add a doc comment to the test to back link to the issue, e.g. //! Regression test for #14082.

@@ -0,0 +1,38 @@
#![allow(unused_imports, dead_code)]

mod test1 {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: can you rename these test modules as something more semantically meaningful, e.g.

  • test1 -> glob_vs_glob_ambiguity
  • test2 -> non_glob_vs_non_glob_name_collision

@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 May 16, 2025
Copy link
Member

@jieyouxu jieyouxu May 16, 2025

Choose a reason for hiding this comment

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

Suggestion:

  • Rename this test as sth more meaningful like import-glob-ambiguity-non-glob-collision
  • Ditto on the doc comment for a backlink.

@bors
Copy link
Collaborator

bors commented Jun 5, 2025

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

@JohnCSimon
Copy link
Member

@pvdrz

ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Please address the feedback from the reviewer. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@pvdrz
Copy link
Contributor Author

pvdrz commented Jul 14, 2025

At this point I think it is easier to redo the PR

@pvdrz pvdrz closed this Jul 14, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 14, 2025
@pvdrz pvdrz deleted the fix-tests branch July 14, 2025 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test for #7663 doesn't test anything (?)
6 participants