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 <bool B, class T = void> struct enable_if { };
+
+template <class T> struct enable_if<true, T> { typedef T type; };
+
+template <bool B, class T = void>
+using enable_if_t = typename enable_if<B, T>::type;
+
+} // namespace std
+// NOLINTEND
+
+template <typename T, typename = typename std::enable_if<T::some_value>::type>
+void valid_function1() {}
+
+template <typename T, typename std::enable_if<T::some_value>::type = nullptr>
+void valid_function2() {}
+
+template <typename T, typename std::enable_if<T::some_value>::type = nullptr>
+struct ValidClass1 {};
+
+template <typename T, typename = std::enable_if<T::some_value>>
+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 <typename T, typename = typename std::enable_if<T::some_value>::type>
+// CHECK-FIXES-CXX20: template <typename T, typename = std::enable_if<T::some_value>::type>
+
+template <typename T, typename = std::enable_if<T::some_value> >
+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 <typename T, typename = typename std::enable_if<T::some_value>::type >
+// CHECK-FIXES-CXX20: template <typename T, typename = std::enable_if<T::some_value>::type >
+
+template <typename T, typename=std::enable_if<T::some_value>>
+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 <typename T, typename=typename std::enable_if<T::some_value>::type>
+// CHECK-FIXES-CXX20: template <typename T, typename=std::enable_if<T::some_value>::type>
+
+template <typename T, typename /*comment1*/ = /*comment2*/std::enable_if<T::some_value>/*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 <typename T, typename /*comment1*/ = /*comment2*/typename std::enable_if<T::some_value>::type/*comment3*/>
+
+template <typename T, typename = std::enable_if<T::some_value>, typename = std::enable_if<T::other_value>>
+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 <typename T, typename = typename std::enable_if<T::some_value>::type, typename = typename std::enable_if<T::other_value>::type>
+// CHECK-FIXES-CXX20: template <typename T, typename = std::enable_if<T::some_value>::type, typename = std::enable_if<T::other_value>::type>
+
+template <typename T, typename = std::enable_if<T::some_value>>
+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 <typename T, typename = typename std::enable_if<T::some_value>::type>
+// CHECK-FIXES-CXX20: template <typename T, typename = std::enable_if<T::some_value>::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 <bugprone/implicit-widening-of-multiplication-result.html>`_, "Yes"
`bugprone-inaccurate-erase <bugprone/inaccurate-erase.html>`_, "Yes"
`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions.html>`_,
+ `bugprone-incorrect-enable-if <bugprone/incorrect-enable-if.html>`_, "Yes"
`bugprone-incorrect-roundings <bugprone/incorrect-roundings.html>`_,
`bugprone-infinite-loop <bugprone/infinite-loop.html>`_,
`bugprone-integer-division <bugprone/integer-division.html>`_,
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<condition>::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 <typename T, typename = typename std::enable_if<T::some_trait>::type>
+ void valid_usage() { ... }
+
+ template <typename T, typename = std::enable_if_t<T::some_trait>>
+ 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
+ // when it should not be a candidates, and/or lead to hard compile errors
+ // if the body of the template does not compile if the condition is not
+ // satisfied.
+ template <typename T, typename = std::enable_if<T::some_trait>>
+ void invalid_usage() { ... }
+
+ // The tool suggests the following replacement for 'invalid_usage':
+ template <typename T, typename = typename std::enable_if<T::some_trait>::type>
+ void fixed_invalid_usage() { ... }
+
+C++14 introduced the trait helper ``std::enable_if_t`` which reduces the
+likelihood of this error. C++20 introduces constraints, which generally
+supersede the use of ``std::enable_if``. See
+`modernize-type-traits <../modernize/type-traits.html>`_ for another tool
+that will replace ``std::enable_if`` with
+``std::enable_if_t``, and see
+`modernize-use-constraints <../modernize/use-constraints.html>`_ for another
+tool that replaces ``std::enable_if`` with C++20 constraints. Consider these
+newer mechanisms where possible.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,6 +123,12 @@
a complex condition and suggests moving them outside to avoid ambiguity in
the variable's value.
+- New :doc:`bugprone-incorrect-enable-if
+ <clang-tidy/checks/bugprone/incorrect-enable-if>` check.
+
+ Detects incorrect usages of ``std::enable_if`` that don't name the nested
+ ``type`` type.
+
- New :doc:`bugprone-multi-level-implicit-pointer-conversion
<clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check.
Index: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
@@ -0,0 +1,34 @@
+//===--- IncorrectEnableIfCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLEIFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLEIFCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects incorrect usages of ``std::enable_if`` that don't name the nested
+/// ``type`` type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-enable-if.html
+class IncorrectEnableIfCheck : public ClangTidyCheck {
+public:
+ IncorrectEnableIfCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLEIFCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
@@ -0,0 +1,71 @@
+//===--- IncorrectEnableIfCheck.cpp - clang-tidy --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "IncorrectEnableIfCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(TemplateTypeParmDecl, hasUnnamedDefaultArgument,
+ ast_matchers::internal::Matcher<TypeLoc>, InnerMatcher) {
+ if (Node.getIdentifier() != nullptr || !Node.hasDefaultArgument() ||
+ Node.getDefaultArgumentInfo() == nullptr)
+ return false;
+
+ TypeLoc DefaultArgTypeLoc = Node.getDefaultArgumentInfo()->getTypeLoc();
+ return InnerMatcher.matches(DefaultArgTypeLoc, Finder, Builder);
+}
+
+} // namespace
+
+void IncorrectEnableIfCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ templateTypeParmDecl(
+ hasUnnamedDefaultArgument(
+ elaboratedTypeLoc(
+ hasNamedTypeLoc(templateSpecializationTypeLoc(
+ loc(qualType(hasDeclaration(namedDecl(
+ hasName("::std::enable_if"))))))
+ .bind("enable_if_specialization")))
+ .bind("elaborated")))
+ .bind("enable_if"),
+ this);
+}
+
+void IncorrectEnableIfCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *EnableIf =
+ Result.Nodes.getNodeAs<TemplateTypeParmDecl>("enable_if");
+ const auto *ElaboratedLoc =
+ Result.Nodes.getNodeAs<ElaboratedTypeLoc>("elaborated");
+ const auto *EnableIfSpecializationLoc =
+ Result.Nodes.getNodeAs<TemplateSpecializationTypeLoc>(
+ "enable_if_specialization");
+
+ if (!EnableIf || !ElaboratedLoc || !EnableIfSpecializationLoc)
+ return;
+
+ const SourceManager &SM = *Result.SourceManager;
+ SourceLocation RAngleLoc =
+ SM.getExpansionLoc(EnableIfSpecializationLoc->getRAngleLoc());
+
+ auto Diag = diag(EnableIf->getBeginLoc(),
+ "incorrect std::enable_if usage detected; use "
+ "'typename std::enable_if<...>::type'");
+ if (!getLangOpts().CPlusPlus20) {
+ Diag << FixItHint::CreateInsertion(ElaboratedLoc->getBeginLoc(),
+ "typename ");
+ }
+ Diag << FixItHint::CreateInsertion(RAngleLoc.getLocWithOffset(1), "::type");
+}
+
+} // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -22,6 +22,7 @@
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
+ IncorrectEnableIfCheck.cpp
SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectRoundingsCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -28,6 +28,7 @@
#include "ImplicitWideningOfMultiplicationResultCheck.h"
#include "InaccurateEraseCheck.h"
#include "IncDecInConditionsCheck.h"
+#include "IncorrectEnableIfCheck.h"
#include "IncorrectRoundingsCheck.h"
#include "InfiniteLoopCheck.h"
#include "IntegerDivisionCheck.h"
@@ -122,6 +123,8 @@
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck<InaccurateEraseCheck>(
"bugprone-inaccurate-erase");
+ CheckFactories.registerCheck<IncorrectEnableIfCheck>(
+ "bugprone-incorrect-enable-if");
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
"bugprone-switch-missing-default-case");
CheckFactories.registerCheck<IncDecInConditionsCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
