njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:188
+ callee(functionDecl(hasName("::std::format")))),
+ hasAnyArgument(materializeTemporaryExpr(
+ has(StringCStrCallExpr))))),
----------------
mikecrowe wrote:
> njames93 wrote:
> > The limitation about only transforming the first argument can be alleviated
> > by using the `forEachArgumentWithParam` matcher
> Thanks for the hint. I did try to overcome this limitation but failed to find
> `forEachArgumentWithParam`.
>
> I think that I've managed to make `forEachArgumentWithParam` work, but only
> if I lose the `materializeTemporaryExpr`. I hope that's not essential. I've
> ended up with:
>
> ```C++
> Finder->addMatcher(
>
>
> traverse(TK_AsIs,
>
>
> callExpr(callee(functionDecl(hasAnyName("::std::print",
> "::std::format"))),
>
> forEachArgumentWithParam(StringCStrCallExpr,
> parmVarDecl()))),
>
> this);
>
>
> ```
> I suspect that there's something better than the `parmVarDecl()` second
> parameter to `forEachArgumentWithParam` though.
`parmVarDecl` is exactly what you need here.
My understanding of the temporary expressions isn't perfect, but from what I
can gather, we shouldn't need temporaries when calling the `.c_str()`method for
all but the first argument.
This does highlight an issue with your test code though.
`std::format` and `std::print` take a `std::format_string` as the first
argument, but in your tests you have it as `const char *`
Anyway I think this means the first argument in real world code would result in
a `materializeTemporaryExpr`
See [[ https://en.cppreference.com/w/cpp/utility/format/basic_format_string |
here ]]
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143342/new/
https://reviews.llvm.org/D143342
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
