llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Aaron Puchert (aaronpuchert) <details> <summary>Changes</summary> Support for attribute parameter packs was added some time ago in commit ead1690d31f815c00fdd2bc23db4766191bbeabc. 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. --- Full diff: https://github.com/llvm/llvm-project/pull/137477.diff 4 Files Affected: - (modified) clang/include/clang/Basic/Attr.td (+8) - (modified) clang/test/SemaCXX/warn-thread-safety-analysis.cpp (+47) - (modified) clang/test/SemaCXX/warn-thread-safety-parsing.cpp (+145-2) - (modified) clang/utils/TableGen/ClangAttrEmitter.cpp (+14-16) ``````````diff diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index dcdcff8c46fe2..2548e60312a0d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3856,6 +3856,7 @@ def AssertCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"assert_shared_capability", 0>, GNU<"assert_shared_lock">]>]; @@ -3873,6 +3874,7 @@ def AcquireCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"acquire_shared_capability", 0>, GNU<"shared_lock_function">]>]; @@ -3890,6 +3892,7 @@ def TryAcquireCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [ExprArgument<"SuccessValue">, VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"try_acquire_shared_capability", 0>, GNU<"shared_trylock_function">]>]; @@ -3907,6 +3910,7 @@ def ReleaseCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"release_shared_capability", 0>]>, Accessor<"isGeneric", @@ -3921,6 +3925,7 @@ def RequiresCapability : InheritableAttr { Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3963,6 +3968,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3974,6 +3980,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3995,6 +4002,7 @@ def LockReturned : InheritableAttr { def LocksExcluded : InheritableAttr { let Spellings = [GNU<"locks_excluded">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 2a04e820eb095..4c17294d11e79 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -57,6 +57,20 @@ class SCOPED_LOCKABLE DoubleMutexLock { ~DoubleMutexLock() UNLOCK_FUNCTION(); }; +template<typename Mu> +class SCOPED_LOCKABLE TemplateMutexLock { +public: + TemplateMutexLock(Mu *mu) EXCLUSIVE_LOCK_FUNCTION(mu); + ~TemplateMutexLock() UNLOCK_FUNCTION(); +}; + +template<typename... Mus> +class SCOPED_LOCKABLE VariadicMutexLock { +public: + VariadicMutexLock(Mus *...mus) EXCLUSIVE_LOCK_FUNCTION(mus...); + ~VariadicMutexLock() UNLOCK_FUNCTION(); +}; + // The universal lock, written "*", allows checking to be selectively turned // off for a particular piece of code. void beginNoWarnOnReads() SHARED_LOCK_FUNCTION("*"); @@ -1821,6 +1835,18 @@ struct TestScopedLockable { a = b + 1; b = a + 1; } + + void foo6() { + TemplateMutexLock<Mutex> mulock1(&mu1), mulock2(&mu2); + a = b + 1; + b = a + 1; + } + + void foo7() { + VariadicMutexLock<Mutex, Mutex> mulock(&mu1, &mu2); + a = b + 1; + b = a + 1; + } }; } // end namespace test_scoped_lockable @@ -3114,6 +3140,16 @@ class SCOPED_LOCKABLE ReaderMutexUnlock { void Unlock() EXCLUSIVE_LOCK_FUNCTION(); }; +template<typename... Mus> +class SCOPED_LOCKABLE VariadicMutexUnlock { +public: + VariadicMutexUnlock(Mus *...mus) EXCLUSIVE_UNLOCK_FUNCTION(mus...); + ~VariadicMutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION(); + + void Lock() EXCLUSIVE_UNLOCK_FUNCTION(); + void Unlock() EXCLUSIVE_LOCK_FUNCTION(); +}; + Mutex mu; int x GUARDED_BY(mu); bool c; @@ -3176,6 +3212,17 @@ void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) { scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}} } +Mutex mu2; +int y GUARDED_BY(mu2); + +void variadic() EXCLUSIVE_LOCKS_REQUIRED(mu, mu2) { + VariadicMutexUnlock<Mutex, Mutex> scope(&mu, &mu2); + x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + y = 3; // expected-warning {{writing variable 'y' requires holding mutex 'mu2' exclusively}} + scope.Lock(); + x = y = 4; +} + class SCOPED_LOCKABLE MutexLockUnlock { public: MutexLockUnlock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_UNLOCK_FUNCTION(mu1) EXCLUSIVE_LOCK_FUNCTION(mu2); diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 752803e4a0543..bad71f7cdd07c 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety %s // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++98 %s -// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s -D CPP11 +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s #define LOCKABLE __attribute__ ((lockable)) #define SCOPED_LOCKABLE __attribute__ ((scoped_lockable)) @@ -643,6 +643,24 @@ int elf_function_bad_6(Mutex x, Mutex y) EXCLUSIVE_LOCK_FUNCTION(0); // \ int elf_function_bad_7() EXCLUSIVE_LOCK_FUNCTION(0); // \ // expected-error {{'exclusive_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int elf_template(Mu& mu) EXCLUSIVE_LOCK_FUNCTION(mu) {} + +template int elf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int elf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int elf_variadic_template(Mus&... mus) EXCLUSIVE_LOCK_FUNCTION(mus...) {} + +template int elf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int elf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Shared Lock Function (slf) @@ -719,6 +737,24 @@ int slf_function_bad_6(Mutex x, Mutex y) SHARED_LOCK_FUNCTION(0); // \ int slf_function_bad_7() SHARED_LOCK_FUNCTION(0); // \ // expected-error {{'shared_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int slf_template(Mu& mu) SHARED_LOCK_FUNCTION(mu) {} + +template int slf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int slf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int slf_variadic_template(Mus&... mus) SHARED_LOCK_FUNCTION(mus...) {} + +template int slf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int slf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Exclusive TryLock Function (etf) @@ -796,6 +832,24 @@ int etf_function_bad_5() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoublePointer); // \ int etf_function_bad_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, umu); // \ // expected-warning {{'exclusive_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int etf_template(Mu& mu) EXCLUSIVE_TRYLOCK_FUNCTION(1, mu) {} + +template int etf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int etf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int etf_variadic_template(Mus&... mus) EXCLUSIVE_TRYLOCK_FUNCTION(1, mus...) {} + +template int etf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int etf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Shared TryLock Function (stf) @@ -874,6 +928,24 @@ int stf_function_bad_5() SHARED_TRYLOCK_FUNCTION(1, muDoublePointer); // \ int stf_function_bad_6() SHARED_TRYLOCK_FUNCTION(1, umu); // \ // expected-warning {{'shared_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int stf_template(Mu& mu) SHARED_TRYLOCK_FUNCTION(1, mu) {} + +template int stf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int stf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int stf_variadic_template(Mus&... mus) SHARED_TRYLOCK_FUNCTION(1, mus...) {} + +template int stf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int stf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Unlock Function (uf) @@ -953,6 +1025,24 @@ int uf_function_bad_6(Mutex x, Mutex y) UNLOCK_FUNCTION(0); // \ int uf_function_bad_7() UNLOCK_FUNCTION(0); // \ // expected-error {{'unlock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int uf_template(Mu& mu) UNLOCK_FUNCTION(mu) {} + +template int uf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int uf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int uf_variadic_template(Mus&... mus) UNLOCK_FUNCTION(mus...) {} + +template int uf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int uf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Lock Returned (lr) @@ -1088,6 +1178,23 @@ int le_function_bad_3() LOCKS_EXCLUDED(muDoublePointer); // \ int le_function_bad_4() LOCKS_EXCLUDED(umu); // \ // expected-warning {{'locks_excluded' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int le_template(Mu& mu) LOCKS_EXCLUDED(mu) {} + +template int le_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int le_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int le_variadic_template(Mus&... mus) LOCKS_EXCLUDED(mus...) {} + +template int le_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int le_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif //-----------------------------------------// @@ -1156,6 +1263,24 @@ int elr_function_bad_3() EXCLUSIVE_LOCKS_REQUIRED(muDoublePointer); // \ int elr_function_bad_4() EXCLUSIVE_LOCKS_REQUIRED(umu); // \ // expected-warning {{'exclusive_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int elr_template(Mu& mu) EXCLUSIVE_LOCKS_REQUIRED(mu) {} + +template int elr_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int elr_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int elr_variadic_template(Mus&... mus) EXCLUSIVE_LOCKS_REQUIRED(mus...) {} + +template int elr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int elr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + @@ -1225,6 +1350,24 @@ int slr_function_bad_3() SHARED_LOCKS_REQUIRED(muDoublePointer); // \ int slr_function_bad_4() SHARED_LOCKS_REQUIRED(umu); // \ // expected-warning {{'shared_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int slr_template(Mu& mu) SHARED_LOCKS_REQUIRED(mu) {} + +template int slr_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int slr_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int slr_variadic_template(Mus&... mus) SHARED_LOCKS_REQUIRED(mus...) {} + +template int slr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int slr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Regression tests for unusual cases. @@ -1582,7 +1725,7 @@ class Foo { } // end namespace FunctionAttributesInsideClass_ICE_Test -#ifdef CPP11 +#if __cplusplus >= 201103 namespace CRASH_POST_R301735 { class SomeClass { public: diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 00df99d04d873..b7f7f8f0b2fe2 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -1329,28 +1329,26 @@ namespace { void writeTemplateInstantiationArgs(raw_ostream &OS) const override { OS << "tempInst" << getUpperName() << ", " - << "A->" << getLowerName() << "_size()"; + << "numTempInst" << getUpperName(); } void writeTemplateInstantiation(raw_ostream &OS) const override { - OS << " auto *tempInst" << getUpperName() - << " = new (C, 16) " << getType() - << "[A->" << getLowerName() << "_size()];\n"; + OS << " size_t numTempInst" << getUpperName() << ";\n"; + OS << " " << getType() << "*tempInst" << getUpperName() << ";\n"; OS << " {\n"; OS << " EnterExpressionEvaluationContext " << "Unevaluated(S, Sema::ExpressionEvaluationContext::Unevaluated);\n"; - OS << " " << getType() << " *TI = tempInst" << getUpperName() - << ";\n"; - OS << " " << getType() << " *I = A->" << getLowerName() - << "_begin();\n"; - OS << " " << getType() << " *E = A->" << getLowerName() - << "_end();\n"; - OS << " for (; I != E; ++I, ++TI) {\n"; - OS << " ExprResult Result = S.SubstExpr(*I, TemplateArgs);\n"; - OS << " if (Result.isInvalid())\n"; - OS << " return nullptr;\n"; - OS << " *TI = Result.get();\n"; - OS << " }\n"; + OS << " ArrayRef<" << getType() << "> ArgsToInstantiate(A->" + << getLowerName() << "_begin(), A->" << getLowerName() << "_end());\n"; + OS << " SmallVector<" << getType() << ", 4> InstArgs;\n"; + OS << " if (S.SubstExprs(ArgsToInstantiate, /*IsCall=*/false, " + "TemplateArgs, InstArgs))\n"; + OS << " return nullptr;\n"; + OS << " numTempInst" << getUpperName() << " = InstArgs.size();\n"; + OS << " tempInst" << getUpperName() << " = new (C, 16) " + << getType() << "[numTempInst" << getUpperName() << "];\n"; + OS << " std::copy(InstArgs.begin(), InstArgs.end(), tempInst" + << getUpperName() << ");\n"; OS << " }\n"; } `````````` </details> https://github.com/llvm/llvm-project/pull/137477 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
