Skip to content

Commit 47f3ab2

Browse files
AaronBallmanAaronBallman
committed
[C23] More improved type compatibility for enumerations (llvm#150946)
The structural equivalence checker was not paying attention to whether enumerations had compatible fixed underlying types or not. Fixes llvm#150594
1 parent d8e9216 commit 47f3ab2

File tree

6 files changed

+168
-0
lines changed

6 files changed

+168
-0
lines changed

‎clang/include/clang/Basic/DiagnosticASTKinds.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,14 @@ def note_odr_number_of_bases : Note<
507507
"class has %0 base %plural{1:class|:classes}0">;
508508
def note_odr_enumerator : Note<"enumerator %0 with value %1 here">;
509509
def note_odr_missing_enumerator : Note<"no corresponding enumerator here">;
510+
def note_odr_incompatible_fixed_underlying_type : Note<
511+
"enumeration %0 declared with incompatible fixed underlying types (%1 vs. "
512+
"%2)">;
513+
def note_odr_fixed_underlying_type : Note<
514+
"enumeration %0 has fixed underlying type here">;
515+
def note_odr_missing_fixed_underlying_type : Note<
516+
"enumeration %0 missing fixed underlying type here">;
517+
510518
def err_odr_field_type_inconsistent : Error<
511519
"field %0 declared with incompatible types in different "
512520
"translation units (%1 vs. %2)">;

‎clang/lib/AST/ASTStructuralEquivalence.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2067,6 +2067,48 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
20672067
!CheckStructurallyEquivalentAttributes(Context, D1, D2))
20682068
return false;
20692069

