This revision was automatically updated to reflect the committed changes. Closed by commit rL370193: [clang-tidy] readability-identifier-naming shouldn't complain about CRTP pseudo… (authored by sammccall, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D66864?vs=217584&id=217610#toc Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66864/new/ https://reviews.llvm.org/D66864 Files: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp @@ -262,6 +262,32 @@ CMyWellNamedClass2<int> x5(42, nullptr); } +class AOverridden { +public: + virtual ~AOverridden() = default; + virtual void BadBaseMethod() = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod' +}; + +class COverriding : public AOverridden { +public: + // Overriding a badly-named base isn't a new violation. + void BadBaseMethod() override {} +}; + +template <typename derived_t> +class CRTPBase { +public: + void BadBaseMethod(int) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'BadBaseMethod' +}; + +class CRTPDerived : CRTPBase<CRTPDerived> { +public: + // Hiding a badly-named base isn't a new violation. + double BadBaseMethod(double) { return 0; } +}; + template<typename T> // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for type template parameter 'T' // CHECK-FIXES: {{^}}template<typename t_t>{{$}} Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -10,6 +10,7 @@ #include "../utils/ASTUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/AST/CXXInheritance.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" @@ -579,6 +580,15 @@ Decl->size_overridden_methods() > 0) return SK_Invalid; + // If this method has the same name as any base method, this is likely + // necessary even if it's not an override. e.g. CRTP. + auto FindHidden = [&](const CXXBaseSpecifier *S, clang::CXXBasePath &P) { + return CXXRecordDecl::FindOrdinaryMember(S, P, Decl->getDeclName()); + }; + CXXBasePaths UnusedPaths; + if (Decl->getParent()->lookupInBases(FindHidden, UnusedPaths)) + return SK_Invalid; + if (Decl->isConstexpr() && NamingStyles[SK_ConstexprMethod]) return SK_ConstexprMethod; Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst @@ -26,6 +26,10 @@ different rules for different kinds of identifiers. In general, the rules are falling back to a more generic rule if the specific case is not configured. +The naming of virtual methods is reported where they occur in the base class, +but not where they are overridden, as it can't be fixed locally there. +This also applies for pseudo-override patterns like CRTP. + Options -------
Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp @@ -262,6 +262,32 @@ CMyWellNamedClass2<int> x5(42, nullptr); } +class AOverridden { +public: + virtual ~AOverridden() = default; + virtual void BadBaseMethod() = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod' +}; + +class COverriding : public AOverridden { +public: + // Overriding a badly-named base isn't a new violation. + void BadBaseMethod() override {} +}; + +template <typename derived_t> +class CRTPBase { +public: + void BadBaseMethod(int) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for method 'BadBaseMethod' +}; + +class CRTPDerived : CRTPBase<CRTPDerived> { +public: + // Hiding a badly-named base isn't a new violation. + double BadBaseMethod(double) { return 0; } +}; + template<typename T> // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for type template parameter 'T' // CHECK-FIXES: {{^}}template<typename t_t>{{$}} Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -10,6 +10,7 @@ #include "../utils/ASTUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/AST/CXXInheritance.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" @@ -579,6 +580,15 @@ Decl->size_overridden_methods() > 0) return SK_Invalid; + // If this method has the same name as any base method, this is likely + // necessary even if it's not an override. e.g. CRTP. + auto FindHidden = [&](const CXXBaseSpecifier *S, clang::CXXBasePath &P) { + return CXXRecordDecl::FindOrdinaryMember(S, P, Decl->getDeclName()); + }; + CXXBasePaths UnusedPaths; + if (Decl->getParent()->lookupInBases(FindHidden, UnusedPaths)) + return SK_Invalid; + if (Decl->isConstexpr() && NamingStyles[SK_ConstexprMethod]) return SK_ConstexprMethod; Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-identifier-naming.rst @@ -26,6 +26,10 @@ different rules for different kinds of identifiers. In general, the rules are falling back to a more generic rule if the specific case is not configured. +The naming of virtual methods is reported where they occur in the base class, +but not where they are overridden, as it can't be fixed locally there. +This also applies for pseudo-override patterns like CRTP. + Options -------
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits