From 5ab3114bd12cdafc1e4e384e3a06c7258723ebde Mon Sep 17 00:00:00 2001
From: Devon Loehr <DKLoehr@users.noreply.github.com>
Date: Mon, 30 Jun 2025 10:51:04 -0400
Subject: [PATCH] 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.
---
 clang/lib/Sema/SemaDecl.cpp                   | 14 +++---
 .../test/SemaCXX/unique_object_duplication.h  | 45 +++++++++++++++++--
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 33b9ef869746..f4bc191d1dae 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 bd0ee6bd14d6..a5da767b7947 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