ymandel added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:29-33
+static ast_matchers::internal::Matcher<NamedDecl>
+hasAnyNameStdString(std::vector<std::string> Names) {
+ return ast_matchers::internal::Matcher<NamedDecl>(
+ new ast_matchers::internal::HasNameMatcher(std::move(Names)));
+}
----------------
`hasAnyName` is a `VariadicMatcher`, so it supports passing a list directly (as
a single argument), see ASTMatchersInternal.h, line 103.
Given that, I'm pretty sure this function is unnecessary; you can just call
`hasAnyName` directly below instead of this function.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:112
cxxConstructExpr(
- hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+ hasDeclaration(cxxMethodDecl(hasStringCtorName)),
hasArgument(0, hasType(CharPtrType)),
----------------
Following up on Aaron's comment above, I think you want something like this:
`hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(hasAnyName(removeNamespaces(StringNames))))`
================
Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:136
+ anyOf(
+ // do not match std::string(ptr, int)
+ // match std::string(ptr, alloc)
----------------
nit: I'd put this comment line before the `anyOf` since it describes the whole
thing, while the following comments describe each branch, respectively.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91015/new/
https://reviews.llvm.org/D91015
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
