njames93 marked 2 inline comments as done.
njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:58
+ }
+ if (B.isInvalid() || E.isInvalid())
+ return llvm::None;
----------------
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
================
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())),
----------------
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
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
