llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) <details> <summary>Changes</summary> The structural equivalence checker was not paying attention to whether enumerations had compatible fixed underlying types or not. Fixes #<!-- -->150594 --- Full diff: https://github.com/llvm/llvm-project/pull/151199.diff 6 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticASTKinds.td (+8) - (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+42) - (added) clang/test/ASTMerge/enum/Inputs/enum3.c (+14) - (added) clang/test/ASTMerge/enum/Inputs/enum4.c (+14) - (added) clang/test/ASTMerge/enum/test2.c (+16) - (modified) clang/test/C/C23/n3037.c (+74) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index a67b9995d3b54..4c7219c78c8bc 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -507,6 +507,14 @@ def note_odr_number_of_bases : Note< "class has %0 base %plural{1:class|:classes}0">; def note_odr_enumerator : Note<"enumerator %0 with value %1 here">; def note_odr_missing_enumerator : Note<"no corresponding enumerator here">; +def note_odr_incompatible_fixed_underlying_type : Note< + "enumeration %0 declared with incompatible fixed underlying types (%1 vs. " + "%2)">; +def note_odr_fixed_underlying_type : Note< + "enumeration %0 has fixed underlying type here">; +def note_odr_missing_fixed_underlying_type : Note< + "enumeration %0 missing fixed underlying type here">; + def err_odr_field_type_inconsistent : Error< "field %0 declared with incompatible types in different " "translation units (%1 vs. %2)">; diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp index 3aa6b37844103..e26b3eaa81969 100644 --- a/clang/lib/AST/ASTStructuralEquivalence.cpp +++ b/clang/lib/AST/ASTStructuralEquivalence.cpp @@ -2067,6 +2067,48 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, !CheckStructurallyEquivalentAttributes(Context, D1, D2)) return false; + // In C23, if one enumeration has a fixed underlying type, the other shall + // have a compatible fixed underlying type (6.2.7). + if (Context.LangOpts.C23) { + if (D1->isFixed() != D2->isFixed()) { + if (Context.Complain) { + Context.Diag2(D2->getLocation(), + Context.getApplicableDiagnostic( + diag::err_odr_tag_type_inconsistent)) + << Context.ToCtx.getTypeDeclType(D2) + << (&Context.FromCtx != &Context.ToCtx); + Context.Diag1(D1->getLocation(), + D1->isFixed() + ? diag::note_odr_fixed_underlying_type + : diag::note_odr_missing_fixed_underlying_type) + << D1; + Context.Diag2(D2->getLocation(), + D2->isFixed() + ? diag::note_odr_fixed_underlying_type + : diag::note_odr_missing_fixed_underlying_type) + << D2; + } + return false; + } + if (D1->isFixed()) { + assert(D2->isFixed() && "enums expected to have fixed underlying types"); + if (!IsStructurallyEquivalent(Context, D1->getIntegerType(), + D2->getIntegerType())) { + if (Context.Complain) { + Context.Diag2(D2->getLocation(), + Context.getApplicableDiagnostic( + diag::err_odr_tag_type_inconsistent)) + << Context.ToCtx.getTypeDeclType(D2) + << (&Context.FromCtx != &Context.ToCtx); + Context.Diag2(D2->getLocation(), + diag::note_odr_incompatible_fixed_underlying_type) + << D2 << D2->getIntegerType() << D1->getIntegerType(); + } + return false; + } + } + } + llvm::SmallVector<const EnumConstantDecl *, 8> D1Enums, D2Enums; auto CopyEnumerators = [](auto &&Range, llvm::SmallVectorImpl<const EnumConstantDecl *> &Cont) { diff --git a/clang/test/ASTMerge/enum/Inputs/enum3.c b/clang/test/ASTMerge/enum/Inputs/enum3.c new file mode 100644 index 0000000000000..32ad5366434ac --- /dev/null +++ b/clang/test/ASTMerge/enum/Inputs/enum3.c @@ -0,0 +1,14 @@ +// [C23] missing underlying types +enum E1 : int { + E1Enumerator1 +}; + +enum E2 { + E2Enumerator1 +}; + +// [C23] Incompatible underlying types +enum E3 : long { + E3Enumerator1 +}; + diff --git a/clang/test/ASTMerge/enum/Inputs/enum4.c b/clang/test/ASTMerge/enum/Inputs/enum4.c new file mode 100644 index 0000000000000..15f5c603c7abb --- /dev/null +++ b/clang/test/ASTMerge/enum/Inputs/enum4.c @@ -0,0 +1,14 @@ +// [C23] missing underlying types +enum E1 { + E1Enumerator1 +}; + +enum E2 : int { + E2Enumerator1 +}; + +// [C23] Incompatible underlying types +enum E3 : short { + E3Enumerator1 +}; + diff --git a/clang/test/ASTMerge/enum/test2.c b/clang/test/ASTMerge/enum/test2.c new file mode 100644 index 0000000000000..2955d8fe50b2e --- /dev/null +++ b/clang/test/ASTMerge/enum/test2.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -std=c23 -emit-pch -o %t.1.ast %S/Inputs/enum3.c +// RUN: %clang_cc1 -std=c23 -emit-pch -o %t.2.ast %S/Inputs/enum4.c +// RUN: not %clang_cc1 -std=c23 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s + +// FIXME: this error should not happen! +// CHECK: error: type 'struct __NSConstantString_tag' has an attribute which currently causes the types to be treated as though they are incompatible +// CHECK: enum3.c:2:6: warning: type 'enum E1' has incompatible definitions in different translation units +// CHECK: enum4.c:2:6: note: enumeration 'E1' missing fixed underlying type here +// CHECK: enum3.c:2:6: note: enumeration 'E1' has fixed underlying type here +// CHECK: enum3.c:6:6: warning: type 'enum E2' has incompatible definitions in different translation units +// CHECK: enum4.c:6:6: note: enumeration 'E2' has fixed underlying type here +// CHECK: enum3.c:6:6: note: enumeration 'E2' missing fixed underlying type here +// CHECK: enum3.c:11:6: warning: type 'enum E3' has incompatible definitions in different translation units +// CHECK: enum3.c:11:6: note: enumeration 'E3' declared with incompatible fixed underlying types ('long' vs. 'short') +// CHECK: 3 warnings and 1 error generated + diff --git a/clang/test/C/C23/n3037.c b/clang/test/C/C23/n3037.c index ce6f4c4ea7acf..3748375692430 100644 --- a/clang/test/C/C23/n3037.c +++ b/clang/test/C/C23/n3037.c @@ -401,3 +401,77 @@ _Static_assert(0 == _Generic(inner_anon_tagged.untagged, struct { int i; } : 1, // unions and structures are both RecordDecl objects, whereas EnumDecl is not). enum { E_Untagged1 } nontag_enum; // both-note {{previous definition is here}} _Static_assert(0 == _Generic(nontag_enum, enum { E_Untagged1 } : 1, default : 0)); // both-error {{redefinition of enumerator 'E_Untagged1'}} + +// Test that enumerations with mixed underlying types are properly handled. +enum GH150594_E1 : int { GH150594_Val1 }; +enum GH150594_E2 : int { GH150594_Val2 }; +enum GH150594_E3 { GH150594_Val3 }; +enum GH150594_E4 : int { GH150594_Val4 }; +void GH150594(void) { + extern enum GH150594_E1 Fn1(void); // both-note {{previous declaration is here}} + extern enum GH150594_E2 Fn2(void); // c17-note {{previous declaration is here}} + extern enum GH150594_E3 Fn3(void); // both-note {{previous declaration is here}} + extern enum GH150594_E4 Fn4(void); // both-note {{previous declaration is here}} + enum GH150594_E1 { GH150594_Val1 }; + enum GH150594_E2 : int { GH150594_Val2 }; + enum GH150594_E3 : int { GH150594_Val3 }; + enum GH150594_E4 : short { GH150594_Val4 }; + extern enum GH150594_E1 Fn1(void); // both-error {{conflicting types for 'Fn1'}} + extern enum GH150594_E2 Fn2(void); // c17-error {{conflicting types for 'Fn2'}} + extern enum GH150594_E3 Fn3(void); // both-error {{conflicting types for 'Fn3'}} + extern enum GH150594_E4 Fn4(void); // both-error {{conflicting types for 'Fn4'}} + + // Show that two declarations in the same scope give expected diagnostics. + enum E1 { e1 }; // both-note {{previous declaration is here}} + enum E1 : int { e1 }; // both-error {{enumeration previously declared with nonfixed underlying type}} + + enum E2 : int { e2 }; // both-note {{previous declaration is here}} + enum E2 { e2 }; // both-error {{enumeration previously declared with fixed underlying type}} + + enum E3 : int { e3 }; // both-note {{previous declaration is here}} + enum E3 : short { e3 }; // both-error {{enumeration redeclared with different underlying type 'short' (was 'int')}} + + typedef short foo; + enum E4 : foo { e4 }; // c17-note 2 {{previous definition is here}} + enum E4 : short { e4 }; // c17-error {{redefinition of 'E4'}} \ + c17-error {{redefinition of enumerator 'e4'}} + + enum E5 : foo { e5 }; // both-note {{previous declaration is here}} + enum E5 : int { e5 }; // both-error {{enumeration redeclared with different underlying type 'int' (was 'foo' (aka 'short'))}} +} + +// Test that enumerations are compatible with their underlying type, but still +// diagnose when "same type" is required rather than merely "compatible type". +enum E1 : int { e1 }; // Fixed underlying type +enum E2 { e2 }; // Unfixed underlying type, defaults to int or unsigned int + +struct GH149965_1 { int h; }; +// This typeof trick is used to get the underlying type of the enumeration in a +// platform agnostic way. +struct GH149965_2 { __typeof__(+(enum E2){}) 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}} + + // 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'}} + + // For Clang, the composite type after declaration merging is the enumeration + // type rather than an integer type. + enum E1 *eptr; + [[maybe_unused]] __typeof__(x1.h) *ptr = eptr; + enum E2 *eptr2; + [[maybe_unused]] __typeof__(x2.h) *ptr2 = eptr2; +} `````````` </details> https://github.com/llvm/llvm-project/pull/151199 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
