Skip to content

[AST] Support structural equivalence checking of attributes on Decls #168769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ahatanak wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[AST] Support structural equivalence checking of attributes on Decls #168769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
516b90f
[AST] Support structural equivalence checking of attributes on Decls
ahatanak Nov 19, 2025
669be8d
Simplify function and rename confusing variables
ahatanak Nov 20, 2025
b9e91d3
Don’t use else after a return
ahatanak Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 126 additions & 16 deletions clang/lib/AST/ASTStructuralEquivalence.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
#include <optional>
#include <set>
#include <utility>

using namespace clang;
Expand Down Expand Up @@ -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<AvailabilityAttr>(A1),
cast<AvailabilityAttr>(A2));
case attr::EnumExtensibility:
return areEnumExtensibilityAttrsEqual(cast<EnumExtensibilityAttr>(A1),
cast<EnumExtensibilityAttr>(A2));
case attr::Unused:
return {AttrComparisonKind::Equal};
Copy link
Collaborator

@AaronBallman AaronBallman Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">]>];

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<const Attr *, decltype(&compareAttrKind)>;
Copy link
Collaborator

@AaronBallman AaronBallman Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would a SmallPtrSet make more sense?

}

/// Collects all supported, non-inherited attributes from the given decl.
/// If the decl doesn't contain any unsupported attributes, returns a nullptr,
/// otherwise returns the first unsupported attribute.
static const Attr *collectComparableAttrs(const Decl *D, AttrSet &Attrs) {
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:
return A; // unsupported attribute
}
}

return nullptr;
}

/// Determines whether D1 and D2 have compatible sets of attributes for the
/// purposes of structural equivalence checking.
static AttrComparisonResult areDeclAttrsEquivalent(const Decl *D1,
const Decl *D2) {
if (D1->isImplicit() || D2->isImplicit())
return {AttrComparisonKind::Equal};

AttrSet A1(&compareAttrKind), A2(&compareAttrKind);

const Attr *UnsupportedAttr1 = collectComparableAttrs(D1, A1);
const Attr *UnsupportedAttr2 = collectComparableAttrs(D2, A2);

if (UnsupportedAttr1 || UnsupportedAttr2)
return {AttrComparisonKind::NotEqual, UnsupportedAttr1, UnsupportedAttr2};

auto I1 = A1.begin(), E1 = A1.end(), I2 = A2.begin(), E2 = A2.end();
for (; I1 != E1 && I2 != E2; ++I1, ++I2) {
AttrComparisonResult R = areAttrsEqual(*I1, *I2);
if (R.Kind != AttrComparisonKind::Equal)
return R;
}

if (I1 != E1)
return {AttrComparisonKind::NotEqual, *I1};
if (I2 != E2)
return {AttrComparisonKind::NotEqual, nullptr, *I2};

return {AttrComparisonKind::Equal};
}

