Parse unnamed fields and anonymous structs or unions

[frank-king]

This PR is an implementation of #49804 and inherited from #99754 but uses a slightly different strategy of recovering anonymous structs or unions.

First, it tries to parse the union { $( $field:ident: $ty:ty )* } as an anonymous union.

If successful, there can be no chance that the union with a record struct body is a union identifier with another kind of block, except for an empty body. We give up the recovery in case of the union {} is just a normal identifier such as in impl union {}.

If any error occurs during the recovery, we also give up it in case of the non-empty block might be another kind of block.

The struct is treated similarly, except that a strict error will be emitted if the struct happens to be in an identifier position.

[rustbot]

r? @fee1-dead

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

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

[frank-king]

r? @calebcartwright
r? @cjgillot

[frank-king]

Trying another recovery strategy. Here are the description copied from AllowAnonStructOrUnion's doc:

Parse

Parse the anonymous structs or unions regularly.

It is always okay to parse it in fields definitions, and nice to
recover.

#[repr(C)]
struct Foo {
    _: struct {
        a: u32,
        b: u64,
        _: union {
            c: f64,
            d: char,
        }
    },
}

Similarly, for types of parameters, where there is no second way to parse
the anonymous structs or unions, we can parse it and report an error that
anonymous structs or unions are not allowed here.

Recover

Recover an anonymous struct or union for diagnostic, but be more
conservative than Yes if there are other ways to parse it.

This mode is preferred where types are allowed to be followed by a block
{ ... }, where the block cannot contain field definitions. Such as:

• impl $(Trait for)? Type {}
• fn foo() -> Type {}

Except for some special cases (will discuss below), anonymous structs or
unions with empty fields are always ambiguous since the empty block {}
can also be valid impl bodies or fn bodies, so we usually shouldn't
allow recovery of empty anonymous structs or unions.

Examples

Empty blocks

# trait Foo {}
# struct union {}
impl union {}
impl Foo for union {}
fn foo() -> union {
    union {}
}

Non-empty blocks

// This will be recovered as an anonymous struct
impl struct { foo: Foo } {
    fn foo() {}
}
// This will be first recovered as an anonymous union (but will fail)
// and then parsed as a type named `union` followed by an `fn` block.
fn bar() -> union {
    let _i = 0;
}

A Special Case

Especially in impl items, even if for an anonymous struct or union with
empty fields, we can still allow recovering it as long as it is followed by
some tokens such as where, for, or {.

impl union {} for union {} where union: union {}

Disabled

Disallow parsing of anonymous structs or unions.

Generally, we should avoid parsing or recovering anonymous structs or unions
if we have no information of what can appear in a block { ... } followed
by a type, (such as a $ty:ty { /* any macro rules */ } in macros), or
otherwise the following codes will be broken.

macro_rules! ty {
    ($ty:ty { $($field:ident : $field_ty:ty)* }) => {};
}
ty!(union { union: union });
ty!(union {});

[petrochenkov]

Does "the initial implementation" mean this earlier comment #84415 (comment)?

Yes, that comment and the PR implementing it - #84571

[petrochenkov]

@joshtriplett

Do you see any downsides to not parsing this in ty, other than what I wrote here?

Only the fact that ty will no longer mean a type, but only a subset of types, but we have some similar precedents due to backward compatibility.
Not sure about practical downsides.
Macros may need to just pass the type further, without inspecting it, in that case ty would work well enough.

I also wonder whether the breakage from parsing struct { ... } in ty is purely theoretical, or it actually happens in practice.
We should probably check it with crater.

[petrochenkov]

@frank-king
I think we can land the non-controversial part of this PR easily enough:

• all the AST structures and AST validation can be kept
• but parser would only parse the new types in struct field positions specifically

, and then think about the remaining parser changes.

[frank-king]

but parser would only parse the new types in struct field positions specifically

Is it that we remove all AllowAnonStructOrUnion::RecoverNonEmptyOrElse(..) and use AllowAnonStructOrUnion::No instead?

Or to simplify, we can also extract the parsing of anonymous structs or unions from parse_ty_common into a separate parse_anon_struct_or_union function and put it in the beginning of parse_ty_for_field_def, guarded by a can_start_anon_struct_or_union condition.

[petrochenkov]

separate parse_anon_struct_or_union function and put it in the beginning of parse_ty_for_field_def

This should be enough for now.

[petrochenkov]

@frank-king
Perhaps it's better to move the implementation for "unnamed fields lite" to a new PR and keep this PR as is for further discussion.

[frank-king]

it's better to move the implementation for "unnamed fields lite"

I'm not quite sure about which things I should move. I guess is to squash these current commits into a single one (opting out the implementation details) and move it onto a new clean PR, is it?

[bors]

☔ The latest upstream changes (presumably #115131) made this pull request unmergeable. Please resolve the merge conflicts.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting. We agreed that :ty should not parse anonymous types.

We discussed, briefly, whether there should be a future macro matcher that includes them (matching anonymous types or matching whole field definitions), but did not reach a consensus, and we don't think this should block on the proposal or acceptance of such a matcher.

Please go ahead with :ty not matching anonymous types.

[scottmcm]

Given that, my first reaction is that ty should not include anonymous structs/unions. But that should be confirmed by lang. I'll raise this in today's lang meeting.

I think that ty is reasonably used in enough places that wouldn't want this -- like someone doing pat_param : ty -- that I also would expect ty to not accept struct {.

[frank-king]

So I think anonymous types should belong to a new kind of FieldDef, is it?

[nikomatsakis]

It doesn't seem that there are any unanswered questions for lang-team to answer, so...

@rustbot labels -I-lang-nominated

[nikomatsakis]

@frank-king

So I think anonymous types should belong to a new kind of FieldDef, is it?

Do you mean like a new kind of macro matcher? I think that can be handled in a separate PR

[frank-king]

@frank-king

So I think anonymous types should belong to a new kind of FieldDef, is it?

Do you mean like a new kind of macro matcher? I think that can be handled in a separate PR

No, that's beyond the goal of this PR. I was just making sure if I understood it correctly (to extend the definition of FieldDef::ty to support unnamed fields, see #115732 for the details)

[Dylan-DPC]

@frank-king any updates on this?

[frank-king]

@frank-king any updates on this?

Oh, I forgot to push my local commits ... I'll do a rebase and push again, and it is now worked on #115732.

[JohnCSimon]

@frank-king
ping from triage - can you post your status on this PR? This PR has not received an update in a few months. Thank you!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[frank-king]

@frank-king ping from triage - can you post your status on this PR? This PR has not received an update in a few months. Thank you!

FYI: when a PR is ready for review, send a message containing @rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

I'm going to close this PR, as it is already implemented by #115131 and I'm now working on #115367. This PR is only retained for discussing the recovery strategy of parsing.