salman-javed-nz added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:1260
if (Decl->isMain() || !Decl->isUserProvided() ||
- Decl->size_overridden_methods() > 0)
+ Decl->size_overridden_methods() > 0 || Decl->hasAttr<OverrideAttr>())
return SK_Invalid;
----------------
Adding a call to `hasAttr<OverrideAttr>()` looks OK to me -- this is in line
with [[ https://clang.llvm.org/doxygen/ASTMatchers_8h_source.html#l06002 |
AST_MATCHER(CXXMethodDecl, isOverride)]]. Other clang-tidy checks have done the
same thing.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:282-294
class AOverridden {
public:
virtual ~AOverridden() = default;
virtual void BadBaseMethod() = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual
method 'BadBaseMethod'
// CHECK-FIXES: {{^}} virtual void v_Bad_Base_Method() = 0;
};
----------------
How about a test to check that diagnostics are generated even if the `override`
keyword is omitted.
This challenges the `Decl->size_overridden_methods() > 0` portion of the `if`
statement.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:321
+ // (note that there could be specializations of the template base
class, though)
+ void BadBaseMethod() override{}
+};
----------------
What should happen if the base method is overridden without the `override`
keyword?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113830/new/
https://reviews.llvm.org/D113830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
