[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D144216#4135395 <https://reviews.llvm.org/D144216#4135395>, @mikecrowe wrote:

>> If you're suggesting that I could use the new <string> header to replace declarations of basic_string etc. in other tests then I think that would be possible with some careful checking to make sure it include the necessary functionality. I think that would easier to review separately rather than in a single patch though.
>
> I had a quick look at likely candidates:
>
> - `abseil/redundant-strcat-calls.cpp` appears to declare `basic_string` outside the `std` namespace and inside it, and does some strange stuff with a base class. If I rip all that out, and replace uses of `string` with `std::string` then the tests pass using `<string>`.
> - `readability/string-compare.cpp` and `readability/container-data-pointer.cpp` require some tweaks to `<string>` but are straightforward.
> - There may be complications if MSVC differs in its `std::basic_string` implementation in ways that the `-msvc` tests care about, but I didn't spot any.
>
> So, it looks like the use of this new `<string>` header could be extended. Do you have a preference for one big patch, one patch per directory (abseil, readability), or one patch per check? Do you require all to be complete before even this patch can be accepted?

If it's possible to re-use this new string header into the other checks that would be ideal. After all, in the real world there's only one `string` header. If it's a drop-in replacement I think it's fine to do multiple checks in one patch. Otherwise if the check / string header needs tweak it's probably better to do one check at a time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144216/new/

https://reviews.llvm.org/D144216