carlosgalvezp added a comment. In D144216#4169781 <https://reviews.llvm.org/D144216#4169781>, @mikecrowe wrote:
> 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.) You are absolutely right, I don't know why I mixed `cstring` with `string`, nevermind! Pushed now, will look at the follow up patches soon. Thanks again for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144216/new/ https://reviews.llvm.org/D144216 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
