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

Mike Crowe via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 5 09:13:27 PST 2023


mikecrowe added a comment.

In D144216#4169772 <https://reviews.llvm.org/D144216#4169772>, @carlosgalvezp wrote:

> In D144216#4169764 <https://reviews.llvm.org/D144216#4169764>, @mikecrowe wrote:
>
>>> I will double check that this is true once my current build is complete.
>>
>> Yes, it's true. I stuck a `#error` in `clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` and I saw the expected error from a file including `<string>`.
>
> Awesome, thanks for checking, I wasn't aware we already had `string.h`. Then if we do have headers named the same as the standard headers, would it make sense to name this header `cstring` instead of `string`?

I think you mean rename `string.h` to `cstring`. (`string` clearly has to be just `string` if it's the standard C++ `string` header.)

A bit of searching in the existing checks shows that `string.h` is included by C checks (`bugprone/signal-handler*.c`) and also by checks that deliberately want `string.h` so they can suggest switching to `cstring` (`modernize/deprecated-headers*`) so it looks like it's necessary to keep `string.h`. If you wish, a `cstring` wrapper that just includes `string.h` could be created, but it really ought to put everything in `namespace std` too. (If so, I think this would probably be better done as a separate change.)


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

https://reviews.llvm.org/D144216



More information about the cfe-commits mailing list