[PATCH] D12031: Const std::move() argument ClangTidy check
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 3 13:43:52 PST 2015
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
More information about the cfe-commits
mailing list