Skip to content

Commit 751f2fc

Browse files
authored
Disable unique-object-duplication warning in templates (#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.
1 parent a278b28 commit 751f2fc

File tree

2 files changed

+15
-75
lines changed

2 files changed

+15
-75
lines changed

clang/lib/Sema/SemaDecl.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13427,9 +13427,13 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
1342713427
FunDcl->getTemplateSpecializationKind() != TSK_Undeclared;
1342813428
}
1342913429

13430-
// Non-inline functions/variables can only legally appear in one TU,
13431-
// unless they were part of a template.
13432-
if (!TargetIsInline && !TargetWasTemplated)
13430+
// Non-inline functions/variables can only legally appear in one TU
13431+
// unless they were part of a template. Unfortunately, making complex
13432+
// template instantiations visible is infeasible in practice, since
13433+
// everything the template depends on also has to be visible. To avoid
13434+
// giving impractical-to-fix warnings, don't warn if we're inside
13435+
// something that was templated, even on inline stuff.
13436+
if (!TargetIsInline || TargetWasTemplated)
1343313437
return false;
1343413438

1343513439
// If the object isn't hidden, the dynamic linker will prevent duplication.
@@ -13469,8 +13473,8 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
1346913473
// FIXME: Windows uses dllexport/dllimport instead of visibility, and we don't
1347013474
// handle that yet. Disable the warning on Windows for now.
1347113475

13472-
// Don't diagnose if we're inside a template;
13473-
// we'll diagnose during instantiation instead.
13476+
// Don't diagnose if we're inside a template, because it's not practical to
13477+
// fix the warning in most cases.
1347413478
if (!Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
1347513479
!VD->isTemplated() &&
1347613480
GloballyUniqueObjectMightBeAccidentallyDuplicated(VD)) {

clang/test/SemaCXX/unique_object_duplication.h

Lines changed: 6 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -165,81 +165,17 @@ namespace GlobalTest {
165165

166166
namespace TemplateTest {
167167

168-
template <typename T>
169-
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}}
170-
171-
template int disallowedTemplate1<int>; // hidden-note {{in instantiation of}}
172-
173-
174-
// Should work for implicit instantiation as well
175-
template <typename T>
176-
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}}
177-
178-
int implicit_instantiate() {
179-
return disallowedTemplate2<int>; // hidden-note {{in instantiation of}}
180-
}
181-
168+
// We never warn inside templates because it's frequently infeasible to actually
169+
// fix the warning.
182170

183-
// Ensure we only get warnings for templates that are actually instantiated
184171
template <typename T>
185-
int maybeAllowedTemplate = 0; // Not instantiated, so no warning here
186-
187-
template <typename T>
188-
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}}
189-
190-
template <>
191-
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}}
192-
193-
template int maybeAllowedTemplate<int*>; // hidden-note {{in instantiation of}}
172+
int allowedTemplate1 = 0;
194173

195-
196-
197-
// Should work the same for static class members
198-
template <typename T>
199-
struct S {
200-
static int staticMember;
201-
};
174+
template int allowedTemplate1<int>;
202175

203176
template <typename T>
204-
int S<T>::staticMember = 0; // Never instantiated
177+
inline int allowedTemplate2 = 0;
205178

206-
// T* specialization
207-
template <typename T>
208-
struct S<T*> {
209-
static int staticMember;
210-
};
211-
212-
template <typename T>
213-
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}}
214-
215-
template class S<int*>; // hidden-note {{in instantiation of}}
216-
217-
// T& specialization, implicitly instantiated
218-
template <typename T>
219-
struct S<T&> {
220-
static int staticMember;
221-
};
222-
223-
template <typename T>
224-
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}}
225-
226-
int implicit_instantiate2() {
227-
return S<bool&>::staticMember; // hidden-note {{in instantiation of}}
228-
}
229-
230-
231-
// Should work for static locals as well
232-
template <typename T>
233-
int* wrapper() {
234-
static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
235-
return &staticLocal;
236-
}
237-
238-
template <>
239-
int* wrapper<int*>() {
240-
static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
241-
return &staticLocal;
242-
}
179+
template int allowedTemplate2<int>;
243180

244-
auto dummy = wrapper<bool>(); // hidden-note {{in instantiation of}}
245181
} // namespace TemplateTest

0 commit comments

Comments
 (0)