Remove Visibility field from enum variants #28442
Conversation
|
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
|
would it be possible to land this with #28440? It's a little unfortunate to have dependencies between PRs. |
|
Agree with Alex that this seems to be a clear bugfix, but more crater runs are always better 👍
|
nagisa
force-pushed
the
remove-enum-vis-field
branch
from
4f4beb3 to
f5a99ae
Compare
|
Crater's having a little trouble this morning so it may be a moment before I get results, but they're on the way! |
|
Looks like this has zero regressions on crater. cc @rust-lang/lang, @brson, any objections to patching this hole without a warning? I'd be fine just mentioning this in the release notes and merging as is personally |
|
Given the crater impact, I'd say no warning cycle is needed. |
|
Agreed, let's just fix it. |
| @@ -1131,14 +1118,10 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { | |||
| check_inherited(tcx, i.span, i.vis); | |||
| } | |||
| } | |||
| hir::ItemEnum(ref def, _) => { | |||
There was a problem hiding this comment.
Ah by the way, I want a super review on this removal by somebody familiar with rustc_privacy. I’m not exactly super familiar with rustc_privacy myself and since no tests got broken, I assume it is fine.
There was a problem hiding this comment.
Yeah this is fine, it's just checking that inside functions any non-inherited visibility yield an error but if variants don't have visibility there's no need to check.
alexcrichton
added
the
relnotes
Followup on #28440 Do not merge before the referenced PR is merged. I will fix the PR once that is merged (or close if it is not)
Followup on #28440
Do not merge before the referenced PR is merged. I will fix the PR once that is merged (or close if it is not)