dexonsmith added a comment.
I don't feel strongly, but IMO the code might be a bit harder to read/maintain
with the explicit flush. I worry that it'd be easy to move the `flush()` away
from the `return`. Not sure I'm right; could just be familiarity with `str()`.
Have you considered other options? Two below.
One option:
std::string Result;
{
llvm::raw_string_ostream OS(Str);
OS << ...;
}
return Result;
which in branch-free cases could shorten to:
std::string Str;
llvm::raw_string_ostream(Str) << ...;
return Str;
I personally find the lifetime a bit more obvious than the explicit flush call.
Another option:
std::string Result;
llvm::raw_string_ostream OS(Str);
OS << ...;
return OS.take();
Where `raw_string_ostream::take()` is just `return std::move(str())`. It
doesn't get NRVO, but I'm not sure that really matters in most of these places.
Benefit is that it's a minimal change and the name is clear / matches other
LLVM things.
================
Comment at: clang/lib/AST/DeclarationName.cpp:236-240
std::string Result;
llvm::raw_string_ostream OS(Result);
OS << *this;
- return OS.str();
+ OS.flush();
+ return Result;
----------------
For trivial cases like this, maybe it'd be worth creating a helper function
that gets reused. Maybe called `llvm::raw_string_ostream::toString()`:
```
lang=c++
template <class T>
std::string raw_string_ostream::toString(const T &V) {
std::string Result;
raw_string_ostream(Result) << V;
return Result;
}
```
and the code here would be:
```
lang=c++
return llvm::raw_string_ostream::toString(*this);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115374/new/
https://reviews.llvm.org/D115374
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
