sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66864
Files:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp
Index: clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp
+++ clang-tools-extra/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/docs/clang-tidy/checks/readability-identifier-naming.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/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/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/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/test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/readability-identifier-naming.cpp
+++ clang-tools-extra/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/docs/clang-tidy/checks/readability-identifier-naming.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/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/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
