Remove Visibility field from enum variants

[nagisa]

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)

[rust-highfive]

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

would it be possible to land this with #28440? It's a little unfortunate to have dependencies between PRs.

[steveklabnik]

Agree with Alex that this seems to be a clear bugfix, but more crater runs are always better 👍

On Sep 16, 2015, at 22:42, Alex Crichton notifications@github.com wrote:

would it be possible to land this with #28440? It's a little unfortunate to have dependencies between PRs.

—
Reply to this email directly or view it on GitHub.

[nagisa]

I’m fine with @matklad cherry-picking commits from this PR and I can rebase onto his work as well.

@matklad what’s your preferred course of action?

(NOTE: I had to rebase e3be84c onto master first, because there were some non-trivial merge conflicts otherwise)

[matklad]

Let's continue with this PR and close #28440

Note that there is one more PR which somehow relates to this issue: #28444

To avoid further PR dependencies, I'd suggest adding

//~ ERROR: expected one of `(`, `,`, `=`, `{`, or `}`, found `duck`

to the parse-fail/issue-28433.

[alexcrichton]

Crater's having a little trouble this morning so it may be a moment before I get results, but they're on the way!

[alexcrichton]

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

[nikomatsakis]

Given the crater impact, I'd say no warning cycle is needed.

[aturon]

Agreed, let's just fix it.

[nagisa]

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.

[alexcrichton]

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]

@bors: r+ a9cb51c

Thanks @nagisa!

[bors]

⌛ Testing commit a9cb51c with merge 72a10fa...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt