[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 29 08:18:04 PDT 2020


njames93 added a comment.

In D84831#2181336 <https://reviews.llvm.org/D84831#2181336>, @gribozavr2 wrote:

>> Passed test cases but failed in the real world as std::string has a non trivial destructor so creates a CXXBindTemporaryExpr.
>
> An idea for a future change: move the std::string mock from this test into a header that is shared across all tests that need a std::string. That will hopefully allow us to combine forces when curating the standard library mocks.

That does sound like a good plan, could also add maybe vector

Also a future change for this specific check would be to replace the call to `c_str()` when its being bound to an r value with an explicit copy constructor call

   void foo(std::string &&);
   void bar(const std::string& S) {
  -  foo(S.c_str());
  +  foo(std::string(S));
   }

This would be more of a performance change though, basically save a call to strlen.
Though arguably it is slightly more readable, as you are explicitly saying you want to pass a copy of the string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84831



More information about the cfe-commits mailing list