mikecrowe marked an inline comment as done.
mikecrowe added a comment.

> 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?


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

Reply via email to