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