Commit 06f3dabe authored by Mitchell Balan's avatar Mitchell Balan
Browse files

[clang-tidy] Fix readability-redundant-string-init for c++17/c++2a 

Summary:
`readability-redundant-string-init` was one of several clang-tidy checks documented as failing for C++17. (The failure mode in C++17 is that it changes `std::string Name = ""`; to `std::string Name = Name;`, which actually compiles but crashes at run-time.)

Analyzing the AST with `clang -Xclang -ast-dump` showed that the outer `CXXConstructExprs` that previously held the correct SourceRange were being elided in C++17/2a, but the containing `VarDecl` expressions still had all the relevant information. So this patch changes the fix to get its source ranges from `VarDecl`.

It adds one test `std::string g = "u", h = "", i = "uuu", j = "", k;` to confirm proper warnings and fixit replacements in a single `DeclStmt` where some strings require replacement and others don't. The readability-redundant-string-init.cpp and readability-redundant-string-init-msvc.cpp tests now pass for C++11/14/17/2a.

Reviewers: gribozavr, etienneb, alexfh, hokein, aaron.ballman, gribozavr2

Patch by: poelmanc

Subscribers: NoQ, MyDeveloperDay, Eugene.Zelenko, dylanmckay, cfe-commits

Tags: #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D69238
parent 90535744

clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp

+8 −4
Original line number Diff line number Diff line
@@ -20,11 +20,13 @@ namespace modernize { 
UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context)

    : ClangTidyCheck(Name, Context),

      IgnoreDestructors(Options.get("IgnoreDestructors", false)),

      AllowOverrideAndFinal(Options.get("AllowOverrideAndFinal", false)),

      OverrideSpelling(Options.get("OverrideSpelling", "override")),

      FinalSpelling(Options.get("FinalSpelling", "final")) {}



void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);

  Options.store(Opts, "AllowOverrideAndFinal", AllowOverrideAndFinal);

  Options.store(Opts, "OverrideSpelling", OverrideSpelling);

  Options.store(Opts, "FinalSpelling", FinalSpelling);

}
@@ -103,7 +105,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) { 
  bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal;

  unsigned KeywordCount = HasVirtual + HasOverride + HasFinal;



  if (!OnlyVirtualSpecified && KeywordCount == 1)

  if ((!OnlyVirtualSpecified && KeywordCount == 1) ||

      (!HasVirtual && HasOverride && HasFinal && AllowOverrideAndFinal))

    return; // Nothing to do.



  std::string Message;
@@ -113,7 +116,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) { 
    Message = "annotate this function with '%0' or (rarely) '%1'";

  } else {

    StringRef Redundant =

        HasVirtual ? (HasOverride && HasFinal ? "'virtual' and '%0' are"

        HasVirtual ? (HasOverride && HasFinal && !AllowOverrideAndFinal

                          ? "'virtual' and '%0' are"

                          : "'virtual' is")

                   : "'%0' is";

    StringRef Correct = HasFinal ? "'%1'" : "'%0'";
@@ -211,7 +215,7 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) { 
    Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);

  }



  if (HasFinal && HasOverride) {

  if (HasFinal && HasOverride && !AllowOverrideAndFinal) {

    SourceLocation OverrideLoc = Method->getAttr<OverrideAttr>()->getLocation();

    Diag << FixItHint::CreateRemoval(

        CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));

clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h

+1 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ public: 


private:

  const bool IgnoreDestructors;

  const bool AllowOverrideAndFinal;

  const std::string OverrideSpelling;

  const std::string FinalSpelling;

};

clang-tools-extra/docs/ReleaseNotes.rst

+6 −0
Original line number Diff line number Diff line
@@ -164,6 +164,12 @@ Improvements to clang-tidy 
  Finds non-static member functions that can be made ``const``

  because the functions don't use ``this`` in a non-const way.



- Improved :doc:`modernize-use-override

  <clang-tidy/checks/modernize-use-override>` check.



  The check now supports the ``AllowOverrideAndFinal`` option to eliminate

  conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.



Improvements to include-fixer

-----------------------------

clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst

+8 −0
Original line number Diff line number Diff line
@@ -27,6 +27,14 @@ Options 


   If set to non-zero, this check will not diagnose destructors. Default is `0`.



.. option:: AllowOverrideAndFinal



   If set to non-zero, this check will not diagnose ``override`` as redundant

   with ``final``. This is useful when code will be compiled by a compiler with

   warning/error checking flags requiring ``override`` explicitly on overriden

   members, such as ``gcc -Wsuggest-override``/``gcc -Werror=suggest-override``.

   Default is `0`.



.. option:: OverrideSpelling



   Specifies a macro to use instead of ``override``. This is useful when