[PATCH] D12031: Const std::move() argument ClangTidy check

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:30
@@ +29,3 @@
+    bool IsVariable = dyn_cast<DeclRefExpr>(Arg) != nullptr;
+    std::string message = "std::move of the ";
+    message += IsConstArg ? "const " : "";
----------------
alexfh wrote:
> Please don't string += as a way to build messages. This creates a temporary each time and reallocates the string buffer. Use one of the these:
>   
>   std::string message = (llvm::Twine("...") + "..." + "...").str()
> 
> (only in a single expression, i.e. don't create variables of the llvm::Twine type, as this can lead to dangling string references), or:
> 
>   std::string buffer;
>   llvm::raw_string_ostream message(buffer);
>   message << "...";
>   message << "...";
>   // then use message.str() where you would use an std::string.
> 
> The second alternative would be more suitable for this kind of code.
> 
> BUT, even this is not needed in the specific case of producing diagnostics, as clang provides a powerful template substitution mechanism, and your code could be written more effectively:
> 
>   diag("std::move of the %select{const |}0 %select{variable|expression}1 ...") << IsConstArg << IsVariable << ...;
This should read:
"Please don't use string += as a way to build messages. This creates a temporary each time and reallocates the string buffer. Instead, use one of these patterns:"


http://reviews.llvm.org/D12031