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

Extra non_snake_case warning when punning field name in pattern #89469

Closed
cole-miller opened this issue Oct 2, 2021 · 2 comments · Fixed by #89473
Closed

Extra non_snake_case warning when punning field name in pattern #89469

cole-miller opened this issue Oct 2, 2021 · 2 comments · Fixed by #89473
Assignees
Labels
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.

Comments

@cole-miller
Copy link

cole-miller commented Oct 2, 2021

Here's a minimized version of the code I'm working with:

#[derive(Clone, Copy)]
#[allow(non_snake_case)]
struct Entry {
    Δ: u16,
    : u16,
    : u8,
}

static TABLE: [Entry; 1] = [Entry { Δ: 0, : 0, : 0 }];

pub mod inner {
    pub fn f(k: usize) -> u16 {
        let super::Entry { Δ,,} = super::TABLE[k]; /* here */
        Δ
    }
}

Compiling this on stable or nightly gives a non_snake_case warning on the indicated line, despite the allow(non_snake_case) attribute on struct Entry. A similar issue was reported before in #66362, and addressed in a follow-up PR #66660, but it seems to be cropping up again here, maybe because a non-ASCII field name is involved. It was agreed in #66362 that punning of non-snake-case fields in patterns should not produce an additional warning, so I think this is a bona fide bug. (Incidentally, on stable the same warning appears twice in the compiler output, but this duplication seems to be fixed on nightly.)

@cole-miller cole-miller 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 Oct 2, 2021
@FabianWolff
Copy link
Contributor

It's actually unrelated to the non-ASCII field names; reduced:

#[allow(non_snake_case)]
struct Entry {
    A: u16,
    a: u16
}

fn foo() -> Entry {todo!()}

pub fn f() {
    let Entry { A, a } = foo();
    let _ = (A, a);
}

@rustbot claim

@cole-miller cole-miller changed the title Extra non_snake_case warning when punning non-ASCII field name in pattern Extra non_snake_case warning when punning field name in pattern Oct 2, 2021
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 5, 2021
…lett

Fix extra `non_snake_case` warning for shorthand field bindings

Fixes rust-lang#89469. The problem is the innermost `if` condition here:
https://github.com/rust-lang/rust/blob/d14731cb3ced8318d7fc83cbe838f0e7f2fb3b40/compiler/rustc_lint/src/nonstandard_style.rs#L435-L452

This code runs for every `PatKind::Binding`, so if a struct has multiple fields, say A and B, and both are bound in a pattern using shorthands, the call to `self.check_snake_case()` will indeed be skipped in the `check_pat()` call for `A`; but when `check_pat()` is called for `B`, the loop will still iterate over `A`, and `field.ident (= A) != ident (= B)` will be true. I have fixed this by only looking at non-shorthand bindings, and only the binding that `check_pat()` was actually called for.
@bors bors closed this as completed in c2bfe45 Oct 5, 2021
@cole-miller
Copy link
Author

Thanks @FabianWolff!

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 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.

2 participants