diff --git a/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp @@ -74,9 +74,15 @@ unless(hasAnyParameter( // No warning: enable_if as constructor parameter. parmVarDecl(hasType(isEnableIf())))), - unless(hasParent(functionTemplateDecl(has(templateTypeParmDecl( + unless(hasParent(functionTemplateDecl(anyOf( // No warning: enable_if as type parameter. - hasDefaultArgument(isEnableIf()))))))) + has(templateTypeParmDecl(hasDefaultArgument(isEnableIf()))), + // No warning: enable_if as non-type template parameter. + has(nonTypeTemplateParmDecl( + hasType(isEnableIf()), + anyOf(hasDescendant(cxxBoolLiteral()), + hasDescendant(cxxNullPtrLiteralExpr()), + hasDescendant(integerLiteral()))))))))) .bind("ctor"); Finder->addMatcher(FindOverload, this); } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst @@ -26,9 +26,14 @@ explicit Person(T&& n, int x = 1) {} // C3: perfect forwarding ctor guarded with enable_if - template,void>> + template, void>> explicit Person(T&& n) {} + // C4: variadic perfect forwarding ctor guarded with enable_if + template, A&&...>, int> = 0> + explicit Person(A&&... a) {} + // (possibly compiler generated) copy ctor Person(const Person& rhs); }; @@ -37,13 +42,15 @@ constructors. We suppress warnings if the copy and the move constructors are both disabled (deleted or private), because there is nothing the perfect forwarding constructor could hide in this case. We also suppress warnings for constructors -like C3 that are guarded with an ``enable_if``, assuming the programmer was aware of -the possible hiding. +like C3 and C4 that are guarded with an ``enable_if``, assuming the programmer was +aware of the possible hiding. Background ---------- For deciding whether a constructor is guarded with enable_if, we consider the -default values of the type parameters and the types of the constructor -parameters. If any part of these types is ``std::enable_if`` or ``std::enable_if_t``, -we assume the constructor is guarded. +types of the constructor parameters, the default values of template type parameters +and the types of non-type template parameters with a default literal value. If any +part of these types is ``std::enable_if`` or ``std::enable_if_t``, we assume the +constructor is guarded. + diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp @@ -150,3 +150,93 @@ constexpr variant(_Arg&& __arg) {} // CHECK-NOTES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors }; + +namespace std { +template struct is_same { static constexpr bool value = false; }; +template struct is_same { static constexpr bool value = true; }; +template constexpr bool is_same_v = is_same::value; +template struct remove_reference { using type = T; }; +template struct remove_reference { using type = T; }; +template struct remove_reference { using type = T; }; +template using remove_reference_t = typename remove_reference::type; +template struct remove_cv { using type = T; }; +template struct remove_cv { using type = T; }; +template struct remove_cv { using type = T; }; +template struct remove_cv { using type = T; }; +template using remove_cv_t = typename remove_cv::type; +template struct remove_cvref { using type = remove_cv_t>; }; +template using remove_cvref_t = typename remove_cvref::type; +} // namespace std + +// Handle enable_if when used as a non-type template parameter. +class Test7 { +public: + // Guarded with enable_if. + template , int>, int>::type = 0> + Test7(T &&t); + + // Guarded with enable_if. + template , Test7> + && !std::is_same_v, bool>, int> = true> + Test7(T &&t); + + Test7(const Test7 &other) = default; + Test7(Test7 &&other) = default; +}; + +// Handle enable_if when used as a non-type template parameter following +// a variadic template parameter pack. +class Test8 { +public: + // Guarded with enable_if. + template , Test8> + || (sizeof...(A) > 0)>* = nullptr> + Test8(T &&t, A &&... a); + + Test8(const Test8 &other) = default; + Test8(Test8 &&other) = default; +}; + +// Non-type template parameter failure cases. +class Test9 { +public: + // Requires a default argument (such as a literal, implicit cast expression, etc.) + template , bool>, int>> + Test9(T &&t); + // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + // CHECK-NOTES: 240:3: note: copy constructor declared here + // CHECK-NOTES: 241:3: note: move constructor declared here + + // Requires a default argument (such as a literal, implicit cast expression, etc.) + template , long>>*> + Test9(T &&t); + // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + // CHECK-NOTES: 240:3: note: copy constructor declared here + // CHECK-NOTES: 241:3: note: move constructor declared here + + // Only std::enable_if or std::enable_if_t are supported + template ::type* = nullptr> + Test9(T &&t); + // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + // CHECK-NOTES: 240:3: note: copy constructor declared here + // CHECK-NOTES: 241:3: note: move constructor declared here + + // Only std::enable_if or std::enable_if_t are supported + template ::type = 0> + Test9(T &&t); + // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors + // CHECK-NOTES: 240:3: note: copy constructor declared here + // CHECK-NOTES: 241:3: note: move constructor declared here + + Test9(const Test9 &other) = default; + Test9(Test9 &&other) = default; +};