static bool
CheckStructurallyEquivalentAttributes(StructuralEquivalenceContext &Context,
const Decl *D1, const Decl *D2,
Expand All @@ -465,21 +579,17 @@ CheckStructurallyEquivalentAttributes(StructuralEquivalenceContext &Context,
// the same semantic attribute, differences in attribute arguments, order
// in which attributes are applied, how to merge attributes if the types are
// structurally equivalent, etc.
const Attr *D1Attr = nullptr, *D2Attr = nullptr;
if (D1->hasAttrs())
D1Attr = *D1->getAttrs().begin();
if (D2->hasAttrs())
D2Attr = *D2->getAttrs().begin();
if ((D1Attr || D2Attr) && !D1->isImplicit() && !D2->isImplicit()) {
AttrComparisonResult R = areDeclAttrsEquivalent(D1, D2);
if (R.Kind != AttrComparisonKind::Equal) {
const auto *DiagnoseDecl = cast<TypeDecl>(PrimaryDecl ? PrimaryDecl : D2);
Context.Diag2(DiagnoseDecl->getLocation(),
diag::warn_odr_tag_type_with_attributes)
<< Context.ToCtx.getTypeDeclType(DiagnoseDecl)
<< (PrimaryDecl != nullptr);
if (D1Attr)
Context.Diag1(D1Attr->getLoc(), diag::note_odr_attr_here) << D1Attr;
if (D2Attr)
Context.Diag1(D2Attr->getLoc(), diag::note_odr_attr_here) << D2Attr;
if (R.A1)
Context.Diag1(R.A1->getLoc(), diag::note_odr_attr_here) << R.A1;
if (R.A2)
Context.Diag1(R.A2->getLoc(), diag::note_odr_attr_here) << R.A2;
}

// The above diagnostic is a warning which defaults to an error. If treated
Expand Down Expand Up @@ -1791,12 +1901,6 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
}
}

// In C23 mode, check for structural equivalence of attributes on the record
// itself. FIXME: Should this happen in C++ as well?
if (Context.LangOpts.C23 &&
!CheckStructurallyEquivalentAttributes(Context, D1, D2))
return false;

// If the records occur in different context (namespace), these should be
// different. This is specially important if the definition of one or both
// records is missing. In C23, different contexts do not make for a different
Expand Down Expand Up @@ -1838,6 +1942,12 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
if (!D1 || !D2)
return !Context.LangOpts.C23;

// In C23 mode, check for structural equivalence of attributes on the record
// itself. FIXME: Should this happen in C++ as well?
if (Context.LangOpts.C23 &&
!CheckStructurallyEquivalentAttributes(Context, D1, D2))
return false;

// If any of the records has external storage and we do a minimal check (or
// AST import) we assume they are equivalent. (If we didn't have this
// assumption then `RecordDecl::LoadFieldsFromExternalStorage` could trigger
Expand Down
87 changes: 87 additions & 0 deletions clang/test/C/C23/n3037.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,90 @@ void gh149965(void) {
enum E2 *eptr2;
[[maybe_unused]] __typeof__(x2.h) *ptr2 = eptr2;
}

struct __attribute__((availability(ios, introduced = 14), availability(macos, introduced = 12))) AvailS0 {
// c17-note@-1 4 {{previous definition is here}}
// c23-note@-2 2 {{attribute 'availability' here}}
int f0 __attribute__((availability(ios, introduced = 15, deprecated = 16)));
// c23-note@-1 {{attribute 'availability' here}}
};

struct __attribute__((availability(ios, introduced = 14), availability(macos, introduced = 12))) AvailS0 {
// c17-error@-1 {{redefinition of 'AvailS0'}}
int f0 __attribute__((availability(ios, introduced = 15, deprecated = 16)));
};

// The order of the attributes matters.
struct __attribute__((availability(macos, introduced = 12), availability(ios, introduced = 14))) AvailS0 {
// c17-error@-1 {{redefinition of 'AvailS0'}}
// c23-error@-2 {{type 'struct AvailS0' has an attribute which currently causes the types to be treated as though they are incompatible}}
// c23-note@-3 {{attribute 'availability' here}}
int f0 __attribute__((availability(ios, introduced = 15, deprecated = 16)));
};

struct __attribute__((availability(ios, introduced = 14))) [[__maybe_unused__]] AvailS0 {
// c17-error@-1 {{redefinition of 'AvailS0'}}
// c23-error@-2 {{type 'struct AvailS0' has an attribute which currently causes the types to be treated as though they are incompatible}}
// c23-note@-3 {{attribute 'maybe_unused' here}}
int f0 __attribute__((availability(ios, introduced = 15, deprecated = 16)));
};

struct __attribute__((availability(ios, introduced = 14), availability(macos, introduced = 12))) AvailS0 {
// c17-error@-1 {{redefinition of 'AvailS0'}}
// c23-error@-2 {{type 'struct AvailS0' has a member with an attribute which currently causes the types to be treated as though they are incompatible}}
int f0 __attribute__((availability(macos, introduced = 13)));
// c23-note@-1 {{attribute 'availability' here}}
};

enum __attribute__((availability(macos, introduced = 12), warn_unused_result)) AvailE0 {
// c17-note@-1 {{previous definition is here}}
// c23-note@-2 {{attribute 'warn_unused_result' here}}
A_E0
// c17-note@-1 {{previous definition is here}}
};

enum __attribute__((availability(macos, introduced = 12), warn_unused_result)) AvailE0 {
// c17-error@-1 {{redefinition of 'AvailE0'}}
// c23-error@-2 {{type 'enum AvailE0' has an attribute which currently causes the types to be treated as though they are incompatible}}
// c23-note@-3 {{attribute 'warn_unused_result' here}}
A_E0
// c17-error@-1 {{redefinition of enumerator 'A_E0'}}
};

enum __attribute__((enum_extensibility(closed))) AvailE1 {
// c17-note@-1 {{previous definition is here}}
A_E1
// c17-note@-1 {{previous definition is here}}
};

enum __attribute__((enum_extensibility(closed))) AvailE1 {
// c17-error@-1 {{redefinition of 'AvailE1'}}
A_E1
// c17-error@-1 {{redefinition of enumerator 'A_E1'}}
};

struct [[__maybe_unused__]] AvailS1;

struct __attribute__((availability(macos, introduced = 12))) AvailS1 {
// c17-note@-1 {{previous definition is here}}
int a;
};

struct __attribute__((availability(macos, introduced = 12))) AvailS1 {
// c17-error@-1 {{redefinition of 'AvailS1'}}
int a;
};

struct __attribute__((warn_unused_result)) AvailS2;
// c23-note@-1 {{attribute 'warn_unused_result' here}}

struct __attribute__((availability(macos, introduced = 12))) AvailS2 {
// c17-note@-1 {{previous definition is here}}
int a;
};

struct __attribute__((availability(macos, introduced = 12))) AvailS2 {
// c17-error@-1 {{redefinition of 'AvailS2'}}
// c23-error@-2 {{type 'struct AvailS2' has an attribute which currently causes the types to be treated as though they are incompatible}}
int a;
};