[C23] AST equivalence of attributes

[AaronBallman]

Implicitly declared types (like __NSConstantString_tag, etc) will be
declared with visibility attributes. This causes problems when merging
ASTs because we currently reject declaration merging for declarations
with attributes.

This relaxes that restriction somewhat; implicit declarations can now
have attributes when merging; we assume that if the compiler generated
it, it's fine.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Implicitly declared types (like __NSConstantString_tag, etc) will be
declared with visibility attributes. This causes problems when merging
ASTs because we currently reject declaration merging for declarations
with attributes.

This relaxes that restriction somewhat; implicit declarations can now
have attributes when merging; we assume that if the compiler generated
it, it's fine.

---

Full diff: https://github.com/llvm/llvm-project/pull/151196.diff

2 Files Affected:

• (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+4-2)
• (modified) clang/test/ASTMerge/enum/test2.c (+2-4)

diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 3649dbac289ce..6100fed6b93c0 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -456,7 +456,9 @@ CheckStructurallyEquivalentAttributes(StructuralEquivalenceContext &Context,
                                       const Decl *D1, const Decl *D2,
                                       const Decl *PrimaryDecl = nullptr) {
   // If either declaration has an attribute on it, we treat the declarations
-  // as not being structurally equivalent.
+  // as not being structurally equivalent unless both declarations are implicit
+  // (ones generated by the compiler like __NSConstantString_tag).
+  //
   // FIXME: this should be handled on a case-by-case basis via tablegen in
   // Attr.td. There are multiple cases to consider: one declaration with the
   // attribute, another without it; different attribute syntax|spellings for
@@ -468,7 +470,7 @@ CheckStructurallyEquivalentAttributes(StructuralEquivalenceContext &Context,
     D1Attr = *D1->getAttrs().begin();
   if (D2->hasAttrs())
     D2Attr = *D2->getAttrs().begin();
-  if (D1Attr || D2Attr) {
+  if ((D1Attr || D2Attr) && !D1->isImplicit() && !D2->isImplicit()) {
     const auto *DiagnoseDecl = cast<TypeDecl>(PrimaryDecl ? PrimaryDecl : D2);
     Context.Diag2(DiagnoseDecl->getLocation(),
                   diag::warn_odr_tag_type_with_attributes)
diff --git a/clang/test/ASTMerge/enum/test2.c b/clang/test/ASTMerge/enum/test2.c
index 2955d8fe50b2e..bdd8b13ee4c21 100644
--- a/clang/test/ASTMerge/enum/test2.c
+++ b/clang/test/ASTMerge/enum/test2.c
@@ -1,9 +1,7 @@
 // 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
+// RUN: %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
@@ -12,5 +10,5 @@
 // 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
+// CHECK: 3 warnings generated
 

[AaronBallman]

Not going to bother with the cherry-pick command here because I already know there will be merge conflicts. I'll link the backport PR once it's up.

[AaronBallman]

/cherry-pick 30a5d56

[llvmbot]

/pull-request #152149