[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7bdaff7cad9: [clang-tidy] Implement
bugprone-incorrect-enable-if (authored by ccotter, committed by PiotrZSL).
Changed prior to commit:
https://reviews.llvm.org/D157239?vs=549777=552077#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157239/new/
https://reviews.llvm.org/D157239
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy -std=c++11 %s bugprone-incorrect-enable-if %t
+// RUN: %check_clang_tidy -check-suffix=CXX20 -std=c++20 %s bugprone-incorrect-enable-if %t
+
+// NOLINTBEGIN
+namespace std {
+template struct enable_if { };
+
+template struct enable_if { typedef T type; };
+
+template
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template ::type>
+void valid_function1() {}
+
+template ::type = nullptr>
+void valid_function2() {}
+
+template ::type = nullptr>
+struct ValidClass1 {};
+
+template >
+void invalid() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
+
+template >
+void invalid_extra_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type >
+// CHECK-FIXES-CXX20: template ::type >
+
+template >
+void invalid_extra_no_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
+
+template /*comment3*/>
+void invalid_extra_comment() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type/*comment3*/>
+
+template , typename = std::enable_if>
+void invalid_multiple() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-MESSAGES: [[@LINE-3]]:65: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type, typename = typename std::enable_if::type>
+// CHECK-FIXES-CXX20: template ::type, typename = std::enable_if::type>
+
+template >
+struct InvalidClass {};
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -94,6 +94,7 @@
:doc:`bugprone-implicit-widening-of-multiplication-result `, "Yes"
:doc:`bugprone-inaccurate-erase `, "Yes"
:doc:`bugprone-inc-dec-in-conditions `,
+ :doc:`bugprone-incorrect-enable-if `, "Yes"
:doc:`bugprone-incorrect-roundings `,
:doc:`bugprone-infinite-loop `,
:doc:`bugprone-integer-division `,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-incorrect-enable-if
+
+bugprone-incorrect-enable-if
+
+
+Detects incorrect usages of ``std::enable_if`` that don't name the nested
+``type`` type.
+
+In C++11 introduced ``std::enable_if`` as a convenient way to leverage SFINAE.
+One form of using ``std::enable_if`` is to declare an unnamed template type
+parameter with a default type equal to
+``typename std::enable_if::type``. If the author forgets to name
+the nested type ``type``, then the code will
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
PiotrZSL added a comment. I'm pushing this, if you want to change anything else, just please open new review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
PiotrZSL accepted this revision. PiotrZSL added a comment. This revision is now accepted and ready to land. LGTM (+-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter marked an inline comment as done. ccotter added a comment. > Do you have plans to also detect the bugprone scenario described in the Notes > here? I didn't have plans in this review, or in the immediate future after. I did name this check broadly as "bugprone-incorrect-enable-if," so I imagine this other scenario could go into this new check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:61-62 + + FixItHint TypenameHint = + FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(), "typename "); + FixItHint TypeHint = PiotrZSL wrote: > do we still need typename in C++20 in this context ? > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0634r3.html Oh, I did not know that. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter updated this revision to Diff 549777.
ccotter marked 9 inline comments as done.
ccotter added a comment.
- Fix docs, handle C++20 simplification
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157239/new/
https://reviews.llvm.org/D157239
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy -std=c++11 %s bugprone-incorrect-enable-if %t
+// RUN: %check_clang_tidy -check-suffix=CXX20 -std=c++20 %s bugprone-incorrect-enable-if %t
+
+// NOLINTBEGIN
+namespace std {
+template struct enable_if { };
+
+template struct enable_if { typedef T type; };
+
+template
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template ::type>
+void valid_function1() {}
+
+template ::type = nullptr>
+void valid_function2() {}
+
+template ::type = nullptr>
+struct ValidClass1 {};
+
+template >
+void invalid() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
+
+template >
+void invalid_extra_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type >
+// CHECK-FIXES-CXX20: template ::type >
+
+template >
+void invalid_extra_no_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
+
+template /*comment3*/>
+void invalid_extra_comment() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type/*comment3*/>
+
+template , typename = std::enable_if>
+void invalid_multiple() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-MESSAGES: [[@LINE-3]]:65: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type, typename = typename std::enable_if::type>
+// CHECK-FIXES-CXX20: template ::type, typename = std::enable_if::type>
+
+template >
+struct InvalidClass {};
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+// CHECK-FIXES-CXX20: template ::type>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -94,6 +94,7 @@
`bugprone-implicit-widening-of-multiplication-result `_, "Yes"
`bugprone-inaccurate-erase `_, "Yes"
`bugprone-inc-dec-in-conditions `_,
+ `bugprone-incorrect-enable-if `_, "Yes"
`bugprone-incorrect-roundings `_,
`bugprone-infinite-loop `_,
`bugprone-integer-division `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-incorrect-enable-if
+
+bugprone-incorrect-enable-if
+
+
+Detects incorrect usages of ``std::enable_if`` that don't name the nested
+``type`` type.
+
+In C++11 introduced ``std::enable_if`` as a convenient way to leverage SFINAE.
+One form of using ``std::enable_if`` is to declare an unnamed template type
+parameter with a default type equal to
+``typename std::enable_if::type``. If the author forgets to name
+the nested type ``type``, then the code will always consider the candidate
+template even if the condition is not met.
+
+Below are some examples of code using ``std::enable_if`` correctly and
+incorrect
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:6 + +Detects incorrect usages of std::enable_if that don't name the nested 'type' +type. Please synchronize with statement in Release Notes after Piotr's comment addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
carlosgalvezp added a comment. Do you have plans to also detect the bugprone scenario described in the `Notes` here? https://en.cppreference.com/w/cpp/types/enable_if No need to have it in this patch, but would be good to keep it in mind if we want to add it in the future (preferably) or create a separate check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:67 + diag(EnableIf->getBeginLoc(), "incorrect std::enable_if usage detected; use " +"'typename std::enable_if<...>::type'") + << TypenameHint << TypeHint; PiotrZSL wrote: > since C++14 we should recommend using enable_if_t I didn't add replacement logic for C++14 or C++20 since there are separate tools for those, and though it'd be cleaner to have independent set of tools (e.g., first run bugprone-incorrect-enable-if, then run modernize-type-traits). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157239/new/ https://reviews.llvm.org/D157239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
PiotrZSL added inline comments.
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:57
+
+ const SourceManager = Result.Context->getSourceManager();
+ SourceLocation RAngleLoc =
Result got source manager
https://clang.llvm.org/doxygen/structclang_1_1ast__matchers_1_1MatchFinder_1_1MatchResult.html
Comment at:
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:61-62
+
+ FixItHint TypenameHint =
+ FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(), "typename ");
+ FixItHint TypeHint =
do we still need typename in C++20 in this context ?
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0634r3.html
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp:67
+ diag(EnableIf->getBeginLoc(), "incorrect std::enable_if usage detected; use "
+"'typename std::enable_if<...>::type'")
+ << TypenameHint << TypeHint;
since C++14 we should recommend using enable_if_t
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:129
+
+ Detects incorrect usages of std::enable_if that don't name the nested 'type'
+ type.
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:21
+
+ template ::type>
+ void valid_usage() { ... }
shoudnlt be enable_if ?
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:34
+ template >
+ void invalid_usage() { ... }
+
this is valid usage, uses enable_if_t
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:37
+ // The tool suggests the following replacement for 'invalid_usage':
+ template ::type>
+ void invalid_usage() { ... }
this may not compile, anyway, what if "type" got "type" ?:P
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:44
+`modernize-type-traits <../modernize/type-traits.html>`_ for another tool
+that will replace ``typename std::enable_if<...>::type`` with
+``std::enable_if_t<...>``, and see
should be suffucient
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst:47
+`modernize-use-constraints <../modernize/use-constraints.html>`_ for another
+tool that replaces ``std::enable_if`` with C++20 constraints. Use these
+newer mechanisms where possible.
maybe "Consider"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157239/new/
https://reviews.llvm.org/D157239
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter updated this revision to Diff 547599.
ccotter added a comment.
- Edited wrong file
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157239/new/
https://reviews.llvm.org/D157239
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++11 %s bugprone-incorrect-enable-if %t
+
+// NOLINTBEGIN
+namespace std {
+template struct enable_if { };
+
+template struct enable_if { typedef T type; };
+
+template
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template ::type>
+void valid_function1() {}
+
+template ::type = nullptr>
+void valid_function2() {}
+
+template ::type = nullptr>
+struct ValidClass1 {};
+
+template >
+void invalid() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+
+template >
+void invalid_extra_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type >
+
+template >
+void invalid_extra_no_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+
+template /*comment3*/>
+void invalid_extra_comment() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type/*comment3*/>
+
+template , typename = std::enable_if>
+void invalid_multiple() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-MESSAGES: [[@LINE-3]]:65: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type, typename = typename std::enable_if::type>
+
+template >
+struct InvalidClass {};
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -94,6 +94,7 @@
`bugprone-implicit-widening-of-multiplication-result `_, "Yes"
`bugprone-inaccurate-erase `_, "Yes"
`bugprone-inc-dec-in-conditions `_,
+ `bugprone-incorrect-enable-if `_, "Yes"
`bugprone-incorrect-roundings `_,
`bugprone-infinite-loop `_,
`bugprone-integer-division `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-incorrect-enable-if
+
+bugprone-incorrect-enable-if
+
+
+Detects incorrect usages of std::enable_if that don't name the nested 'type'
+type.
+
+In C++11 introduced ``std::enable_if`` as a convenient way to leverage SFINAE.
+One form of using ``std::enable_if`` is to declare an unnamed template type
+parameter with a default type equal to
+``typename std::enable_if::type``. If the author forgets to name
+the nested type ``type``, then the code will always consider the candidate
+template even if the condition is not met.
+
+Below are some examples of code using ``std::enable_if`` correctly and
+incorrect examples that this check flags.
+
+.. code-block:: c++
+
+ template ::type>
+ void valid_usage() { ... }
+
+ template >
+ void valid_usage_with_trait_helpers() { ... }
+
+ // The below code is not a correct application of SFINAE. Even if
+ // T::some_trait is not true, the function will still be considered in the
+ // set of function candidates. It can either incorrectly select the function
+
[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if
ccotter created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Detects incorrect usages of std::enable_if that don't name the
nested 'type' type.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157239
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/IncDecInConditionsCheck.h
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++11 %s bugprone-incorrect-enable-if %t
+
+// NOLINTBEGIN
+namespace std {
+template struct enable_if { };
+
+template struct enable_if { typedef T type; };
+
+template
+using enable_if_t = typename enable_if::type;
+
+} // namespace std
+// NOLINTEND
+
+template ::type>
+void valid_function1() {}
+
+template ::type = nullptr>
+void valid_function2() {}
+
+template ::type = nullptr>
+struct ValidClass1 {};
+
+template >
+void invalid() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+
+template >
+void invalid_extra_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type >
+
+template >
+void invalid_extra_no_whitespace() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
+
+template /*comment3*/>
+void invalid_extra_comment() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type/*comment3*/>
+
+template , typename = std::enable_if>
+void invalid_multiple() {}
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-MESSAGES: [[@LINE-3]]:65: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type, typename = typename std::enable_if::type>
+
+template >
+struct InvalidClass {};
+// CHECK-MESSAGES: [[@LINE-2]]:23: warning: incorrect std::enable_if usage detected; use 'typename std::enable_if<...>::type' [bugprone-incorrect-enable-if]
+// CHECK-FIXES: template ::type>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -94,6 +94,7 @@
`bugprone-implicit-widening-of-multiplication-result `_, "Yes"
`bugprone-inaccurate-erase `_, "Yes"
`bugprone-inc-dec-in-conditions `_,
+ `bugprone-incorrect-enable-if `_, "Yes"
`bugprone-incorrect-roundings `_,
`bugprone-infinite-loop `_,
`bugprone-integer-division `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-incorrect-enable-if
+
+bugprone-incorrect-enable-if
+
+
+Detects incorrect usages of std::enable_if that don't name the nested 'type'
+type.
+
+In C++11 introduced ``std::enable_if`` as a convenient way to leverage SFINAE.
+One form of using ``std::enable_if`` is to declare an unnamed template type
+parameter with a default type equal to
+``typename std::enable_if::type``. If the author forgets to name
+the nested type ``type``, then the code will always consider the candidate
+template even if the condition is not met.
+
+Below are some examples of code using ``std::enable_if`` correctly and
+incorrect examples that this check flags.
+
+.. code-block:: c++
+
+ template ::type>
+ void valid_usage() { ... }
+
+ template >
+
