C23 structure and union compatibility should allow for compatible members

[Halalaluyafail3]

It seems like Clang is working to support C23 new tag compatibility rules, however there are some issues currently. Here is one of them:

struct A{int h;};
int main(){
    extern struct A x;
    enum E:int{e};
    struct A{enum E h;};
    extern struct A x;
}

Clang rejects this code because it considers the two structure types to be incompatible. They should be compatible because int and enum E are compatible, exact type equivalence is not required by the standard. The resulting composite type for x should have h as the composite type of int and enum E.

[frederick-vs-ja]

CC @AaronBallman @uecker

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (Halalaluyafail3)

It seems like Clang is working to support C23 new tag compatibility rules, however there are some issues currently. Here is one of them: ```c struct A{int h;}; int main(){ extern struct A x; enum E:int{e}; struct A{enum E h;}; extern struct A x; } ``` Clang rejects this code because it considers the two structure types to be incompatible. They should be compatible because int and enum E are compatible, exact type equivalence is not required by the standard. The resulting composite type for x should have h as the composite type of int and enum E.

[AaronBallman]

I believe you're correct that this should be accepted. But it seems GCC also rejects (though SDCC accepts): https://godbolt.org/z/dPs8nvrzr

[uecker]

For redeclarations in the same scope we have the stricter requirement that the member do not only have to be compatible but also the "same" type. This is the same rule as for typedef redeclarations: https://godbolt.org/z/eh7GKTYdh

[uecker]

Better example: https://godbolt.org/z/Pqe49P4e1 (I do not understand what SDCC is doing)

[Halalaluyafail3]

I believe you're correct that this should be accepted. But it seems GCC also rejects (though SDCC accepts): https://godbolt.org/z/dPs8nvrzr

The original program I provided is accepted https://godbolt.org/z/vss8KdYbG.

[AaronBallman]

For redeclarations in the same scope we have the stricter requirement that the member do not only have to be compatible but also the "same" type. This is the same rule as for typedef redeclarations: https://godbolt.org/z/eh7GKTYdh

Thank you! I always forget to look at 6.7.3.4 when reasoning about this, which is why I missed the "have the same (and not merely compatible) types" logic. But that probably explains the difference in GCC behavior between my example (same scope) and the original example (different scopes).

I think Clang gets this correct in terms of the redefinitions of the types: https://godbolt.org/z/vMGcc8K4E
Where I think we're falling down is with the redeclaration chain itself, we're not diagnosing the definition of struct A, we're diagnosing the redeclaration of x: https://godbolt.org/z/TaK1dM8b3

Still investigating...

[AaronBallman]

I think the issue is here:

llvm-project/clang/lib/AST/ASTStructuralEquivalence.cpp

Line 864 in 081b74c

if (T1->getTypeClass() != T2->getTypeClass()) {

We're checking for the equivalence of two types, the type classes are not the same, but we're not paying attention to enumerations there only functions with/without prototypes.

[AaronBallman]

Ugh, if I'm understanding correctly, I think we actually have another problem that's much harder to resolve -- Clang doesn't consider scope when checking structural equivalence. This functionality was originally implemented in support of C++ modules which don't care about the scope of declarations.

// Test that enumerations are compatible with their underlying type, but still
// diagnose when "same type" is required rather than merely "compatible type".
struct GH149965_1 { int h; };
struct GH149965_2 { int h; };
void gh149965(void) {
  extern struct GH149965_1 x1; // c17-note {{previous declaration is here}}
  extern struct GH149965_2 x2; // c17-note {{previous declaration is here}}

  enum E1 : int { e1 }; // Fixed underlying type
  enum E2 { e2 };       // Unfixed underlying type, defaults to int in Clang (unsigned in GCC)

  // Both the structure and the variable declarations are fine because only a
  // compatible type is required, not the same type, because the structures are
  // declared in different scopes.
  struct GH149965_1 { enum E1 h; };
  struct GH149965_2 { enum E2 h; };

  extern struct GH149965_1 x1; // c17-error {{redeclaration of 'x1'}}
  extern struct GH149965_2 x2; // c17-error {{redeclaration of 'x2'}}

  // However, in the same scope, the same type is required, not just compatible
  // types.
  // FIXME: this should be an error in both C17 and C23 mode.
  struct GH149965_3 { int h; };     // c17-note {{previous definition is here}}
  struct GH149965_3 { enum E1 h; }; // c17-error {{redefinition of 'GH149965_3'}}
}

GCC rejects the last declaration which I believe is the correct behavior. With my fix Clang will accept it. Trying to factor scope into things is... nontrivial (we run into this code when we try to merge two types, scope information is not always available and definitely not threaded through to where the checking happens). Refactoring that would be a major undertaking, so I'm not certain whether it's better to reject as we do today or to accept incorrectly with a partial fix.