Skip to content

Commit 228b330

Browse files
aaronpuchertaaronpuchert
authored
Support pack expansion for Clang Thread Safety attributes (#137477)
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 b85f37a commit 228b330

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)