[PATCH] D143342: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr

[Nathan James via Phabricator via cfe-commits]

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