Author: Devon Loehr Date: 2025-06-30T10:51:04-04:00 New Revision: 5ab3114bd12cdafc1e4e384e3a06c7258723ebde
URL: https://github.com/llvm/llvm-project/commit/5ab3114bd12cdafc1e4e384e3a06c7258723ebde DIFF: https://github.com/llvm/llvm-project/commit/5ab3114bd12cdafc1e4e384e3a06c7258723ebde.diff LOG: Expand annotation check for -Wunique-object-duplication on Windows. (#145944) Since dllexport/dllimport annotations don't propagate the same way as visibility, the unique object duplication warning needs to check both the object in question and its containing class. Previously, we restricted this check to static data members, but it applies to all objects inside a class, including functions. Not checking functions leads to false positives, so remove that restriction. Added: Modified: clang/lib/Sema/SemaDecl.cpp clang/test/SemaCXX/unique_object_duplication.h Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 33b9ef869746a..f4bc191d1dae6 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -13531,21 +13531,19 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated( // The target is "hidden" (from the dynamic linker) if: // 1. On posix, it has hidden visibility, or - // 2. On windows, it has no import/export annotation + // 2. On windows, it has no import/export annotation, and neither does the + // class which directly contains it. if (Context.getTargetInfo().shouldDLLImportComdatSymbols()) { if (Target->hasAttr<DLLExportAttr>() || Target->hasAttr<DLLImportAttr>()) return false; // If the variable isn't directly annotated, check to see if it's a member // of an annotated class. - const VarDecl *VD = dyn_cast<VarDecl>(Target); + const CXXRecordDecl *Ctx = + dyn_cast<CXXRecordDecl>(Target->getDeclContext()); + if (Ctx && (Ctx->hasAttr<DLLExportAttr>() || Ctx->hasAttr<DLLImportAttr>())) + return false; - if (VD && VD->isStaticDataMember()) { - const CXXRecordDecl *Ctx = dyn_cast<CXXRecordDecl>(VD->getDeclContext()); - if (Ctx && - (Ctx->hasAttr<DLLExportAttr>() || Ctx->hasAttr<DLLImportAttr>())) - return false; - } } else if (Lnk.getVisibility() != HiddenVisibility) { // Posix case return false; diff --git a/clang/test/SemaCXX/unique_object_duplication.h b/clang/test/SemaCXX/unique_object_duplication.h index bd0ee6bd14d64..a5da767b79474 100644 --- a/clang/test/SemaCXX/unique_object_duplication.h +++ b/clang/test/SemaCXX/unique_object_duplication.h @@ -173,10 +173,6 @@ namespace GlobalTest { // Always visible VISIBLE static inline float allowedStaticMember2 = 0.0; - - // Tests here are sparse because the AddrTest case below will define plenty - // more, which aren't problematic to define (because they're immutable), but - // may still cause problems if their address is taken. }; inline float Test::disallowedStaticMember2 = 2.3; // hidden-warning {{'disallowedStaticMember2' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}} @@ -211,3 +207,44 @@ inline int allowedTemplate2 = 0; template int allowedTemplate2<int>; } // namespace TemplateTest + +/****************************************************************************** + * Case four: Nested classes + ******************************************************************************/ + +namespace NestedClassTest { +// Unlike visibility, declexport annotations do not propagate down to nested +// classes. Hence on windows, we only avoid duplication of class members if +// their immediate containing class is annotated. On posix, we get avoid +// duplication if any containing class is annotated. + +class VISIBLE Outer { + // Visible because their class is exported + inline static int allowedOuterMember = 0; + int* allowedOuterFunction() { + static int allowed = 0; + return &allowed; + } + + // No annotation, and visibility is only propagated on posix. + class HiddenOnWindows { + inline static int disallowedInnerMember = 0; // windows-warning {{'disallowedInnerMember' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}} + + + int* disallowedInnerFunction() { + static int disallowed = 0; // windows-warning {{'disallowed' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}} + return &disallowed; + } + }; + + class VISIBLE AlwaysVisible { + inline static int allowedInnerMember = 0; + + int* allowedInnerFunction() { + static int allowed = 0; + return &allowed; + } + }; +}; + +} \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
