https://github.com/j-hui created https://github.com/llvm/llvm-project/pull/222526
API notes are applied to every declaration of an entity. Since #217187 a redeclaration also inherits the previous declaration's unique 'swift_attr's, so every redeclaration ended up with two copies of each annotation. This patch fixes that behavior to ensure each swift_attr API note is only applied once. rdar://187058947 >From 376c6f30bcd17c301da21d7b41431336ba367399 Mon Sep 17 00:00:00 2001 From: John Hui <[email protected]> Date: Wed, 9 Sep 2026 23:47:50 -0700 Subject: [PATCH] [clang][APINotes] Apply API notes 'swift_attr's idempotently API notes are applied to every declaration of an entity. Since #217187 a redeclaration also inherits the previous declaration's unique 'swift_attr's, so every redeclaration ended up with two copies of each annotation. This patch fixes that behavior to ensure each swift_attr API note is only applied once. rdar://187058947 --- clang/lib/Sema/SemaAPINotes.cpp | 37 +++++++-------- .../APINotes/swift-attr-redeclaration.cpp | 46 +++++++++++++++++++ clang/test/APINotes/swift-import-as.cpp | 12 +++-- 3 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 clang/test/APINotes/swift-attr-redeclaration.cpp diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index 78153d9ddf39d..4c7e5ea16cfd7 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -247,6 +247,15 @@ static void handleAPINotedRetainCountConvention( } } +/// Add a 'swift_attr' unless \p D already carries that exact annotation. +static void addSwiftAttrIfAbsent(Sema &S, Decl *D, StringRef Attribute) { + for (const auto *A : D->specific_attrs<SwiftAttrAttr>()) + if (A->getAttribute() == Attribute) + return; + + D->addAttr(SwiftAttrAttr::Create(S.Context, Attribute)); +} + static void ProcessAPINotes(Sema &S, Decl *D, const api_notes::CommonEntityInfo &Info, VersionedInfoMetadata Metadata) { @@ -362,8 +371,7 @@ static void ProcessAPINotes(Sema &S, Decl *D, } if (auto ConformsTo = Info.getSwiftConformance()) - D->addAttr( - SwiftAttrAttr::Create(S.Context, "conforms_to:" + ConformsTo.value())); + addSwiftAttrIfAbsent(S, D, "conforms_to:" + ConformsTo.value()); ProcessAPINotes(S, D, static_cast<const api_notes::CommonEntityInfo &>(Info), Metadata); @@ -607,8 +615,7 @@ static void ProcessAPINotes(Sema &S, FunctionOrMethod AnyFunc, // returns_(un)retained if (!Info.SwiftReturnOwnership.empty()) - D->addAttr(SwiftAttrAttr::Create(S.Context, - "returns_" + Info.SwiftReturnOwnership)); + addSwiftAttrIfAbsent(S, D, "returns_" + Info.SwiftReturnOwnership); // Result type override. QualType OverriddenResultType; @@ -725,32 +732,22 @@ static void ProcessAPINotes(Sema &S, ObjCMethodDecl *D, static_cast<const api_notes::FunctionInfo &>(Info), Metadata); } -static void addSwiftAttrIfAbsent(Sema &S, Decl *D, StringRef Attribute) { - for (const auto *A : D->specific_attrs<SwiftAttrAttr>()) - if (A->getAttribute() == Attribute) - return; - - D->addAttr(SwiftAttrAttr::Create(S.Context, Attribute)); -} - /// Process API notes for a tag. static void ProcessAPINotes(Sema &S, TagDecl *D, const api_notes::TagInfo &Info, VersionedInfoMetadata Metadata) { if (auto ImportAs = Info.SwiftImportAs) - D->addAttr(SwiftAttrAttr::Create(S.Context, "import_" + ImportAs.value())); + addSwiftAttrIfAbsent(S, D, "import_" + ImportAs.value()); if (auto RetainOp = Info.SwiftRetainOp) - D->addAttr(SwiftAttrAttr::Create(S.Context, "retain:" + RetainOp.value())); + addSwiftAttrIfAbsent(S, D, "retain:" + RetainOp.value()); if (auto ReleaseOp = Info.SwiftReleaseOp) - D->addAttr( - SwiftAttrAttr::Create(S.Context, "release:" + ReleaseOp.value())); + addSwiftAttrIfAbsent(S, D, "release:" + ReleaseOp.value()); if (auto DestroyOp = Info.SwiftDestroyOp) - D->addAttr( - SwiftAttrAttr::Create(S.Context, "destroy:" + DestroyOp.value())); + addSwiftAttrIfAbsent(S, D, "destroy:" + DestroyOp.value()); if (auto DefaultOwnership = Info.SwiftDefaultOwnership) - D->addAttr(SwiftAttrAttr::Create( - S.Context, "returned_as_" + DefaultOwnership.value() + "_by_default")); + addSwiftAttrIfAbsent( + S, D, "returned_as_" + DefaultOwnership.value() + "_by_default"); if (auto Copyable = Info.isSwiftCopyable()) { if (!*Copyable) diff --git a/clang/test/APINotes/swift-attr-redeclaration.cpp b/clang/test/APINotes/swift-attr-redeclaration.cpp new file mode 100644 index 0000000000000..f2e9e58f61bd6 --- /dev/null +++ b/clang/test/APINotes/swift-attr-redeclaration.cpp @@ -0,0 +1,46 @@ +// An API-noted tag that is forward-declared before its definition must not +// accumulate duplicate 'swift_attr's on the definition. +// RUN: rm -rf %t && split-file %s %t +// +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \ +// RUN: -fapinotes-modules -I %t/Inputs -x c++ %t/test.cpp \ +// RUN: -ast-dump -ast-dump-filter FwdThenDefined \ +// RUN: | FileCheck --check-prefix=FWD --implicit-check-not=SwiftAttrAttr %s + +// The forward declaration carries one copy of each annotation. +// +// FWD: Dumping FwdThenDefined: +// FWD: CXXRecordDecl {{.*}} struct FwdThenDefined +// FWD-NEXT: SwiftAttrAttr {{.*}} "import_reference" +// FWD-NEXT: SwiftAttrAttr {{.*}} "retain:FTDRetain" +// FWD-NEXT: SwiftAttrAttr {{.*}} "release:FTDRelease" + +// So must the definition: the copies it inherits and the copies API notes +// applies to it are the same three annotations. +// +// FWD: Dumping FwdThenDefined: +// FWD: CXXRecordDecl {{.*}} prev {{.*}} struct FwdThenDefined definition +// FWD: SwiftAttrAttr {{.*}} "import_reference" +// FWD-NEXT: SwiftAttrAttr {{.*}} "retain:FTDRetain" +// FWD-NEXT: SwiftAttrAttr {{.*}} "release:FTDRelease" + +//--- Inputs/module.modulemap +module Redecl { + header "Redecl.h" +} + +//--- Inputs/Redecl.apinotes +--- +Name: Redecl +Tags: +- Name: FwdThenDefined + SwiftImportAs: reference + SwiftRetainOp: FTDRetain + SwiftReleaseOp: FTDRelease + +//--- Inputs/Redecl.h +struct FwdThenDefined; +struct FwdThenDefined {}; + +//--- test.cpp +#include "Redecl.h" diff --git a/clang/test/APINotes/swift-import-as.cpp b/clang/test/APINotes/swift-import-as.cpp index 1ff09900350e2..212d5a7b4c8e5 100644 --- a/clang/test/APINotes/swift-import-as.cpp +++ b/clang/test/APINotes/swift-import-as.cpp @@ -46,15 +46,17 @@ // CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "import_reference" // CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "retain:ORCRetain" // CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "release:ORCRelease" -// CHECK-OPAQUE-REF-COUNTED-NOT: SwiftAttrAttr {{.+}} <<invalid sloc>> "release:ORCRelease" +// CHECK-OPAQUE-REF-COUNTED-NOT: SwiftAttrAttr {{.*}}"release:ORCRelease" +// The redeclaration inherits the annotations rather than having API notes +// applied a second time, so it carries one copy of each, marked Inherited. // CHECK-OPAQUE-REF-COUNTED: Dumping OpaqueRefCountedType: // CHECK-OPAQUE-REF-COUNTED-NEXT: CXXRecordDecl {{.+}} imported in SwiftImportAs{{.*}}struct OpaqueRefCountedType -// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "import_reference" -// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "retain:ORCRetain" -// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> "release:ORCRelease" +// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> Inherited "import_reference" +// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> Inherited "retain:ORCRetain" +// CHECK-OPAQUE-REF-COUNTED: SwiftAttrAttr {{.+}} <<invalid sloc>> Inherited "release:ORCRelease" -// CHECK-OPAQUE-REF-COUNTED-NOT: SwiftAttrAttr {{.+}} <<invalid sloc>> "release: +// CHECK-OPAQUE-REF-COUNTED-NOT: SwiftAttrAttr {{.*}}"release: // CHECK-NON-COPYABLE: Dumping NonCopyableType: // CHECK-NON-COPYABLE-NEXT: CXXRecordDecl {{.+}} imported in SwiftImportAs {{.+}} struct NonCopyableType // CHECK-NON-COPYABLE: SwiftAttrAttr {{.+}} <<invalid sloc>> "~Copyable" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
