diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule AvoidGotoCheck.cpp AvoidNonConstGlobalVariablesCheck.cpp + ComparisonOperatorCheck.cpp CppCoreGuidelinesTidyModule.cpp InitVariablesCheck.cpp InterfacesGlobalInitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.h @@ -0,0 +1,41 @@ +//===--- ComparisonOperatorCheck.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_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// Finds comparison operators which are not noexcept, define as member +/// functions or have parameters of different type. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-comparison-operator.html +class ComparisonOperatorCheck : public ClangTidyCheck { +public: + ComparisonOperatorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void diagCanThrow(const FunctionDecl *FD); +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_COMPARISONOPERATORCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp @@ -0,0 +1,89 @@ +//===--- ComparisonOperatorCheck.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 "ComparisonOperatorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +void ComparisonOperatorCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(functionDecl(hasAnyOverloadedOperatorName( + "==", "!=", "<", "<=", ">", ">=", "<=>")) + .bind("operator"), + this); +} + +static bool canThrow(const FunctionDecl *FD) { + const auto *Proto = FD->getType()->getAs(); + if (isUnresolvedExceptionSpec(Proto->getExceptionSpecType())) + return false; + + return Proto->canThrow() == CT_Can; +} + +static bool hasSameParam(const FunctionDecl *FD) { + constexpr unsigned int ExpectedNumParams = 2; + if (FD->getNumParams() != ExpectedNumParams) + return false; + + QualType LH = FD->getParamDecl(0)->getOriginalType(); + QualType RH = FD->getParamDecl(1)->getOriginalType(); + + return LH == RH; +} + +void ComparisonOperatorCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Operator = Result.Nodes.getNodeAs("operator"); + const FunctionDecl *CanonOperator = Operator->getCanonicalDecl(); + + if (isa(CanonOperator)) { + diag(CanonOperator->getLocation(), "%0 should not be member function") + << CanonOperator; + return; + } + + if (canThrow(CanonOperator)) + diagCanThrow(CanonOperator); + + if (!hasSameParam(CanonOperator)) + diag(CanonOperator->getLocation(), + "%0 should have 2 parameters of the same type") + << CanonOperator; +} + +static SourceLocation getInsertionLoc(const FunctionDecl *FD) { + FunctionTypeLoc FTL = FD->getFunctionTypeLoc(); + if (!FTL) + return SourceLocation(); + + SourceLocation RParenLoc = FTL.getRParenLoc(); + return RParenLoc.getLocWithOffset(1); +} + +void ComparisonOperatorCheck::diagCanThrow(const FunctionDecl *FD) { + diag(FD->getLocation(), "%0 should be marked noexcept") << FD; + + DiagnosticBuilder FixDiag = + diag(FD->getLocation(), "mark 'noexcept'", DiagnosticIDs::Note); + SourceRange ExceptionSpecRange = FD->getExceptionSpecSourceRange(); + SourceLocation InsertionLoc = getInsertionLoc(FD); + + if (ExceptionSpecRange.isValid()) + FixDiag << FixItHint::CreateReplacement(ExceptionSpecRange, "noexcept"); + else if (InsertionLoc.isValid()) + FixDiag << FixItHint::CreateInsertion(InsertionLoc, " noexcept"); +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -16,6 +16,7 @@ #include "../readability/MagicNumbersCheck.h" #include "AvoidGotoCheck.h" #include "AvoidNonConstGlobalVariablesCheck.h" +#include "ComparisonOperatorCheck.h" #include "InitVariablesCheck.h" #include "InterfacesGlobalInitCheck.h" #include "MacroUsageCheck.h" @@ -52,6 +53,8 @@ "cppcoreguidelines-avoid-magic-numbers"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-non-const-global-variables"); + CheckFactories.registerCheck( + "cppcoreguidelines-comparison-operator"); CheckFactories.registerCheck( "cppcoreguidelines-explicit-virtual-functions"); CheckFactories.registerCheck( 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 @@ -83,6 +83,13 @@ Finds ``pthread_setcanceltype`` function calls where a thread's cancellation type is set to asynchronous. +- New :doc:`cppcoreguidelines-comparison-operator + ` check. + + Finds comparison operators(``==``, ``!=``, ``<``, ``<=``, ``>``, ``>=``, ``<=>``) + which are not ``noexcept``, define as member functions or have parameters of + different type. + - New :doc:`cppcoreguidelines-prefer-member-initializer ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-comparison-operator.rst @@ -0,0 +1,36 @@ +.. title:: clang-tidy - cppcoreguidelines-comparison-operator + +cppcoreguidelines-comparison-operator +===================================== + +Finds comparison operators(``==``, ``!=``, ``<``, ``<=``, ``>``, ``>=``, ``<=>``) +which are not ``noexcept``, define as member functions or have parameters of +different type. + +Examples: + +.. code-block:: c++ + + struct A { + int Var; + bool operator==(const A &a) const; // 'operator==' should not be member function + }; + +.. code-block:: c++ + + struct C; + bool operator==(const C &lh, const C &rh); // 'operator==' should be marked noexcept + +.. code-block:: c++ + + struct B; + bool operator==(const B &lh, int rh) noexcept; // 'operator==' should have 2 parameters of the same type + + +The relevant rules in the C++ Core Guidelines are: + +- `C.86: Make == symmetric with respect to operand types and noexcept + `_ + +- `C.161: Use non-member functions for symmetric operators + `_ 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 @@ -142,6 +142,7 @@ `concurrency-thread-canceltype-asynchronous `_, `cppcoreguidelines-avoid-goto `_, `cppcoreguidelines-avoid-non-const-global-variables `_, + `cppcoreguidelines-comparison-operator `_, "Yes" `cppcoreguidelines-init-variables `_, "Yes" `cppcoreguidelines-interfaces-global-init `_, `cppcoreguidelines-macro-usage `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp @@ -0,0 +1,140 @@ +// RUN: %check_clang_tidy -std=c++20 %s cppcoreguidelines-comparison-operator %t -- --fix-notes + +// member function +namespace test_member_function { +struct A { + int Var; + + bool operator==(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator==' should not be member function [cppcoreguidelines-comparison-operator] + + bool operator!=(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator!=' should not be member function [cppcoreguidelines-comparison-operator] + + bool operator<(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator<' should not be member function [cppcoreguidelines-comparison-operator] + + bool operator<=(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator<=' should not be member function [cppcoreguidelines-comparison-operator] + + bool operator>(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator>' should not be member function [cppcoreguidelines-comparison-operator] + + bool operator>=(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator>=' should not be member function [cppcoreguidelines-comparison-operator] + + auto operator<=>(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator<=>' should not be member function [cppcoreguidelines-comparison-operator] +}; +} // namespace test_member_function + +// parameters types differ +namespace test_params_type_differ { +struct B; +bool operator==(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator!=(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator!=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator<(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator<=(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator>(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator>=(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>=' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator<=>(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=>' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +} // namespace test_params_type_differ + +// can throw +namespace test_can_throw { +struct C; +bool operator==(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator==(const C &lh, const C &rh) noexcept + +bool operator!=(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator!=' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator!=(const C &lh, const C &rh) noexcept; + +bool operator<(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator<(const C &lh, const C &rh) noexcept; + +bool operator<=(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator<=(const C &lh, const C &rh) noexcept; + +bool operator>(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator>(const C &lh, const C &rh) noexcept; + +bool operator>=(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator>=' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator>=(const C &lh, const C &rh) noexcept; + +bool operator<=>(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator<=>' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator<=>(const C &lh, const C &rh) noexcept; +} // namespace test_can_throw + +// ok +namespace test_ok { +struct D; +bool operator==(const D &lh, const D &rh) noexcept; +bool operator!=(const D &lh, const D &rh) noexcept; +bool operator<(const D &lh, const D &rh) noexcept; +bool operator<=(const D &lh, const D &rh) noexcept; +bool operator>(const D &lh, const D &rh) noexcept; +bool operator>=(const D &lh, const D &rh) noexcept; +bool operator<=>(const D &lh, const D &rh) noexcept; +} // namespace test_ok + +// only warn on the canonical declaration +namespace test_canonical_declaration { +// member function +struct A { + int Var; + bool operator==(const A &a) const; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'operator==' should not be member function [cppcoreguidelines-comparison-operator] +}; + +bool A::operator==(const A &) const { // Don't warn here + return true; +} + +// parameters types differ +struct B; +bool operator==(const B &lh, int rh) noexcept; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should have 2 parameters of the same type [cppcoreguidelines-comparison-operator] + +bool operator==(const B &lh, int rh) noexcept { // Don't warn here + return true; +} + +// can throw +struct C; +bool operator==(const C &lh, const C &rh); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'operator==' should be marked noexcept [cppcoreguidelines-comparison-operator] +// CHECK-MESSAGES: :[[@LINE-2]]:6: note: mark 'noexcept' +// CHECK-FIXES: bool operator==(const C &lh, const C &rh) noexcept + +bool operator==(const C &lh, const C &rh) { // Don't warn here + return true; +} +} // namespace test_canonical_declaration