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

Edge case does not suggest changing invalid public with pub #101626

Closed
Rageking8 opened this issue Sep 9, 2022 · 7 comments · Fixed by #101668
Closed

Edge case does not suggest changing invalid public with pub #101626

Rageking8 opened this issue Sep 9, 2022 · 7 comments · Fixed by #101668
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Rageking8
Copy link
Contributor

Rageking8 commented Sep 9, 2022

Given the following code: link

mod test {
    public const X: i32 = 123;
}

fn main() {
    println!("{}", test::X);
}

The current output on stable 1.63.0:

   Compiling playground v0.0.1 (/playground)
error: expected one of `!` or `::`, found keyword `const`
 --> src/main.rs:2:12
  |
2 |     public const X: i32 = 123;
  |            ^^^^^ expected one of `!` or `::`

error: could not compile `playground` due to previous error

The current output on Beta version 1.64.0-beta.6 (2022-09-09 25912c0):

   Compiling playground v0.0.1 (/playground)
error: expected one of `!` or `::`, found keyword `const`
 --> src/main.rs:2:12
  |
2 |     public const X: i32 = 123;
  |            ^^^^^ expected one of `!` or `::`
  |
help: write `pub` instead of `public` to make the item public
  |
2 |     pub const X: i32 = 123;
  |     ~~~

error: could not compile `playground` due to previous error

The current output on Nightly version 1.65.0-nightly (2022-09-09 1d37ed6):

   Compiling playground v0.0.1 (/playground)
error: expected one of `!` or `::`, found keyword `const`
 --> src/main.rs:2:12
  |
2 |     public const X: i32 = 123;
  |            ^^^^^ expected one of `!` or `::`

error: could not compile `playground` due to previous error

Since #99903 ( cc @gimbles ) is somewhat recent the help diagnostics improvement currently is only on beta. However, the latest nightly has regressions regarding the suggestion as shown above. Hence, the output should suggest changing public with pub as that is the desired output.

Note: base issue for improved diagnostics (public -> pub): #99653

Note: since this is technically a regression from beta to nightly, what should be the appropriate label to add? The one below this?
@rustbot label +regression-untriaged

@Rageking8 Rageking8 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 9, 2022
@rustbot rustbot added regression-untriaged Untriaged performance or correctness regression. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 9, 2022
@gimbling-away
Copy link
Contributor

Can you please add the Rustc versions?

@gimbling-away
Copy link
Contributor

[Note that I don't think this should have happened at all, I wrote tests for the changed diagnostic suggestions]

@Rageking8
Copy link
Contributor Author

@gimbles I have retested the above code snippet against the current stable, beta and nightly version (I have added the version numbers above) of the compiler in the official rust playground and the regression still persist.

Not sure if this only occurs on the rust playground although I highly doubt it. Maybe you could look into this as the implementation of that diagnostics improvement is written by you. If you have any evidence that this regression issue is misinformed do let me know and I will happily close the issue. Thanks a lot.

@Rageking8
Copy link
Contributor Author

[Note that I don't think this should have happened at all, I wrote tests for the changed diagnostic suggestions]

Not sure if this is the cause but maybe the test you wrote in #99903 is slightly different to the above code snippet as the test uses the wrong public keyword in the top-level. However, the above code snippet uses the incorrect public keyword inside of a user-defined module. Hence, this may lead to a PR that potentially causes a regression in the recovery of public -> pub to not trip off a CI fail. Therefore, undetected.

That being said, all those are just my guess and it may be wrong.

@rustbot label +E-needs-bisection

@rustbot rustbot added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 10, 2022
@chenyukang
Copy link
Member

This may related to my PR:
#100188

@chenyukang
Copy link
Member

@rustbot claim

@Rageking8
Copy link
Contributor Author

@rustbot label -E-needs-bisection

@rustbot rustbot removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 11, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 12, 2022
Suggest pub instead of public for const type item

Fixes rust-lang#101626
@bors bors closed this as completed in 975dd6c Sep 12, 2022
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 20, 2022
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 regression-untriaged Untriaged performance or correctness regression. 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.

5 participants