[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently
Logan Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 8 13:46:28 PST 2021
logan-5 added inline comments.
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36
+ OS.flush();
+ return Str;
}
----------------
Quuxplusone wrote:
> FWIW, it appears to me that in most (all?) of these cases, what's really wanted is not "a string //and// a stream" but rather "a stream that owns a string" (`std::ostringstream` or the LLVM-codebase equivalent thereof). Then the return can be `return std::move(OS).str();` — for `std::ostringstream`, this Does The Right Thing since C++20, and if LLVM had its own stringstream it could make it Do The Right Thing today.
> https://en.cppreference.com/w/cpp/io/basic_ostringstream/str
>
True enough. Although `return std::move(OS).str();` is still much harder to type than the less efficient `return OS.str();`, and it requires at minimum a move of the underlying string--whereas `return Str;` is the easiest of all to type, and it opens things up for NRVO. If (as I said in the patch summary) `raw_string_ostream` were changed to be guaranteed to not need flushing, `return Str;` would IMHO be cemented as the clear winner.
That said, you're clearly right that all these cases are semantically trying to do "a stream that owns a string", and it's clunky to execute with the existing APIs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115374/new/
https://reviews.llvm.org/D115374
More information about the cfe-commits
mailing list