Commit 50e99563 authored by Mitchell Balan's avatar Mitchell Balan
Browse files

[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
parent ee0882bd
Loading
Loading
Loading
Loading
+13 −13
Original line number Diff line number Diff line
@@ -46,23 +46,23 @@ void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) {
  //     string bar("");
  Finder->addMatcher(
      namedDecl(
          varDecl(hasType(hasUnqualifiedDesugaredType(recordType(
          varDecl(
              hasType(hasUnqualifiedDesugaredType(recordType(
                  hasDeclaration(cxxRecordDecl(hasName("basic_string")))))),
              hasInitializer(expr(ignoringImplicit(anyOf(
                                          EmptyStringCtorExpr,
                                          EmptyStringCtorExprWithTemporaries)))
                                     .bind("expr"))),
          unless(parmVarDecl()))
          .bind("decl"),
                  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
+1 −2
Original line number Diff line number Diff line
// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
// FIXME: Fix the checker to work in C++17 mode.
// RUN: %check_clang_tidy %s readability-redundant-string-init %t

namespace std {
template <typename T>
+6 −2
Original line number Diff line number Diff line
// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
// FIXME: Fix the checker to work in C++17 mode.
// RUN: %check_clang_tidy %s readability-redundant-string-init %t

namespace std {
template <typename T>
@@ -131,6 +130,11 @@ void k() {
  // CHECK-FIXES: std::string a, b, c;

  std::string d = "u", e = "u", f = "u";

  std::string g = "u", h = "", i = "uuu", j = "", k;
  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: redundant string initialization
  // CHECK-MESSAGES: [[@LINE-2]]:43: warning: redundant string initialization
  // CHECK-FIXES: std::string g = "u", h, i = "uuu", j, k;
}

// These cases should not generate warnings.