[PATCH] D72448: [clang-tidy] readability-redundant-string-init now flags redundant initialisation in Field Decls and Constructor Initialisers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:58
+  }
+  if (B.isInvalid() || E.isInvalid())
+    return llvm::None;
----------------
njames93 wrote:
> aaron.ballman wrote:
> > Should we be worried about macros here?
> > 
> > It looks a bit like we're ignoring macros entirely for this check, so maybe that can be done as a separate patch instead. The situation I am worried about is:
> > ```
> > #if SOMETHING
> > #define INIT ""
> > #else
> > #define INIT "haha"
> > #endif
> > 
> > std::string S = INIT;
> > ```
> I feel that everything in this check needs to check macros, but that's probably a follow up patch
Okay, separate patch it is.


================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:110
       namedDecl(
-          varDecl(
-              hasType(hasUnqualifiedDesugaredType(recordType(
-                  hasDeclaration(cxxRecordDecl(hasStringTypeName))))),
-              hasInitializer(expr(ignoringImplicit(anyOf(
-                  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)))))
-              .bind("vardecl"),
+          varDecl(StringType, hasInitializer(EmptyStringInit)).bind("vardecl"),
           unless(parmVarDecl())),
----------------
njames93 wrote:
> aaron.ballman wrote:
> > Should this also match on something like `std::string foo{};` as a redundant init? Similar question for the other cases. (Could be done in a follow-up patch if desired.)
> Probably should, however this is not what this patch is about
Okay, I'm fine with that as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72448/new/

https://reviews.llvm.org/D72448