aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:58
+  }
+  if (B.isInvalid() || E.isInvalid())
+    return llvm::None;
----------------
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;
```


================
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())),
----------------
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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72448



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to