[clang-tools-extra] a7bdaff - [clang-tidy] Implement bugprone-incorrect-enable-if
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 21 10:39:09 PDT 2023
Author: Chris Cotter
Date: 2023-08-21T17:38:30Z
New Revision: a7bdaff7cad93e96ee9aaf3dc8b2a46c84717361
URL: https://github.com/llvm/llvm-project/commit/a7bdaff7cad93e96ee9aaf3dc8b2a46c84717361
DIFF: https://github.com/llvm/llvm-project/commit/a7bdaff7cad93e96ee9aaf3dc8b2a46c84717361.diff
LOG: [clang-tidy] Implement bugprone-incorrect-enable-if
Detects incorrect usages of std::enable_if that don't name the
nested 'type' type.
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D157239
Added:
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a52bb91df0ca8f..a67a91eedd1048 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/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 @@ class BugproneModule : public ClangTidyModule {
"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>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index a9e62cce4d9df9..3c768021feb150 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -22,6 +22,7 @@ add_clang_library(clangTidyBugproneModule
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
+ IncorrectEnableIfCheck.cpp
SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectRoundingsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp
new file mode 100644
index 00000000000000..09aaf3e31d5dd7
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h
new file mode 100644
index 00000000000000..37a52b425aa807
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a3c614a787ec19..6b910920f6b7b6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,6 +123,12 @@ New checks
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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-if.rst
new file mode 100644
index 00000000000000..a7860a96e3081b
--- /dev/null
+++ b/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
+:doc:`modernize-type-traits <../modernize/type-traits>` for another tool
+that will replace ``std::enable_if`` with
+``std::enable_if_t``, and see
+:doc:`modernize-use-constraints <../modernize/use-constraints>` for another
+tool that replaces ``std::enable_if`` with C++20 constraints. Consider these
+newer mechanisms where possible.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 9eaf86a292b746..75a86431d26441 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -94,6 +94,7 @@ Clang-Tidy Checks
:doc:`bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result>`, "Yes"
:doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
:doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
+ :doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
:doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
:doc:`bugprone-infinite-loop <bugprone/infinite-loop>`,
:doc:`bugprone-integer-division <bugprone/integer-division>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-if.cpp
new file mode 100644
index 00000000000000..6ebd97fabd524f
--- /dev/null
+++ b/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>
More information about the cfe-commits
mailing list