https://github.com/DKLoehr created
https://github.com/llvm/llvm-project/pull/129120
I've been trying to resolve instances of the unique-object-duplication warning
in chromium code. Unfortunately, I've found that practically speaking, it's
near-impossible to actually fix the problem when templates are involved.
My understanding is that the warning is correct -- the variables it's flagging
are indeed duplicated and potentially causing bugs as a result. The problem is
that hiddenness is contagious: if a templated class or variable depends on
something hidden, then it itself must also be hidden, even if the user
explicitly marked it visible. In order to make it actually visible, the user
must manually figure out everything that it depends on, mark them as visible,
and do so recursively until all of its ancestors are visible.
This process is extremely difficult and unergonomic, negating much of the
benefits of templates since now each new use requires additional work.
Furthermore, the process doesn't work if the user can't edit some of the files,
e.g. if they're in a third-party library.
Since a warning that can't practically be fixed isn't useful, this PR disables
the warning for _all_ templated code by inverting the check. The warning
remains active (and, in my experience, easily fixable) in non-templated code.
>From a52c1e72885f7a93ef77f707cb56db3053d5bd34 Mon Sep 17 00:00:00 2001
From: Devon Loehr <dlo...@google.com>
Date: Thu, 27 Feb 2025 21:01:48 +0000
Subject: [PATCH] Disable UOD warning in templates
---
clang/lib/Sema/SemaDecl.cpp | 14 ++--
.../test/SemaCXX/unique_object_duplication.h | 76 ++-----------------
2 files changed, 15 insertions(+), 75 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 285bd27a35a76..86e65e56accc8 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13427,9 +13427,13 @@ bool
Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
FunDcl->getTemplateSpecializationKind() != TSK_Undeclared;
}
- // Non-inline functions/variables can only legally appear in one TU,
- // unless they were part of a template.
- if (!TargetIsInline && !TargetWasTemplated)
+ // Non-inline functions/variables can only legally appear in one TU
+ // unless they were part of a template. Unfortunately, making complex
+ // template instantiations visible is infeasible in practice, since
+ // everything the template depends on also has to be visible. To avoid
+ // giving impractical-to-fix warnings, don't warn if we're inside
+ // something that was templated, even on inline stuff.
+ if (!TargetIsInline || TargetWasTemplated)
return false;
// If the object isn't hidden, the dynamic linker will prevent duplication.
@@ -13469,8 +13473,8 @@ void Sema::DiagnoseUniqueObjectDuplication(const
VarDecl *VD) {
// FIXME: Windows uses dllexport/dllimport instead of visibility, and we
don't
// handle that yet. Disable the warning on Windows for now.
- // Don't diagnose if we're inside a template;
- // we'll diagnose during instantiation instead.
+ // Don't diagnose if we're inside a template, because it's not practical to
+ // fix the warning in most cases.
if (!Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
!VD->isTemplated() &&
GloballyUniqueObjectMightBeAccidentallyDuplicated(VD)) {
diff --git a/clang/test/SemaCXX/unique_object_duplication.h
b/clang/test/SemaCXX/unique_object_duplication.h
index 861175766db70..e5c63efbf918c 100644
--- a/clang/test/SemaCXX/unique_object_duplication.h
+++ b/clang/test/SemaCXX/unique_object_duplication.h
@@ -165,81 +165,17 @@ namespace GlobalTest {
namespace TemplateTest {
-template <typename T>
-int disallowedTemplate1 = 0; // hidden-warning {{'disallowedTemplate1<int>'
may be duplicated when built into a shared library: it is mutable, has hidden
visibility, and external linkage}}
-
-template int disallowedTemplate1<int>; // hidden-note {{in instantiation of}}
-
-
-// Should work for implicit instantiation as well
-template <typename T>
-int disallowedTemplate2 = 0; // hidden-warning {{'disallowedTemplate2<int>'
may be duplicated when built into a shared library: it is mutable, has hidden
visibility, and external linkage}}
-
-int implicit_instantiate() {
- return disallowedTemplate2<int>; // hidden-note {{in instantiation of}}
-}
-
+// We never warn inside templates because it's frequently infeasible to
actually
+// fix the warning.
-// Ensure we only get warnings for templates that are actually instantiated
template <typename T>
-int maybeAllowedTemplate = 0; // Not instantiated, so no warning here
-
-template <typename T>
-int maybeAllowedTemplate<T*> = 1; // hidden-warning
{{'maybeAllowedTemplate<int *>' may be duplicated when built into a shared
library: it is mutable, has hidden visibility, and external linkage}}
-
-template <>
-int maybeAllowedTemplate<bool> = 2; // hidden-warning
{{'maybeAllowedTemplate<bool>' may be duplicated when built into a shared
library: it is mutable, has hidden visibility, and external linkage}}
-
-template int maybeAllowedTemplate<int*>; // hidden-note {{in instantiation of}}
+int allowedTemplate1 = 0;
-
-
-// Should work the same for static class members
-template <typename T>
-struct S {
- static int staticMember;
-};
+template int allowedTemplate1<int>;
template <typename T>
-int S<T>::staticMember = 0; // Never instantiated
+inline int allowedTemplate2 = 0;
-// T* specialization
-template <typename T>
-struct S<T*> {
- static int staticMember;
-};
-
-template <typename T>
-int S<T*>::staticMember = 1; // hidden-warning {{'staticMember' may be
duplicated when built into a shared library: it is mutable, has hidden
visibility, and external linkage}}
-
-template class S<int*>; // hidden-note {{in instantiation of}}
-
-// T& specialization, implicitly instantiated
-template <typename T>
-struct S<T&> {
- static int staticMember;
-};
-
-template <typename T>
-int S<T&>::staticMember = 2; // hidden-warning {{'staticMember' may be
duplicated when built into a shared library: it is mutable, has hidden
visibility, and external linkage}}
-
-int implicit_instantiate2() {
- return S<bool&>::staticMember; // hidden-note {{in instantiation of}}
-}
-
-
-// Should work for static locals as well
-template <typename T>
-int* wrapper() {
- static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated
when built into a shared library: it is mutable, has hidden visibility, and
external linkage}}
- return &staticLocal;
-}
-
-template <>
-int* wrapper<int*>() {
- static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated
when built into a shared library: it is mutable, has hidden visibility, and
external linkage}}
- return &staticLocal;
-}
+template int allowedTemplate2<int>;
-auto dummy = wrapper<bool>(); // hidden-note {{in instantiation of}}
} // namespace TemplateTest
\ 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
