[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `clang-m68k-linux-cross` running on `suse-gary-m68k-cross` while building `clang` at step 5 "ninja check 1". Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/21601 Here is the relevant piece of the build log for the reference ``` Step 5 (ninja check 1) failure: stage 1 checked (failure) ... [33/394] Building CXX object tools/clang/tools/extra/unittests/clang-move/CMakeFiles/ClangMoveTests.dir/ClangMoveTests.cpp.o [34/394] Building CXX object tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/RecordTest.cpp.o [35/394] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Parse/ParseHLSLRootSignatureTest.cpp.o [36/394] Building CXX object tools/clang/tools/extra/unittests/clang-tidy/CMakeFiles/ClangTidyTests.dir/IncludeCleanerTest.cpp.o [37/394] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Driver/ToolChainTest.cpp.o [38/394] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Lex/PPCallbacksTest.cpp.o [39/394] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/DependencyScanning/DependencyScanningWorkerTest.cpp.o [40/394] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CloneDetectionTest.cpp.o [41/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ASTSignalsTests.cpp.o [42/394] Building CXX object tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o FAILED: tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o /usr/bin/c++ -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/tools/extra/include-cleaner/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/include-cleaner/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/include-cleaner/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/include-cleaner/unittests/../lib -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-dangling-reference -Wno-redundant-move -Wno-pessimizing-move -Wno-array-bounds -Wno-stringop-overread -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -std=c++17 -UNDEBUG -Wno-variadic-macros -fno-exceptions -funwind-tables -fno-rtti -Wno-suggest-override -MD -MT tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o -MF tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o.d -o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o -c /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp c++: fatal error: Killed signal terminated program cc1plus compilation terminated. [43/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CollectMacrosTests.cpp.o [44/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CallHierarchyTests.cpp.o [45/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompletionStringsTests.cpp.o [46/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ASTTests.cpp.o [47/394] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ClangdLSPServerT
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/AaronBallman approved this pull request. Thanks @ChuanqiXu9 for the background! Because this code isn't used by modules for C++ and it's a good improvement for C, I think it's ready to land as-is and we can think about how to reuse the attribute logic later. https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
ChuanqiXu9 wrote: > I thought that modules was using the AST structural equivalence code as well No, as far as I know. In modules, it looks like only ObjC uses AST structural equivalence. In C++, we use the previously mentioned ODRHash mechanism to judge if two AST Definition are the same. > if you confirm, do you think we should do the C++ changes as well, either in > this PR or a follow-up? Or is there a better way for us to structure the code > for sharing? I guess a reason why we use ODRHash is that, we want to avoid deserializing a declaration if possible. That is, for AST structural equivalence, we need the full AST structure. But in serialization, if we're comparing the identification of two classes (which may not be deserialized fully yet), it will be much better if we're able to compare the unsigned int directly. This may be the reason why we have ODRHash. This is historical. CC @zygoloid if you know the reason modules code use ODRHash instead of AST structural equivalence in the first place. https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
AaronBallman wrote:
> > > > > I concluded that the local queue introduced in the previous commits
> > > > > isn’t necessary in C mode. Since attribute equivalence is currently
> > > > > checked only for C23, I removed the code entirely.
> > > > > If we later enable attribute equivalence checking in C++ mode, we may
> > > > > need to revisit this and adopt a similar approach. For example,
> > > > > without a separate queue, clang will diagnose the attribute but fail
> > > > > to report the mismatch in the type of member `a` when attempting to
> > > > > merge the following struct:
> > > > > ```
> > > > > struct GuardedBy {
> > > > > int b __attribute__((guarded_by(lock)));
> > > > > // The pair of S is added to NonEquivalentDecls, but the pair of
> > > > > Lock isn't.
> > > > > struct __attribute__ ((lockable)) Lock {
> > > > > struct S {
> > > > > #ifdef D1
> > > > > unsigned a;
> > > > > #else
> > > > > int a;
> > > > > #endif
> > > > > } s;
> > > > > } lock;
> > > > > };
> > > > > ```
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > $ clang -cc1 -std=c++20 -emit-pch -o f1.ast -x c++ test.h $ clang
> > > > > -cc1 -std=c++20 -emit-pch -o f2.ast -DD1 -x c++ test.h $ clang -cc1
> > > > > -std=c++20 -ast-merge f1.ast -ast-merge f2.ast -x c++ test.cpp
> > > >
> > > >
> > > > CC @ChuanqiXu9 @Bigcheese as C++ modules maintainers. I am under the
> > > > impression we need this for both C++ modules support and C23's
> > > > redefinition checking. If we don't need the more complicated approach
> > > > right now, I think the current approach is reasonable for C (but CC
> > > > @erichkeane for awareness of the tablegen changes), I just want to make
> > > > sure we're not making further work harder for ourselves.
> > >
> > >
> > > In modules (Serialization), we already have ODR checks to check the
> > > redefinitions are the same. So this is not needed for modules.
> >
> >
> > Can you point me to the code that's handling checking attribute
> > compatibility?
>
> Oh, you're talking about attribute. I was thought you're talking about the
> compatibility of struct fields (according to the code example you mentioned).
>
> For attribute, now we don't check the compatibility. We can have a
> declaration chain with redeclarations have different attributes now.
That's what this PR is doing, but it's currently only doing it for C and not
for C++. Because we need the facilities for both C and C++ and the logic should
be basically identical between the languages (I don't know of any attributes
supported in both languages which impact layout or semantics in one language
but do not in the other). I thought that modules was using the AST structural
equivalence code as well; if you confirm, do you think we should do the C++
changes as well, either in this PR or a follow-up? Or is there a better way for
us to structure the code for sharing?
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
ChuanqiXu9 wrote:
> > I concluded that the local queue introduced in the previous commits isn’t
> > necessary in C mode. Since attribute equivalence is currently checked only
> > for C23, I removed the code entirely.
> > If we later enable attribute equivalence checking in C++ mode, we may need
> > to revisit this and adopt a similar approach. For example, without a
> > separate queue, clang will diagnose the attribute but fail to report the
> > mismatch in the type of member `a` when attempting to merge the following
> > struct:
> > ```
> > struct GuardedBy {
> > int b __attribute__((guarded_by(lock)));
> > // The pair of S is added to NonEquivalentDecls, but the pair of Lock
> > isn't.
> > struct __attribute__ ((lockable)) Lock {
> > struct S {
> > #ifdef D1
> > unsigned a;
> > #else
> > int a;
> > #endif
> > } s;
> > } lock;
> > };
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > $ clang -cc1 -std=c++20 -emit-pch -o f1.ast -x c++ test.h $ clang -cc1
> > -std=c++20 -emit-pch -o f2.ast -DD1 -x c++ test.h $ clang -cc1 -std=c++20
> > -ast-merge f1.ast -ast-merge f2.ast -x c++ test.cpp
>
> CC @ChuanqiXu9 @Bigcheese as C++ modules maintainers. I am under the
> impression we need this for both C++ modules support and C23's redefinition
> checking. If we don't need the more complicated approach right now, I think
> the current approach is reasonable for C (but CC @erichkeane for awareness of
> the tablegen changes), I just want to make sure we're not making further work
> harder for ourselves.
In modules (Serialization), we already have ODR checks to check the
redefinitions are the same. So this is not needed for modules.
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
AaronBallman wrote:
> I concluded that the local queue introduced in the previous commits isn’t
> necessary in C mode. Since attribute equivalence is currently checked only
> for C23, I removed the code entirely.
>
> If we later enable attribute equivalence checking in C++ mode, we may need to
> revisit this and adopt a similar approach. For example, without a separate
> queue, clang will diagnose the attribute but fail to report the mismatch in
> the type of member `a` when attempting to merge the following struct:
>
> ```
> struct GuardedBy {
> int b __attribute__((guarded_by(lock)));
> // The pair of S is added to NonEquivalentDecls, but the pair of Lock
> isn't.
> struct __attribute__ ((lockable)) Lock {
> struct S {
> #ifdef D1
> unsigned a;
> #else
> int a;
> #endif
> } s;
> } lock;
> };
> ```
>
> $ clang -cc1 -std=c++20 -emit-pch -o f1.ast -x c++ test.h $ clang -cc1
> -std=c++20 -emit-pch -o f2.ast -DD1 -x c++ test.h $ clang -cc1 -std=c++20
> -ast-merge f1.ast -ast-merge f2.ast -x c++ test.cpp
CC @ChuanqiXu9 @Bigcheese as C++ modules maintainers. I am under the impression
we need this for both C++ modules support and C23's redefinition checking. If
we don't need the more complicated approach right now, I think the current
approach is reasonable for C (but CC @erichkeane for awareness of the tablegen
changes), I just want to make sure we're not making further work harder for
ourselves.
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/ahatanak updated
https://github.com/llvm/llvm-project/pull/168769
>From 516b90f080fe28c93e9127686773d3f72c96517b Mon Sep 17 00:00:00 2001
From: Akira Hatanaka
Date: Wed, 19 Nov 2025 12:09:26 -0800
Subject: [PATCH 1/8] [AST] Support structural equivalence checking of
attributes on Decls
The structural equivalence checker currently treats any explicit
attributes on a declaration as a reason to consider the declarations
non-equivalent in C23 mode, even when both declarations carry the same
attributes. This is unnecessarily strict and causes two otherwise
equivalent declarations to be rejected just because they carry
explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent
and appear in the same order. The initial implementation adds support
for three attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also
allows these utilities to be generated automatically by tablegen in
the future.
Inherited attributes that are supported are ignored when determining
structural equivalence, because inherited attributes should not affect
whether two definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes
so that attribute comparison is performed between two definitions of
record decls rather than between a declaration and a definition.
rdar://163304242
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 146 ++---
clang/test/C/C23/n3037.c | 87
2 files changed, 217 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
+}
+
+/// Collects all supported, non-inherited attributes from the given decl.
+/// Returns true on success. If the decl contains any unsupported attribute,
+/// returns false and sets UnsupportedAttr to point to that attribute.
+static bool collectComparableAttrs(const Decl *D, AttrSet &Attrs,
+ const Attr *&UnsupportedAttr) {
+ for (const Attr *A : D->attrs()) {
+switch (A->getKind()) {
+case attr::Availability:
+case attr::EnumExtensibility:
+case attr::Unused:
+ if (!A->isInherited())
+Attrs.insert(A);
+ break;
+
+default:
+ UnsupportedAttr
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/ahatanak updated
https://github.com/llvm/llvm-project/pull/168769
>From 516b90f080fe28c93e9127686773d3f72c96517b Mon Sep 17 00:00:00 2001
From: Akira Hatanaka
Date: Wed, 19 Nov 2025 12:09:26 -0800
Subject: [PATCH 1/7] [AST] Support structural equivalence checking of
attributes on Decls
The structural equivalence checker currently treats any explicit
attributes on a declaration as a reason to consider the declarations
non-equivalent in C23 mode, even when both declarations carry the same
attributes. This is unnecessarily strict and causes two otherwise
equivalent declarations to be rejected just because they carry
explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent
and appear in the same order. The initial implementation adds support
for three attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also
allows these utilities to be generated automatically by tablegen in
the future.
Inherited attributes that are supported are ignored when determining
structural equivalence, because inherited attributes should not affect
whether two definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes
so that attribute comparison is performed between two definitions of
record decls rather than between a declaration and a definition.
rdar://163304242
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 146 ++---
clang/test/C/C23/n3037.c | 87
2 files changed, 217 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
+}
+
+/// Collects all supported, non-inherited attributes from the given decl.
+/// Returns true on success. If the decl contains any unsupported attribute,
+/// returns false and sets UnsupportedAttr to point to that attribute.
+static bool collectComparableAttrs(const Decl *D, AttrSet &Attrs,
+ const Attr *&UnsupportedAttr) {
+ for (const Attr *A : D->attrs()) {
+switch (A->getKind()) {
+case attr::Availability:
+case attr::EnumExtensibility:
+case attr::Unused:
+ if (!A->isInherited())
+Attrs.insert(A);
+ break;
+
+default:
+ UnsupportedAttr
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/ahatanak updated
https://github.com/llvm/llvm-project/pull/168769
>From 516b90f080fe28c93e9127686773d3f72c96517b Mon Sep 17 00:00:00 2001
From: Akira Hatanaka
Date: Wed, 19 Nov 2025 12:09:26 -0800
Subject: [PATCH 1/6] [AST] Support structural equivalence checking of
attributes on Decls
The structural equivalence checker currently treats any explicit
attributes on a declaration as a reason to consider the declarations
non-equivalent in C23 mode, even when both declarations carry the same
attributes. This is unnecessarily strict and causes two otherwise
equivalent declarations to be rejected just because they carry
explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent
and appear in the same order. The initial implementation adds support
for three attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also
allows these utilities to be generated automatically by tablegen in
the future.
Inherited attributes that are supported are ignored when determining
structural equivalence, because inherited attributes should not affect
whether two definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes
so that attribute comparison is performed between two definitions of
record decls rather than between a declaration and a definition.
rdar://163304242
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 146 ++---
clang/test/C/C23/n3037.c | 87
2 files changed, 217 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
+}
+
+/// Collects all supported, non-inherited attributes from the given decl.
+/// Returns true on success. If the decl contains any unsupported attribute,
+/// returns false and sets UnsupportedAttr to point to that attribute.
+static bool collectComparableAttrs(const Decl *D, AttrSet &Attrs,
+ const Attr *&UnsupportedAttr) {
+ for (const Attr *A : D->attrs()) {
+switch (A->getKind()) {
+case attr::Availability:
+case attr::EnumExtensibility:
+case attr::Unused:
+ if (!A->isInherited())
+Attrs.insert(A);
+ break;
+
+default:
+ UnsupportedAttr
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
ahatanak wrote: https://github.com/llvm/llvm-project/pull/173266 is an NFC patch that constifies IdentifierInfo. I can merge that first if that makes reviewing this PR easier. https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff origin/main HEAD --extensions h,c,cpp --
clang/include/clang/AST/ASTStructuralEquivalence.h
clang/include/clang/AST/Attr.h clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTStructuralEquivalence.cpp clang/lib/AST/AttrImpl.cpp
clang/lib/AST/DeclBase.cpp clang/lib/Index/CommentToXML.cpp
clang/lib/Sema/SemaAvailability.cpp clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExprObjC.cpp clang/lib/Sema/SemaHLSL.cpp
clang/lib/Sema/SemaObjC.cpp clang/test/C/C23/n3037.c
clang/utils/TableGen/ClangAttrEmitter.cpp --diff_from_common_commit
``
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
View the diff from clang-format here.
``diff
diff --git a/clang/include/clang/AST/ASTStructuralEquivalence.h
b/clang/include/clang/AST/ASTStructuralEquivalence.h
index 8fa21f8a2..2b56273f2 100644
--- a/clang/include/clang/AST/ASTStructuralEquivalence.h
+++ b/clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -171,7 +171,6 @@ struct StructuralEquivalenceContext {
bool checkDeclQueue();
private:
-
/// Finish checking all of the structural equivalences.
///
/// \returns true if the equivalence check failed (non-equivalence detected),
@@ -197,7 +196,7 @@ bool isEquivalent(StructuralEquivalenceContext &Context,
QualType T1,
bool isEquivalent(StructuralEquivalenceContext &Context, const Stmt *S1,
const Stmt *S2);
bool isEquivalent(const IdentifierInfo *Name1, const IdentifierInfo *Name2);
-}
+} // namespace ASTStructuralEquivalence
} // namespace clang
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 36c0b2b1c..61fb40e79 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1402,8 +1402,7 @@ namespace {
class VariadicIdentifierArgument : public VariadicArgument {
public:
VariadicIdentifierArgument(const Record &Arg, StringRef Attr)
- : VariadicArgument(Arg, Attr, "const IdentifierInfo *")
-{}
+: VariadicArgument(Arg, Attr, "const IdentifierInfo *") {}
};
class VariadicStringArgument : public VariadicArgument {
``
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/ahatanak updated
https://github.com/llvm/llvm-project/pull/168769
>From 516b90f080fe28c93e9127686773d3f72c96517b Mon Sep 17 00:00:00 2001
From: Akira Hatanaka
Date: Wed, 19 Nov 2025 12:09:26 -0800
Subject: [PATCH 1/5] [AST] Support structural equivalence checking of
attributes on Decls
The structural equivalence checker currently treats any explicit
attributes on a declaration as a reason to consider the declarations
non-equivalent in C23 mode, even when both declarations carry the same
attributes. This is unnecessarily strict and causes two otherwise
equivalent declarations to be rejected just because they carry
explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent
and appear in the same order. The initial implementation adds support
for three attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also
allows these utilities to be generated automatically by tablegen in
the future.
Inherited attributes that are supported are ignored when determining
structural equivalence, because inherited attributes should not affect
whether two definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes
so that attribute comparison is performed between two definitions of
record decls rather than between a declaration and a definition.
rdar://163304242
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 146 ++---
clang/test/C/C23/n3037.c | 87
2 files changed, 217 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
+}
+
+/// Collects all supported, non-inherited attributes from the given decl.
+/// Returns true on success. If the decl contains any unsupported attribute,
+/// returns false and sets UnsupportedAttr to point to that attribute.
+static bool collectComparableAttrs(const Decl *D, AttrSet &Attrs,
+ const Attr *&UnsupportedAttr) {
+ for (const Attr *A : D->attrs()) {
+switch (A->getKind()) {
+case attr::Availability:
+case attr::EnumExtensibility:
+case attr::Unused:
+ if (!A->isInherited())
+Attrs.insert(A);
+ break;
+
+default:
+ UnsupportedAttr
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
@@ -451,6 +452,119 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
AaronBallman wrote:
Do we want to require the spellings to be the same? e.g., `[[maybe_unused]]` vs
`__attribute__((unused))`
I think in general we have to because we have attributes with multiple
spellings but the same kinds. e.g.,
```
def Ownership : InheritableAttr {
let Spellings = [Clang<"ownership_holds">, Clang<"ownership_returns">,
Clang<"ownership_takes">];
let Accessors = [Accessor<"isHolds", [Clang<"ownership_holds">]>,
Accessor<"isReturns", [Clang<"ownership_returns">]>,
Accessor<"isTakes", [Clang<"ownership_takes">]>];
```
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/AaronBallman commented: Thank you for working on this, and sorry for the delayed review (holiday schedules are hard). In general, I think we should be doing this with tablegen from the start. With the current patch, we have to update a few switch statements for the attribute to be checked properly and I'm worried people will just continue to do ad hoc solutions here instead of doing the tablegen work, especially as the list grows longer (it's easier to do the tablegen work when there's 3 attributes than when there's 40; and we have ~550 attributes in total so we know the ad hoc solution won't scale). And when I think about the fact that a lot of this is keyed off the attribute *kind* rather than the spelling, I get worried that the current approach isn't really the direction we should be going. The trouble is: * Same attribute kind doesn't mean same attribute; we have attributes with a single semantic `Attr` subclass but multiple spellings that mean different things. * We also have the situation where attributes share the same spelling but are different semantic attributes. I think this situation is okay though because those attributes should always be target-specific and I think structural equivalence across targets is handled elsewhere anyway. * Different attribute arguments don't mean the two declarations aren't structurally equivalent. e.g., `[[deprecated]]` with different messages aren't *structurally* different, are they? But even with availability as in this PR, is a version of `10` structurally different from `10.0` different from `10.0.0`? There's also `AvailabilityAttr::equivalentPlatformNames()`, etc. Would you be amenable to doing the tablegen approach instead? I was thinking of a design like: * If there's no other marking in the attribute definition, then a strict equivalence is checked (same kind, same spelling, identical arguments (so `0` and `0 + 0` are structurally different). * Otherwise, there can be a custom comparator implemented explicitly via a `code` field with a signature along the lines of `bool (const T *, const T *)`. But the design is flexible if you've got other ideas, too. https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
@@ -451,6 +452,119 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
AaronBallman wrote:
Would a `SmallPtrSet` make more sense?
https://github.com/llvm/llvm-project/pull/168769
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
ahatanak wrote: Any comments? https://github.com/llvm/llvm-project/pull/168769 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
https://github.com/ahatanak created
https://github.com/llvm/llvm-project/pull/168769
The structural equivalence checker currently treats any explicit attributes on
a declaration as a reason to consider the declarations non-equivalent in C23
mode, even when both declarations carry the same attributes. This is
unnecessarily strict and causes two otherwise equivalent declarations to be
rejected just because they carry explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent and
appear in the same order. The initial implementation adds support for three
attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also allows
these utilities to be generated automatically by tablegen in the future.
Inherited attributes that are supported are ignored when determining structural
equivalence, because inherited attributes should not affect whether two
definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes so that
attribute comparison is performed between two definitions of record decls
rather than between a declaration and a definition.
rdar://163304242
>From 516b90f080fe28c93e9127686773d3f72c96517b Mon Sep 17 00:00:00 2001
From: Akira Hatanaka
Date: Wed, 19 Nov 2025 12:09:26 -0800
Subject: [PATCH] [AST] Support structural equivalence checking of attributes
on Decls
The structural equivalence checker currently treats any explicit
attributes on a declaration as a reason to consider the declarations
non-equivalent in C23 mode, even when both declarations carry the same
attributes. This is unnecessarily strict and causes two otherwise
equivalent declarations to be rejected just because they carry
explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent
and appear in the same order. The initial implementation adds support
for three attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also
allows these utilities to be generated automatically by tablegen in
the future.
Inherited attributes that are supported are ignored when determining
structural equivalence, because inherited attributes should not affect
whether two definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes
so that attribute comparison is performed between two definitions of
record decls rather than between a declaration and a definition.
rdar://163304242
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 146 ++---
clang/test/C/C23/n3037.c | 87
2 files changed, 217 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparison
[clang] [AST] Support structural equivalence checking of attributes on Decls (PR #168769)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Akira Hatanaka (ahatanak)
Changes
The structural equivalence checker currently treats any explicit attributes on
a declaration as a reason to consider the declarations non-equivalent in C23
mode, even when both declarations carry the same attributes. This is
unnecessarily strict and causes two otherwise equivalent declarations to be
rejected just because they carry explicitly annotated attributes.
This patch enables structural equivalence checking to accept selected
attributes as long as the attributes on both definitions are equivalent and
appear in the same order. The initial implementation adds support for three
attributes: Availability, EnumExtensibility, and Unused.
Additional attribute kinds can be added incrementally. This design also allows
these utilities to be generated automatically by tablegen in the future.
Inherited attributes that are supported are ignored when determining structural
equivalence, because inherited attributes should not affect whether two
definitions are structurally compatible.
This patch also moves the call to CheckStructurallyEquivalentAttributes so that
attribute comparison is performed between two definitions of record decls
rather than between a declaration and a definition.
rdar://163304242
---
Full diff: https://github.com/llvm/llvm-project/pull/168769.diff
2 Files Affected:
- (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+130-16)
- (modified) clang/test/C/C23/n3037.c (+87)
``diff
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp
b/clang/lib/AST/ASTStructuralEquivalence.cpp
index da64c92221837..fc7f44c6ba563 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include
#include
+#include
#include
using namespace clang;
@@ -451,6 +452,123 @@ class StmtComparer {
};
} // namespace
+namespace {
+enum class AttrComparisonKind { Equal, NotEqual };
+
+/// Represents the result of comparing the attribute sets on two decls. If the
+/// sets are incompatible, A1/A2 point to the offending attributes.
+struct AttrComparisonResult {
+ AttrComparisonKind Kind = AttrComparisonKind::Equal;
+ const Attr *A1 = nullptr, *A2 = nullptr;
+};
+} // namespace
+
+static AttrComparisonResult
+areAvailabilityAttrsEqual(const AvailabilityAttr *A1,
+ const AvailabilityAttr *A2) {
+ if (A1->getPlatform() == A2->getPlatform() &&
+ A1->getIntroduced() == A2->getIntroduced() &&
+ A1->getDeprecated() == A2->getDeprecated() &&
+ A1->getObsoleted() == A2->getObsoleted() &&
+ A1->getUnavailable() == A2->getUnavailable() &&
+ A1->getMessage() == A2->getMessage() &&
+ A1->getReplacement() == A2->getReplacement() &&
+ A1->getStrict() == A2->getStrict() &&
+ A1->getPriority() == A2->getPriority() &&
+ A1->getEnvironment() == A2->getEnvironment())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult
+areEnumExtensibilityAttrsEqual(const EnumExtensibilityAttr *A1,
+ const EnumExtensibilityAttr *A2) {
+ if (A1->getExtensibility() == A2->getExtensibility())
+return {AttrComparisonKind::Equal};
+ return {AttrComparisonKind::NotEqual, A1, A2};
+}
+
+static AttrComparisonResult areAttrsEqual(const Attr *A1, const Attr *A2) {
+ auto Kind1 = A1->getKind(), Kind2 = A2->getKind();
+ if (Kind1 != Kind2)
+return {AttrComparisonKind::NotEqual, A1, A2};
+
+ switch (Kind1) {
+ case attr::Availability:
+return areAvailabilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::EnumExtensibility:
+return areEnumExtensibilityAttrsEqual(cast(A1),
+ cast(A2));
+ case attr::Unused:
+return {AttrComparisonKind::Equal};
+ default:
+llvm_unreachable("unexpected attr kind");
+ }
+}
+
+static bool compareAttrKind(const Attr *A1, const Attr *A2) {
+ return A1->getKind() < A2->getKind();
+}
+
+namespace {
+using AttrSet = std::multiset;
+}
+
+/// Collects all supported, non-inherited attributes from the given decl.
+/// Returns true on success. If the decl contains any unsupported attribute,
+/// returns false and sets UnsupportedAttr to point to that attribute.
+static bool collectComparableAttrs(const Decl *D, AttrSet &Attrs,
+ const Attr *&UnsupportedAttr) {
+ for (const Attr *A : D->attrs()) {
+switch (A->getKind()) {
+case attr::Availability:
+case attr::EnumExtensibility:
+case attr::Unused:
+ if (!A->isInherited())
+Attrs.insert(A);
+ break;
+
+default:
+ UnsupportedAttr = A;
+ return false; // unsupported attribute
+}
+ }
+
+ return true;
+}
+
+/// Determines whether D1 and D2 have compatible sets of attributes for the
+/// purposes of
