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

Attributes are silently ignored on .. (rest) in struct pattern (StructPatternEtCetera) #81282

Closed
Aaron1011 opened this issue Jan 22, 2021 · 7 comments · Fixed by #136490
Closed
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-parser Area: The parsing of Rust source code to an AST A-patterns Relating to patterns and pattern matching C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Aaron1011
Copy link
Member

The following code compiles successfully:

struct Foo {
    val: bool
}

fn main() {
    let data = Foo { val: true };
    match data {
        Foo { #[missing_attr] #[cfg(FALSE)] .. } => {}
    }
}

However, placing attributes on a normal field pattern:

struct Foo {
    val: bool
}

fn main() {
    let data = Foo { val: true };
    match data {
        Foo { #[missing_attr] #[cfg(FALSE)] val } => {}
    }
}

produces the following error:

error[E0027]: pattern does not mention field `val`
 --> src/main.rs:8:9
  |
8 |         Foo { #[missing_attr] #[cfg(FALSE)] val } => {}
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `val`
  |
help: include the missing field in the pattern
  |
8 |         Foo { val } => {}
  |             ^^^^^^^
help: if you don't care about this missing field, you can explicitly ignore it
  |
8 |         Foo { .. } => {}
  |             ^^^^^^

error: aborting due to previous error

We should either handle attributes consistently across field patterns and .. patterns, or emit an error when any attributes are present on .. patterns. This is technically a breaking change, but there's precedent here (we've made misplaced #[inline] attributes into hard errors).

@Aaron1011 Aaron1011 added A-attributes Area: Attributes (`#[…]`, `#![…]`) C-bug Category: This is a bug. A-patterns Relating to patterns and pattern matching labels Jan 22, 2021
@Aaron1011 Aaron1011 changed the title Attributes are silently ignored in .. in struct pattern Attributes are silently ignored on .. in struct pattern Jan 22, 2021
@petrochenkov
Copy link
Contributor

either handle attributes consistently across field patterns and .. patterns

Technically .. is not a pattern in this context (unlike .. in other contexts), it's a part of the struct pattern syntax, so attributes are supposed to be syntactically rejected on it.

@Aaron1011
Copy link
Member Author

The reference states that .. (StructPatternEtCetera) supports outer attributes: https://doc.rust-lang.org/reference/patterns.html#struct-patterns. Should it be updated?

@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 22, 2021

Actually disregard my comment, it's true that .. is not a pattern in this context, but it doesn't matter - attributes on regular patterns are not supported at all.
So whether attributes should be supported on .. in struct patterns is a standalone question.

I'd say they should not despite what the reference says.
The equivalent syntax is not supported in expressions

struct S {}

fn main() {
    let s = S {};
    let z = S { #[nonexistent] ..s }; // error: expected identifier, found `..`
}

and we should be able to remove it from patterns too because it's not implemented correctly (the attributes on .. are not resolved/expanded).

If someone provides good motivation, then it can always be added in the future.

@jplatte
Copy link
Contributor

jplatte commented Apr 19, 2021

I have a use case for this: Allowing exhaustive matching of conceptually non-exhaustive structs, like what syn does for enums:

let Foo {
    field_a,
    field_b,
    field_c,
    #[cfg(test)]
    __test_exhaustive: _,
    #[cfg(not(test))]
    ..
} = foo;

or alternatively with the lint suggested in #44109 (comment):

let Foo {
    field_a,
    field_b,
    field_c,
    // could also be warn(reachable) + `-D warnings` in CI
    #[cfg_attr(test, deny(reachable))]
    ..
} = foo;

With that, you have an automated way of being notified (through test failure) when a struct got additional fields without failing compilation entirely, in the second case for anything that's #[non_exhaustive], without help from the library defining the type.

@Nadrieril
Copy link
Member

@jplatte I believe the non_exhaustive_omitted_patterns lint handles your case without attributes.

@jplatte
Copy link
Contributor

jplatte commented Dec 7, 2023

Heh yes, I'm well aware of it ^^
I still would have found that a #[warn(reachable)] lint would have been more obvious when reading, but given how things turned out instead, that use case is not really valid anymore, so just forbidding attributes on .. in struct patterns might be the better way forward.

@Nadrieril
Copy link
Member

Yeahh, I opted against deny(reachable) for consistency in nested cases; I did not give much thought to readability

@ehuss ehuss marked this as a duplicate of #136108 Jan 27, 2025
@fmease fmease changed the title Attributes are silently ignored on .. in struct pattern Attributes are silently ignored on .. (rest) in struct pattern (StructPatternEtCetera) Jan 27, 2025
@jieyouxu jieyouxu added A-parser Area: The parsing of Rust source code to an AST T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Feb 3, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 3, 2025
… r=<try>

Do not allow attributes on struct field rest patterns

Fixes rust-lang#81282.

This removes support for attributes on struct field rest patterns (the `..` bit) from the parser. Previously any attributes were being parsed but dropped from the AST, so didn't work and were deleted by rustfmt.

This needs an equivalent change to the reference but I wanted to see how this PR is received first.
The error message it produces isn't great, however it does match the error you get if you try to add attributes to .. in struct expressions atm, although I can understand wanting to do better given this was previously accepted. I think I could move attribute parsing back up to where it was and then emit a specific new error for this case, however I might need some guidance as this is the first time I've messed around inside the compiler.

While this is technically breaking I don't think it's much of an issue: attributes in this position don't currently do anything and rustfmt outright deletes them, meaning it's incredibly unlikely to affect anyone. I have already made the equivalent change to *add* support for attributes (mostly) but the conversation in the linked issue suggested it would be more reasonable to just remove them (and pointed out it's much easier to add support later if we realise we need them).
@bors bors closed this as completed in 06b2f62 Feb 16, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2025
Rollup merge of rust-lang#136490 - Skepfyr:no-field-rest-pattern-attrs, r=compiler-errors

Do not allow attributes on struct field rest patterns

Fixes rust-lang#81282.

This removes support for attributes on struct field rest patterns (the `..` bit) from the parser. Previously any attributes were being parsed but dropped from the AST, so didn't work and were deleted by rustfmt.

This needs an equivalent change to the reference but I wanted to see how this PR is received first.
The error message it produces isn't great, however it does match the error you get if you try to add attributes to .. in struct expressions atm, although I can understand wanting to do better given this was previously accepted. I think I could move attribute parsing back up to where it was and then emit a specific new error for this case, however I might need some guidance as this is the first time I've messed around inside the compiler.

While this is technically breaking I don't think it's much of an issue: attributes in this position don't currently do anything and rustfmt outright deletes them, meaning it's incredibly unlikely to affect anyone. I have already made the equivalent change to *add* support for attributes (mostly) but the conversation in the linked issue suggested it would be more reasonable to just remove them (and pointed out it's much easier to add support later if we realise we need them).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-parser Area: The parsing of Rust source code to an AST A-patterns Relating to patterns and pattern matching C-bug Category: This is a bug. T-lang Relevant to the language 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