Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostic regression (1.47 -> 1.48): public enum fields incorrectly described as "inaccessible fields". #79593

Closed
eddyb opened this issue Dec 1, 2020 · 10 comments · Fixed by #79956
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-visibility Area: Visibility / privacy. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Dec 1, 2020

Test example (try on playground):

mod foo {
    pub struct Pub { private: () }

    pub enum Enum {
        Variant { x: (), y: () },
        Other
    }
    
    fn correct() {
        Pub {};
        Enum::Variant { x: () };
    }
}

fn correct() {
    foo::Pub {};
}

fn wrong() {
    foo::Enum::Variant { x: () };
}

Only foo::Pub {} in the top level correct should say "cannot construct ... due to inaccessible fields", the other 3 errors should be "missing field ... in initializer", but it seems like it's treating the enum field as private (even though it inherits pub from the enum).

The "cannot construct ... due to inaccessible fields" error seems to be new in 1.48, as part of #76524, and unmodified since.

It's using field.vis.is_accessible_from(...) which seems correct, but it's possible enum fields are incapable of giving the right result, if they don't track the enum's own privacy (and they only know they inherit it, whatever it is).

cc @davidtwco @estebank

@XAMPPRocky XAMPPRocky added A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. labels Dec 1, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 1, 2020

I would also suspect #78077.

@jyn514 jyn514 added A-visibility Area: Visibility / privacy. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Dec 1, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 1, 2020
@camelid
Copy link
Member

camelid commented Dec 1, 2020

I ran into this too, but I couldn't make a repro. Thanks eddyb!

@spastorino spastorino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2020
@camelid
Copy link
Member

camelid commented Dec 11, 2020

P-high: I've run into this a bunch of times and I imagine others have too. It's really annoying and confusing, likely even more so for newcomers!

@camelid camelid added P-high High priority D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Dec 11, 2020
@camelid
Copy link
Member

camelid commented Dec 11, 2020

searched nightlies: from nightly-2020-08-01 to nightly-2020-10-15
regressed nightly: nightly-2020-09-11
searched commits: from e2be5f5 to a1947b3
regressed commit: ad3a6f7

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-apple-darwin
Reproduce with:

cargo bisect-rustc --preserve --prompt --start=2020-08-01 --end=2020-10-15 

@camelid
Copy link
Member

camelid commented Dec 11, 2020

Seems very likely that this is due to #76524, as eddyb said.

EDIT: Though, as jyn514 suggested, the underlying bug could be #78077, which was then surfaced in #76524.

@camelid camelid self-assigned this Dec 11, 2020
@camelid
Copy link
Member

camelid commented Dec 12, 2020

Update: I've been reading the source code and print-debugging and I think the issue was not introduced in either of those two PRs, but rather that they surfaced a pre-existing issue.

@camelid
Copy link
Member

camelid commented Dec 12, 2020

Well, it took a while, but I think I fixed it!

@bors bors closed this as completed in d149b65 Dec 13, 2020
@ht990332
Copy link

Does this necessitate a 1.48.1 release or shall I wait for 1.49?

@jyn514
Copy link
Member

jyn514 commented Dec 13, 2020

@ht990332
Copy link

ht990332 commented Dec 13, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-visibility Area: Visibility / privacy. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants