Conversation
This comment has been minimized.
This comment has been minimized.
jieyouxu
left a comment
There was a problem hiding this comment.
Thanks for the fix. A few nits.
| mod foo { | ||
| pub use d::*; // this imports d::Foo | ||
| pub use m::Foo; // this should shadow d::Foo | ||
| } |
There was a problem hiding this comment.
Suggestion: a few things:
- Move this test to
tests/ui/resolve/, and - Rename it to sth more meaningful, like
non-glob-vs-glob-reexport-precedence - 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 { | |||
There was a problem hiding this comment.
Suggestion: can you rename these test modules as something more semantically meaningful, e.g.
test1->glob_vs_glob_ambiguitytest2->non_glob_vs_non_glob_name_collision
There was a problem hiding this comment.
Suggestion:
- Rename this test as sth more meaningful like
import-glob-ambiguity-non-glob-collision - Ditto on the doc comment for a backlink.
|
☔ The latest upstream changes (presumably #142033) made this pull request unmergeable. Please resolve the merge conflicts. |
|
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 |
|
At this point I think it is easier to redo the PR |
Fixes #140780 and #140765
r? jieyouxu