[clang-tidy] modernize-use-override new option AllowOverrideAndFinal

Summary:
In addition to adding `override` wherever possible, clang-tidy's `modernize-use-override` nicely removes `virtual` when `override` or `final` is specified, and further removes override when final is specified. While this is great default behavior, when code needs to be compiled with gcc at high warning levels that include `gcc -Wsuggest-override` or `gcc -Werror=suggest-override`, clang-tidy's removal of the redundant `override` keyword causes gcc to emit a warning or error. This discrepancy / conflict has been noted by others including a comment on Stack Overflow and by Mozilla's Firefox developers.

This patch adds an AllowOverrideAndFinal option defaulting to 0 - thus preserving current behavior - that when enabled allows both `override` and `final` to co-exist, while still fixing all other issues.

The patch includes a test file verifying all combinations of virtual/override/final, and mentions the new option in the release notes.

Reviewers: alexfh, djasper, JonasToth

Patch by: poelmanc

Subscribers: JonasToth, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70165

1 files changed, 13 insertions, 13 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
index 9a9a3830201..a140e17fabf 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -46,23 +46,23 @@ void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) {
   //     string bar("");
   Finder->addMatcher(
       namedDecl(
-          varDecl(hasType(hasUnqualifiedDesugaredType(recordType(
-                      hasDeclaration(cxxRecordDecl(hasName("basic_string")))))),
-                  hasInitializer(expr(ignoringImplicit(anyOf(
-                                          EmptyStringCtorExpr,
-                                          EmptyStringCtorExprWithTemporaries)))
-                                     .bind("expr"))),
-          unless(parmVarDecl()))
-          .bind("decl"),
+          varDecl(
+              hasType(hasUnqualifiedDesugaredType(recordType(
+                  hasDeclaration(cxxRecordDecl(hasName("basic_string")))))),
+              hasInitializer(expr(ignoringImplicit(anyOf(
+                  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)))))
+              .bind("vardecl"),
+          unless(parmVarDecl())),
       this);
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CtorExpr = Result.Nodes.getNodeAs<Expr>("expr");
-  const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl");
-  diag(CtorExpr->getExprLoc(), "redundant string initialization")
-      << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
-                                      Decl->getName());
+  const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl");
+  // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+  // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+  SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+  diag(VDecl->getLocation(), "redundant string initialization")
+      << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
 }
 
 } // namespace readability