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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
