diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- 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 @@ "bugprone-implicit-widening-of-multiplication-result"); CheckFactories.registerCheck( "bugprone-inaccurate-erase"); + CheckFactories.registerCheck( + "bugprone-incorrect-enable-if"); CheckFactories.registerCheck( "bugprone-switch-missing-default-case"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.h new file mode 100644 --- /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/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/IncorrectEnableIfCheck.cpp new file mode 100644 --- /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, 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("enable_if"); + const auto *ElaboratedLoc = + Result.Nodes.getNodeAs("elaborated"); + const auto *EnableIfSpecializationLoc = + Result.Nodes.getNodeAs( + "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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 + ` check. + + Detects incorrect usages of ``std::enable_if`` that don't name the nested + ``type`` type. + - New :doc:`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 --- /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::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 + // 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 > + void invalid_usage() { ... } + + // The tool suggests the following replacement for 'invalid_usage': + template ::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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/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 `, 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 --- /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 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>