2070+
// In C23, if one enumeration has a fixed underlying type, the other shall
2071+
// have a compatible fixed underlying type (6.2.7).
2072+
if (Context.LangOpts.C23) {
2073+
if (D1->isFixed() != D2->isFixed()) {
2074+
if (Context.Complain) {
2075+
Context.Diag2(D2->getLocation(),
2076+
Context.getApplicableDiagnostic(
2077+
diag::err_odr_tag_type_inconsistent))
2078+
<< Context.ToCtx.getTypeDeclType(D2)
2079+
<< (&Context.FromCtx != &Context.ToCtx);
2080+
Context.Diag1(D1->getLocation(),
2081+
D1->isFixed()
2082+
? diag::note_odr_fixed_underlying_type
2083+
: diag::note_odr_missing_fixed_underlying_type)
2084+
<< D1;
2085+
Context.Diag2(D2->getLocation(),
2086+
D2->isFixed()
2087+
? diag::note_odr_fixed_underlying_type
2088+
: diag::note_odr_missing_fixed_underlying_type)
2089+
<< D2;
2090+
}
2091+
return false;
2092+
}
2093+
if (D1->isFixed()) {
2094+
assert(D2->isFixed() && "enums expected to have fixed underlying types");
2095+
if (!IsStructurallyEquivalent(Context, D1->getIntegerType(),
2096+
D2->getIntegerType())) {
2097+
if (Context.Complain) {
2098+
Context.Diag2(D2->getLocation(),
2099+
Context.getApplicableDiagnostic(
2100+
diag::err_odr_tag_type_inconsistent))
2101+
<< Context.ToCtx.getTypeDeclType(D2)
2102+
<< (&Context.FromCtx != &Context.ToCtx);
2103+
Context.Diag2(D2->getLocation(),
2104+
diag::note_odr_incompatible_fixed_underlying_type)
2105+
<< D2 << D2->getIntegerType() << D1->getIntegerType();
2106+
}
2107+
return false;
2108+
}
2109+
}
2110+
}
2111+
20702112
llvm::SmallVector<const EnumConstantDecl *, 8> D1Enums, D2Enums;
20712113
auto CopyEnumerators =
20722114
[](auto &&Range, llvm::SmallVectorImpl<const EnumConstantDecl *> &Cont) {

‎clang/test/ASTMerge/enum/Inputs/enum3.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// [C23] missing underlying types
2+
enum E1 : int {
3+
E1Enumerator1
4+
};
5+
6+
enum E2 {
7+
E2Enumerator1
8+
};
9+
10+
// [C23] Incompatible underlying types
11+
enum E3 : long {
12+
E3Enumerator1
13+
};
14+

‎clang/test/ASTMerge/enum/Inputs/enum4.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// [C23] missing underlying types
2+
enum E1 {
3+
E1Enumerator1
4+
};
5+
6+
enum E2 : int {
7+
E2Enumerator1
8+
};
9+
10+
// [C23] Incompatible underlying types
11+
enum E3 : short {
12+
E3Enumerator1
13+
};
14+

‎clang/test/ASTMerge/enum/test2.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// RUN: %clang_cc1 -std=c23 -emit-pch -o %t.1.ast %S/Inputs/enum3.c
2+
// RUN: %clang_cc1 -std=c23 -emit-pch -o %t.2.ast %S/Inputs/enum4.c
3+
// RUN: not %clang_cc1 -std=c23 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
4+
5+
// FIXME: this error should not happen!
6+
// CHECK: error: type 'struct __NSConstantString_tag' has an attribute which currently causes the types to be treated as though they are incompatible
7+
// CHECK: enum3.c:2:6: warning: type 'enum E1' has incompatible definitions in different translation units
8+
// CHECK: enum4.c:2:6: note: enumeration 'E1' missing fixed underlying type here
9+
// CHECK: enum3.c:2:6: note: enumeration 'E1' has fixed underlying type here
10+
// CHECK: enum3.c:6:6: warning: type 'enum E2' has incompatible definitions in different translation units
11+
// CHECK: enum4.c:6:6: note: enumeration 'E2' has fixed underlying type here
12+
// CHECK: enum3.c:6:6: note: enumeration 'E2' missing fixed underlying type here
13+
// CHECK: enum3.c:11:6: warning: type 'enum E3' has incompatible definitions in different translation units
14+
// CHECK: enum3.c:11:6: note: enumeration 'E3' declared with incompatible fixed underlying types ('long' vs. 'short')
15+
// CHECK: 3 warnings and 1 error generated
16+

‎clang/test/C/C23/n3037.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,3 +401,77 @@ _Static_assert(0 == _Generic(inner_anon_tagged.untagged, struct { int i; } : 1,
401401
// unions and structures are both RecordDecl objects, whereas EnumDecl is not).
402402
enum { E_Untagged1 } nontag_enum; // both-note {{previous definition is here}}
403403
_Static_assert(0 == _Generic(nontag_enum, enum { E_Untagged1 } : 1, default : 0)); // both-error {{redefinition of enumerator 'E_Untagged1'}}
404+
405+
// Test that enumerations with mixed underlying types are properly handled.
406+
enum GH150594_E1 : int { GH150594_Val1 };
407+
enum GH150594_E2 : int { GH150594_Val2 };
408+
enum GH150594_E3 { GH150594_Val3 };
409+
enum GH150594_E4 : int { GH150594_Val4 };
410+
void GH150594(void) {
411+
extern enum GH150594_E1 Fn1(void); // both-note {{previous declaration is here}}
412+
extern enum GH150594_E2 Fn2(void); // c17-note {{previous declaration is here}}
413+
extern enum GH150594_E3 Fn3(void); // both-note {{previous declaration is here}}
414+
extern enum GH150594_E4 Fn4(void); // both-note {{previous declaration is here}}
415+
enum GH150594_E1 { GH150594_Val1 };
416+
enum GH150594_E2 : int { GH150594_Val2 };
417+
enum GH150594_E3 : int { GH150594_Val3 };
418+
enum GH150594_E4 : short { GH150594_Val4 };
419+
extern enum GH150594_E1 Fn1(void); // both-error {{conflicting types for 'Fn1'}}
420+
extern enum GH150594_E2 Fn2(void); // c17-error {{conflicting types for 'Fn2'}}
421+
extern enum GH150594_E3 Fn3(void); // both-error {{conflicting types for 'Fn3'}}
422+
extern enum GH150594_E4 Fn4(void); // both-error {{conflicting types for 'Fn4'}}
423+
424+
// Show that two declarations in the same scope give expected diagnostics.
425+
enum E1 { e1 }; // both-note {{previous declaration is here}}
426+
enum E1 : int { e1 }; // both-error {{enumeration previously declared with nonfixed underlying type}}
427+
428+
enum E2 : int { e2 }; // both-note {{previous declaration is here}}
429+
enum E2 { e2 }; // both-error {{enumeration previously declared with fixed underlying type}}
430+
431+
enum E3 : int { e3 }; // both-note {{previous declaration is here}}
432+
enum E3 : short { e3 }; // both-error {{enumeration redeclared with different underlying type 'short' (was 'int')}}
433+
434+
typedef short foo;
435+
enum E4 : foo { e4 }; // c17-note 2 {{previous definition is here}}
436+
enum E4 : short { e4 }; // c17-error {{redefinition of 'E4'}} \
437+
c17-error {{redefinition of enumerator 'e4'}}
438+
439+
enum E5 : foo { e5 }; // both-note {{previous declaration is here}}
440+
enum E5 : int { e5 }; // both-error {{enumeration redeclared with different underlying type 'int' (was 'foo' (aka 'short'))}}
441+
}
442+
443+
// Test that enumerations are compatible with their underlying type, but still
444+
// diagnose when "same type" is required rather than merely "compatible type".
445+
enum E1 : int { e1 }; // Fixed underlying type
446+
enum E2 { e2 }; // Unfixed underlying type, defaults to int or unsigned int
447+
448+
struct GH149965_1 { int h; };
449+
// This typeof trick is used to get the underlying type of the enumeration in a
450+
// platform agnostic way.
451+
struct GH149965_2 { __typeof__(+(enum E2){}) h; };
452+
void gh149965(void) {
453+
extern struct GH149965_1 x1; // c17-note {{previous declaration is here}}
454+
extern struct GH149965_2 x2; // c17-note {{previous declaration is here}}
455+
456+
// Both the structure and the variable declarations are fine because only a
457+
// compatible type is required, not the same type, because the structures are
458+
// declared in different scopes.
459+
struct GH149965_1 { enum E1 h; };
460+
struct GH149965_2 { enum E2 h; };
461+
462+
extern struct GH149965_1 x1; // c17-error {{redeclaration of 'x1'}}
463+
extern struct GH149965_2 x2; // c17-error {{redeclaration of 'x2'}}
464+
465+
// However, in the same scope, the same type is required, not just compatible
466+
// types.
467+
// FIXME: this should be an error in both C17 and C23 mode.
468+
struct GH149965_3 { int h; }; // c17-note {{previous definition is here}}
469+
struct GH149965_3 { enum E1 h; }; // c17-error {{redefinition of 'GH149965_3'}}
470+
471+
// For Clang, the composite type after declaration merging is the enumeration
472+
// type rather than an integer type.
473+
enum E1 *eptr;
474+
[[maybe_unused]] __typeof__(x1.h) *ptr = eptr;
475+
enum E2 *eptr2;
476+
[[maybe_unused]] __typeof__(x2.h) *ptr2 = eptr2;
477+
}

0 commit comments

Comments
 (0)