Undo the backwards incompatable part of the fix for issue #30159 in PR #30294. #30324
Conversation
jseyfried
force-pushed
the
unfix_30159
branch
from
d5c868e to
a5e5e82
Compare
jseyfried
changed the title
|
r+, but tests are failing I do think we should land the breaking change, but agree a crater run first would be good. |
|
If it's a bugfix, don't remove it completely, leave a warning at least. |
…to avoid breakage.
jseyfried
force-pushed
the
unfix_30159
branch
from
a5e5e82 to
de0de61
Compare
|
@nrc Fixed -- I was failing part of my own test from the first PR. |
|
@petrochenkov If we warned whenever the visibility got overwritten, that could lead to false positives -- warnings for code that would still be correct without the overwritten visibility. We could avoid some of these false positives with some extra work, but it would be very difficult to avoid all of them. On second thought, we may decide that false positive warnings aren't that bad because they always can be fixed by reordering the imports so the globs come first. Regardless, there is no point in implementing warnings and paying the cost of false positives if we are going to land the breaking change. Also, I want to land this PR asap since right now #30294 can break more than what is described in the merge commit. |
|
@bors: r+ |
|
📌 Commit de0de61 has been approved by |
r? @nrc Since PR #30294 unintentionally fixed issue #30159, it can cause breakage for a different reason than I originally stated in the PR (see #30159, I characterized the issue precisely there). This commit limits the scope of the breakage to what I originally stated in the PR by "unfixing" the backwards incompatible part of #30159. I think fixing #30159 has enough potential for breakage to warrant a crater run. If you disagree, I can cancel this PR, leaving it fixed.
… can make preceding imports public (fixes rust-lang#30159).
This reverts PR #30324, fixing bug #30159 in which a public a glob import makes public any preceding imports that share a name with an item in the module being glob imported from. For example, ```rust pub fn f() {} pub mod foo { fn f() {} } mod bar { use f; use f as g; pub use foo::*; // This makes the first import public but does not affect the second import. } ``` This is a [breaking-change].
r? @nrc
Since PR #30294 unintentionally fixed issue #30159, it can cause breakage for a different reason than I originally stated in the PR (see #30159, I characterized the issue precisely there).
This commit limits the scope of the breakage to what I originally stated in the PR by "unfixing" the backwards incompatible part of #30159.
I think fixing #30159 has enough potential for breakage to warrant a crater run. If you disagree, I can cancel this PR, leaving it fixed.