Skip to content

Commit a8a89da

Browse files
aaronpuchertaaronpuchert
committed
Support pack expansion for Clang Thread Safety attributes
Support for attribute parameter packs was added some time ago in commit ead1690. But template substitution didn't expand the packs yet. For now expansion can only happen within a `VariadicExprArgument`: i.e. in `try_acquire_capability`, which takes a regular and a variadic argument, the template can't have a single pack that then expands to cover both arguments. This is a prerequisite for #42000.
1 parent 826f237 commit a8a89da

File tree

4 files changed

+214
-18
lines changed

4 files changed

+214
-18
lines changed

‎clang/include/clang/Basic/Attr.td‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3856,6 +3856,7 @@ def AssertCapability : InheritableAttr {
38563856
let ParseArgumentsAsUnevaluated = 1;
38573857
let InheritEvenIfAlreadyPresent = 1;
38583858
let Args = [VariadicExprArgument<"Args">];
3859+
let AcceptsExprPack = 1;
38593860
let Accessors = [Accessor<"isShared",
38603861
[Clang<"assert_shared_capability", 0>,
38613862
GNU<"assert_shared_lock">]>];
@@ -3873,6 +3874,7 @@ def AcquireCapability : InheritableAttr {
38733874
let ParseArgumentsAsUnevaluated = 1;
38743875
let InheritEvenIfAlreadyPresent = 1;
38753876
let Args = [VariadicExprArgument<"Args">];
3877+
let AcceptsExprPack = 1;
38763878
let Accessors = [Accessor<"isShared",
38773879
[Clang<"acquire_shared_capability", 0>,
38783880
GNU<"shared_lock_function">]>];
@@ -3890,6 +3892,7 @@ def TryAcquireCapability : InheritableAttr {
38903892
let ParseArgumentsAsUnevaluated = 1;
38913893
let InheritEvenIfAlreadyPresent = 1;
38923894
let Args = [ExprArgument<"SuccessValue">, VariadicExprArgument<"Args">];
3895+
let AcceptsExprPack = 1;
38933896
let Accessors = [Accessor<"isShared",
38943897
[Clang<"try_acquire_shared_capability", 0>,
38953898
GNU<"shared_trylock_function">]>];
@@ -3907,6 +3910,7 @@ def ReleaseCapability : InheritableAttr {
39073910
let ParseArgumentsAsUnevaluated = 1;
39083911
let InheritEvenIfAlreadyPresent = 1;
39093912
let Args = [VariadicExprArgument<"Args">];
3913+
let AcceptsExprPack = 1;
39103914
let Accessors = [Accessor<"isShared",
39113915
[Clang<"release_shared_capability", 0>]>,
39123916
Accessor<"isGeneric",
@@ -3921,6 +3925,7 @@ def RequiresCapability : InheritableAttr {
39213925
Clang<"requires_shared_capability", 0>,
39223926
Clang<"shared_locks_required", 0>];
39233927
let Args = [VariadicExprArgument<"Args">];
3928+
let AcceptsExprPack = 1;
39243929
let LateParsed = LateAttrParseStandard;
39253930
let TemplateDependent = 1;
39263931
let ParseArgumentsAsUnevaluated = 1;
@@ -3963,6 +3968,7 @@ def PtGuardedBy : InheritableAttr {
39633968
def AcquiredAfter : InheritableAttr {
39643969
let Spellings = [GNU<"acquired_after">];
39653970
let Args = [VariadicExprArgument<"Args">];
3971+
let AcceptsExprPack = 1;
39663972
let LateParsed = LateAttrParseExperimentalExt;
39673973
let TemplateDependent = 1;
39683974
let ParseArgumentsAsUnevaluated = 1;
@@ -3974,6 +3980,7 @@ def AcquiredAfter : InheritableAttr {
39743980
def AcquiredBefore : InheritableAttr {
39753981
let Spellings = [GNU<"acquired_before">];
39763982
let Args = [VariadicExprArgument<"Args">];
3983+
let AcceptsExprPack = 1;
39773984
let LateParsed = LateAttrParseExperimentalExt;
39783985
let TemplateDependent = 1;
39793986
let ParseArgumentsAsUnevaluated = 1;
@@ -3995,6 +4002,7 @@ def LockReturned : InheritableAttr {
39954002
def LocksExcluded : InheritableAttr {
39964003
let Spellings = [GNU<"locks_excluded">];
39974004
let Args = [VariadicExprArgument<"Args">];
4005+
let AcceptsExprPack = 1;
39984006
let LateParsed = LateAttrParseStandard;
39994007
let TemplateDependent = 1;
40004008
let ParseArgumentsAsUnevaluated = 1;

‎clang/test/SemaCXX/warn-thread-safety-analysis.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ class SCOPED_LOCKABLE DoubleMutexLock {
5757
~DoubleMutexLock() UNLOCK_FUNCTION();
5858
};
5959

60+
template<typename Mu>
61+
class SCOPED_LOCKABLE TemplateMutexLock {
62+
public:
63+
TemplateMutexLock(Mu *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
64+
~TemplateMutexLock() UNLOCK_FUNCTION();
65+
};
66+
67+
template<typename... Mus>
68+
class SCOPED_LOCKABLE VariadicMutexLock {
69+
public:
70+
VariadicMutexLock(Mus *...mus) EXCLUSIVE_LOCK_FUNCTION(mus...);
71+
~VariadicMutexLock() UNLOCK_FUNCTION();
72+
};
73+
6074
// The universal lock, written "*", allows checking to be selectively turned
6175
// off for a particular piece of code.
6276
void beginNoWarnOnReads() SHARED_LOCK_FUNCTION("*");
@@ -1821,6 +1835,18 @@ struct TestScopedLockable {
18211835
a = b + 1;
18221836
b = a + 1;
18231837
}
1838+
1839+
void foo6() {
1840+
TemplateMutexLock<Mutex> mulock1(&mu1), mulock2(&mu2);
1841+
a = b + 1;
1842+
b = a + 1;
1843+
}
1844+
1845+
void foo7() {
1846+
VariadicMutexLock<Mutex, Mutex> mulock(&mu1, &mu2);
1847+
a = b + 1;
1848+
b = a + 1;
1849+
}
18241850
};
18251851

18261852
} // end namespace test_scoped_lockable
@@ -3114,6 +3140,16 @@ class SCOPED_LOCKABLE ReaderMutexUnlock {
31143140
void Unlock() EXCLUSIVE_LOCK_FUNCTION();
31153141
};
31163142

3143+
template<typename... Mus>
3144+
class SCOPED_LOCKABLE VariadicMutexUnlock {
3145+
public:
3146+
VariadicMutexUnlock(Mus *...mus) EXCLUSIVE_UNLOCK_FUNCTION(mus...);
3147+
~VariadicMutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION();
3148+
3149+
void Lock() EXCLUSIVE_UNLOCK_FUNCTION();
3150+
void Unlock() EXCLUSIVE_LOCK_FUNCTION();
3151+
};
3152+
31173153
Mutex mu;
31183154
int x GUARDED_BY(mu);
31193155
bool c;
@@ -3176,6 +3212,17 @@ void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) {
31763212
scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
31773213
}
31783214

3215+
Mutex mu2;
3216+
int y GUARDED_BY(mu2);
3217+
3218+
void variadic() EXCLUSIVE_LOCKS_REQUIRED(mu, mu2) {
3219+
VariadicMutexUnlock<Mutex, Mutex> scope(&mu, &mu2);
3220+
x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
3221+
y = 3; // expected-warning {{writing variable 'y' requires holding mutex 'mu2' exclusively}}
3222+
scope.Lock();
3223+
x = y = 4;
3224+
}
3225+
31793226
class SCOPED_LOCKABLE MutexLockUnlock {
31803227
public:
31813228
MutexLockUnlock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_UNLOCK_FUNCTION(mu1) EXCLUSIVE_LOCK_FUNCTION(mu2);

‎clang/test/SemaCXX/warn-thread-safety-parsing.cpp‎

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
22
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++98 %s
3-
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s -D CPP11
3+
// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s
44

55
#define LOCKABLE __attribute__ ((lockable))
66
#define SCOPED_LOCKABLE __attribute__ ((scoped_lockable))
@@ -643,6 +643,24 @@ int elf_function_bad_6(Mutex x, Mutex y) EXCLUSIVE_LOCK_FUNCTION(0); // \
643643
int elf_function_bad_7() EXCLUSIVE_LOCK_FUNCTION(0); // \
644644
// expected-error {{'exclusive_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}}
645645

646+
template<typename Mu>
647+
int elf_template(Mu& mu) EXCLUSIVE_LOCK_FUNCTION(mu) {}
648+
649+
template int elf_template<Mutex>(Mutex&);
650+
// FIXME: warn on template instantiation.
651+
template int elf_template<UnlockableMu>(UnlockableMu&);
652+
653+
#if __cplusplus >= 201103
654+
655+
template<typename... Mus>
656+
int elf_variadic_template(Mus&... mus) EXCLUSIVE_LOCK_FUNCTION(mus...) {}
657+
658+
template int elf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
659+
// FIXME: warn on template instantiation.
660+
template int elf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
661+
662+
#endif
663+
646664

647665
//-----------------------------------------//
648666
// Shared Lock Function (slf)
@@ -719,6 +737,24 @@ int slf_function_bad_6(Mutex x, Mutex y) SHARED_LOCK_FUNCTION(0); // \
719737
int slf_function_bad_7() SHARED_LOCK_FUNCTION(0); // \
720738
// expected-error {{'shared_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}}
721739

740+
template<typename Mu>
741+
int slf_template(Mu& mu) SHARED_LOCK_FUNCTION(mu) {}
742+
743+
template int slf_template<Mutex>(Mutex&);
744+
// FIXME: warn on template instantiation.
745+
template int slf_template<UnlockableMu>(UnlockableMu&);
746+
747+
#if __cplusplus >= 201103
748+
749+
template<typename... Mus>
750+
int slf_variadic_template(Mus&... mus) SHARED_LOCK_FUNCTION(mus...) {}
751+
752+
template int slf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
753+
// FIXME: warn on template instantiation.
754+
template int slf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
755+
756+
#endif
757+
722758

723759
//-----------------------------------------//
724760
// Exclusive TryLock Function (etf)
@@ -796,6 +832,24 @@ int etf_function_bad_5() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoublePointer); // \
796832
int etf_function_bad_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, umu); // \
797833
// expected-warning {{'exclusive_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}}
798834

835+
template<typename Mu>
836+
int etf_template(Mu& mu) EXCLUSIVE_TRYLOCK_FUNCTION(1, mu) {}
837+
838+
template int etf_template<Mutex>(Mutex&);
839+
// FIXME: warn on template instantiation.
840+
template int etf_template<UnlockableMu>(UnlockableMu&);
841+
842+
#if __cplusplus >= 201103
843+
844+
template<typename... Mus>
845+
int etf_variadic_template(Mus&... mus) EXCLUSIVE_TRYLOCK_FUNCTION(1, mus...) {}
846+
847+
template int etf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
848+
// FIXME: warn on template instantiation.
849+
template int etf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
850+
851+
#endif
852+
799853

800854
//-----------------------------------------//
801855
// Shared TryLock Function (stf)
@@ -874,6 +928,24 @@ int stf_function_bad_5() SHARED_TRYLOCK_FUNCTION(1, muDoublePointer); // \
874928
int stf_function_bad_6() SHARED_TRYLOCK_FUNCTION(1, umu); // \
875929
// expected-warning {{'shared_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}}
876930

931+
template<typename Mu>
932+
int stf_template(Mu& mu) SHARED_TRYLOCK_FUNCTION(1, mu) {}
933+
934+
template int stf_template<Mutex>(Mutex&);
935+
// FIXME: warn on template instantiation.
936+
template int stf_template<UnlockableMu>(UnlockableMu&);
937+
938+
#if __cplusplus >= 201103
939+
940+
template<typename... Mus>
941+
int stf_variadic_template(Mus&... mus) SHARED_TRYLOCK_FUNCTION(1, mus...) {}
942+
943+
template int stf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
944+
// FIXME: warn on template instantiation.
945+
template int stf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
946+
947+
#endif
948+
877949

878950
//-----------------------------------------//
879951
// Unlock Function (uf)
@@ -953,6 +1025,24 @@ int uf_function_bad_6(Mutex x, Mutex y) UNLOCK_FUNCTION(0); // \
9531025
int uf_function_bad_7() UNLOCK_FUNCTION(0); // \
9541026
// expected-error {{'unlock_function' attribute parameter 1 is out of bounds: no parameters to index into}}
9551027

1028+
template<typename Mu>
1029+
int uf_template(Mu& mu) UNLOCK_FUNCTION(mu) {}
1030+
1031+
template int uf_template<Mutex>(Mutex&);
1032+
// FIXME: warn on template instantiation.
1033+
template int uf_template<UnlockableMu>(UnlockableMu&);
1034+
1035+
#if __cplusplus >= 201103
1036+
1037+
template<typename... Mus>
1038+
int uf_variadic_template(Mus&... mus) UNLOCK_FUNCTION(mus...) {}
1039+
1040+
template int uf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
1041+
// FIXME: warn on template instantiation.
1042+
template int uf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
1043+
1044+
#endif
1045+
9561046

9571047
//-----------------------------------------//
9581048
// Lock Returned (lr)
@@ -1088,6 +1178,23 @@ int le_function_bad_3() LOCKS_EXCLUDED(muDoublePointer); // \
10881178
int le_function_bad_4() LOCKS_EXCLUDED(umu); // \
10891179
// expected-warning {{'locks_excluded' attribute requires arguments whose type is annotated with 'capability' attribute}}
10901180

1181+
template<typename Mu>
1182+
int le_template(Mu& mu) LOCKS_EXCLUDED(mu) {}
1183+
1184+
template int le_template<Mutex>(Mutex&);
1185+
// FIXME: warn on template instantiation.
1186+
template int le_template<UnlockableMu>(UnlockableMu&);
1187+
1188+
#if __cplusplus >= 201103
1189+
1190+
template<typename... Mus>
1191+
int le_variadic_template(Mus&... mus) LOCKS_EXCLUDED(mus...) {}
1192+
1193+
template int le_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
1194+
// FIXME: warn on template instantiation.
1195+
template int le_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
1196+
1197+
#endif
10911198

10921199

10931200
//-----------------------------------------//
@@ -1156,6 +1263,24 @@ int elr_function_bad_3() EXCLUSIVE_LOCKS_REQUIRED(muDoublePointer); // \
11561263
int elr_function_bad_4() EXCLUSIVE_LOCKS_REQUIRED(umu); // \
11571264
// expected-warning {{'exclusive_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}}
11581265

1266+
template<typename Mu>
1267+
int elr_template(Mu& mu) EXCLUSIVE_LOCKS_REQUIRED(mu) {}
1268+
1269+
template int elr_template<Mutex>(Mutex&);
1270+
// FIXME: warn on template instantiation.
1271+
template int elr_template<UnlockableMu>(UnlockableMu&);
1272+
1273+
#if __cplusplus >= 201103
1274+
1275+
template<typename... Mus>
1276+
int elr_variadic_template(Mus&... mus) EXCLUSIVE_LOCKS_REQUIRED(mus...) {}
1277+
1278+
template int elr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
1279+
// FIXME: warn on template instantiation.
1280+
template int elr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
1281+
1282+
#endif
1283+
11591284

11601285

11611286

@@ -1225,6 +1350,24 @@ int slr_function_bad_3() SHARED_LOCKS_REQUIRED(muDoublePointer); // \
12251350
int slr_function_bad_4() SHARED_LOCKS_REQUIRED(umu); // \
12261351
// expected-warning {{'shared_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}}
12271352

1353+
template<typename Mu>
1354+
int slr_template(Mu& mu) SHARED_LOCKS_REQUIRED(mu) {}
1355+
1356+
template int slr_template<Mutex>(Mutex&);
1357+
// FIXME: warn on template instantiation.
1358+
template int slr_template<UnlockableMu>(UnlockableMu&);
1359+
1360+
#if __cplusplus >= 201103
1361+
1362+
template<typename... Mus>
1363+
int slr_variadic_template(Mus&... mus) SHARED_LOCKS_REQUIRED(mus...) {}
1364+
1365+
template int slr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&);
1366+
// FIXME: warn on template instantiation.
1367+
template int slr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&);
1368+
1369+
#endif
1370+
12281371

12291372
//-----------------------------------------//
12301373
// Regression tests for unusual cases.
@@ -1582,7 +1725,7 @@ class Foo {
15821725
} // end namespace FunctionAttributesInsideClass_ICE_Test
15831726

15841727

1585-
#ifdef CPP11
1728+
#if __cplusplus >= 201103
15861729
namespace CRASH_POST_R301735 {
15871730
class SomeClass {
15881731
public:

‎clang/utils/TableGen/ClangAttrEmitter.cpp‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,28 +1329,26 @@ namespace {
13291329

13301330
void writeTemplateInstantiationArgs(raw_ostream &OS) const override {
13311331
OS << "tempInst" << getUpperName() << ", "
1332-
<< "A->" << getLowerName() << "_size()";
1332+
<< "numTempInst" << getUpperName();
13331333
}
13341334

13351335
void writeTemplateInstantiation(raw_ostream &OS) const override {
1336-
OS << " auto *tempInst" << getUpperName()
1337-
<< " = new (C, 16) " << getType()
1338-
<< "[A->" << getLowerName() << "_size()];\n";
1336+
OS << " size_t numTempInst" << getUpperName() << ";\n";
1337+
OS << " " << getType() << "*tempInst" << getUpperName() << ";\n";
13391338
OS << " {\n";
13401339
OS << " EnterExpressionEvaluationContext "
13411340
<< "Unevaluated(S, Sema::ExpressionEvaluationContext::Unevaluated);\n";
1342-
OS << " " << getType() << " *TI = tempInst" << getUpperName()
1343-
<< ";\n";
1344-
OS << " " << getType() << " *I = A->" << getLowerName()
1345-
<< "_begin();\n";
1346-
OS << " " << getType() << " *E = A->" << getLowerName()
1347-
<< "_end();\n";
1348-
OS << " for (; I != E; ++I, ++TI) {\n";
1349-
OS << " ExprResult Result = S.SubstExpr(*I, TemplateArgs);\n";
1350-
OS << " if (Result.isInvalid())\n";
1351-
OS << " return nullptr;\n";
1352-
OS << " *TI = Result.get();\n";
1353-
OS << " }\n";
1341+
OS << " ArrayRef<" << getType() << "> ArgsToInstantiate(A->"
1342+
<< getLowerName() << "_begin(), A->" << getLowerName() << "_end());\n";
1343+
OS << " SmallVector<" << getType() << ", 4> InstArgs;\n";
1344+
OS << " if (S.SubstExprs(ArgsToInstantiate, /*IsCall=*/false, "
1345+
"TemplateArgs, InstArgs))\n";
1346+
OS << " return nullptr;\n";
1347+
OS << " numTempInst" << getUpperName() << " = InstArgs.size();\n";
1348+
OS << " tempInst" << getUpperName() << " = new (C, 16) "
1349+
<< getType() << "[numTempInst" << getUpperName() << "];\n";
1350+
OS << " std::copy(InstArgs.begin(), InstArgs.end(), tempInst"
1351+
<< getUpperName() << ");\n";
13541352
OS << " }\n";
13551353
}
13561354

0 commit comments

Comments
 (0)