[NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

[logan-5]

Summary

Returning OS.str() is guaranteed to copy the underlying string, whereas OS.flush(); return Underlying; makes Underlying a candidate for NRVO, or at worst, implicit move.

To keep this kind of inefficiency at bay in the future, the fast code should probably be made easier to type than the slow code (as it's currently the opposite). Perhaps this could be solved by:

• making raw_string_ostream guarantee+document that it does not need to be flush()ed (similar to raw_svector_ostream), and/or
• making raw_string_ostream::str() return a StringRef rather than std::string&, to discourage using it to initialize std::strings

Implementing those two ideas would make simply return Underlying; the natural, correct, and efficient choice. They are, however, out of scope for this patch.

[• Quuxplusone]

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

36

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

[logan-5]

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

36

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.

[dexonsmith]

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.

clang/lib/AST/DeclarationName.cpp

236–240

For trivial cases like this, maybe it'd be worth creating a helper function that gets reused. Maybe called llvm::raw_string_ostream::toString():

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:

return llvm::raw_string_ostream::toString(*this);

[dexonsmith]

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

36

If (as I said in the patch summary) raw_string_ostream were changed to be guaranteed to not need flushing

This sounds like a one-line patch; might be better to just do it rather than having to churn all these things twice.

[logan-5]

In D115374#3181383, @dexonsmith wrote:

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().

I definitely hear you. I don't really mind it personally, and I did it this way because I saw precedent in a couple spots (there's one on CompilerInvocation.cpp:653, several in clangd, etc.). I definitely see how it could be a little bit spooky though.

std::string Str;
llvm::raw_string_ostream(Str) << ...;
return Str;

I like this idea. I'd be happy to go back through and change the simple ones to this pattern.

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.

I suppose the question then becomes whether to name it take() or str() && for symmetry with C++20's std::ostringstream. (Also for the record, I agree that NRVOing some std::strings isn't going to make a giant difference; my opinion is simply that if we can get it, we might as well.)

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

36

If (as I said in the patch summary) raw_string_ostream were changed to be guaranteed to not need flushing

This sounds like a one-line patch; might be better to just do it rather than having to churn all these things twice.

I guess this change kind of freaks me out. Currently you can call SetBuffered() on raw_string_ostream (though I don't know why you would...), which creates an intermediate buffer and then flush() syncs the buffer with the underlying std::string&. Removing that ability would be a breaking change, and I'm not sure how we could make it while being confident we're not breaking anything downstream.

(On the other hand, you can call SetBuffered() on raw_svector_ostream too, whose documentation more or less says it doesn't support buffering. If I'm reading right, you get an assert failure in ~raw_ostream() if you do.)

[logan-5]

Eliminate some explicit .flush()es by using temporary raw_string_ostreams where possible.

[dexonsmith]

In D115374#3181491, @logan-5 wrote:

In D115374#3181383, @dexonsmith wrote:

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().

I definitely hear you. I don't really mind it personally, and I did it this way because I saw precedent in a couple spots (there's one on CompilerInvocation.cpp:653, several in clangd, etc.). I definitely see how it could be a little bit spooky though.

I haven't done the git-blame, but I somewhat suspect str() was added specifically as a convenience to make the flush+access/return a one-liner.

I suppose the question then becomes whether to name it take() or str() && for symmetry with C++20's std::ostringstream. (Also for the record, I agree that NRVOing some std::strings isn't going to make a giant difference; my opinion is simply that if we can get it, we might as well.)

I think take() would be clearer for use in LLVM code, despite the symmetry. Especially since the two stream libraries have significant differences. (Also, maybe I'm mis-remembering, but I think I've seen places where OS.str() is passed to some function when it's partially written; seems dangerous to silent start selecting a different overload. Could be I'm wrong though.)

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

36

I don't think we need to be afraid. raw_ostream seems like a bit of a leaky abstraction, but IMO we need to trust that code doesn't arbitrarily call SetBuffered() on without full control. It's essentially never the right thing to do except from subclasses (where it could be protected) and maybe in unit tests for convenience. As you pointed, out this would also break svector_stream.

What I'd be more worried about is the micro-performance-penalty of std::string null-terminating with every write. But probably that's not particularly important either.

[logan-5]

To be clear, it sounds like we should either add .take() for moving the string out of raw_string_ostream's string reference, or make raw_string_ostream not need to be flushed (after which there won't be as clear a use for .take(), since you can just use the underlying string directly).

I vote for the second option, making raw_string_ostream not need to be flushed, since it allows for simpler code at the call site (return Str;), permits NRVO at the call site, and avoids some possibly weird questions .take() would bring with it (like whether it would ever be surprising that it moves out of a reference that someone else might also have).

If that's the direction that sounds best, I can submit an updated patch sometime tomorrow.

[dexonsmith]

In D115374#3181670, @logan-5 wrote:

To be clear, it sounds like we should either add .take() for moving the string out of raw_string_ostream's string reference, or make raw_string_ostream not need to be flushed (after which there won't be as clear a use for .take(), since you can just use the underlying string directly).

I vote for the second option, making raw_string_ostream not need to be flushed, since it allows for simpler code at the call site (return Str;), permits NRVO at the call site, and avoids some possibly weird questions .take() would bring with it (like whether it would ever be surprising that it moves out of a reference that someone else might also have).

If that's the direction that sounds best, I can submit an updated patch sometime tomorrow.

Yeah, probably one or the other. I'm leaning toward the no-flush approach as well, but I'd suggest making that a separate prep patch to reduce churn/dependencies in case there's some reason it needs to be reverted (e.g., compile time regression). Removing the no-longer-needed .str()s in a follow-up a few days later (rebasing this without the flushes) should be trivial.

[dexonsmith]

Returning from https://reviews.llvm.org/D115421, it looks like that raw_string_ostream has unbuffered by default since 65b13610a5226b84889b923bae884ba395ad084d. Seems like all the new flush()s in this patch are no-ops. You can simplify this patch to just mechanically converting return OS.str() to return S.

[logan-5]

Removed .flush()es. Seems like this might be able to land without https://reviews.llvm.org/D115421?

[dexonsmith]

In D115374#3183776, @logan-5 wrote:

Removed .flush()es. Seems like this might be able to land without https://reviews.llvm.org/D115421?

Yup! LGTM if you remove the cleanups that are no longer related to this patch, and/or split them into separate follow-up NFC commits. Might be nice to split even the main commit into a few pieces (e.g., by functional area) to push separately, just in case something needs to be reverted due to a call to SetBuffered() in an unexpected place.

Please make sure the commit message points at 65b13610a5226b84889b923bae884ba395ad084d, which made this correct.

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

73–74

Given there's no scope change needed, these cleanups are no longer related to the patch and are adding noise to the diff. I suggest committing them separately (or not at all).

clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h

 class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
 private:
   ASTContext &ACtx;
 
   std::string printStmt(const Stmt *S) {
     std::string Str;
     llvm::raw_string_ostream OS(Str);
     S->printPretty(OS, nullptr, PrintingPolicy(ACtx.getLangOpts()));
-    return OS.str();
+    OS.flush();
+    return Str;
   }
 
   bool isThisObject(const SymbolicRegion *R) {
     if (auto S = dyn_cast<SymbolRegionValue>(R->getSymbol()))
       if (isa<CXXThisRegion>(S->getRegion()))
         return true;
     return false;
   }
std::string VisitLocMemRegionVal(loc::MemRegionVal V) {
     return "pointer to " + Visit(R);
   }
 
   std::string VisitLocConcreteInt(loc::ConcreteInt V) {
     const llvm::APSInt &I = V.getValue();
     std::string Str;
     llvm::raw_string_ostream OS(Str);
     OS << "concrete memory address '" << I << "'";
-    return OS.str();
+    OS.flush();
+    return Str;
   }
 
   std::string VisitNonLocSymbolVal(nonloc::SymbolVal V) {
     return Visit(V.getSymbol());
   }
 
   std::string VisitNonLocConcreteInt(nonloc::ConcreteInt V) {
     const llvm::APSInt &I = V.getValue();
     std::string Str;
     llvm::raw_string_ostream OS(Str);
     OS << (I.isSigned() ? "signed " : "unsigned ") << I.getBitWidth()
        << "-bit integer '" << I << "'";
-    return OS.str();
+    OS.flush();
+    return Str;
   }
 
   std::string VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal V) {
     return "lazily frozen compound value of " + Visit(V.getRegion());
   }
 
   std::string VisitSymbolRegionValue(const SymbolRegionValue *S) {
     const MemRegion *R = S->getRegion();
public:
   }
 
   std::string VisitSymIntExpr(const SymIntExpr *S) {
     std::string Str;
     llvm::raw_string_ostream OS(Str);
     OS << "(" << Visit(S->getLHS()) << ") "
        << std::string(BinaryOperator::getOpcodeStr(S->getOpcode())) << " "
        << S->getRHS();
-    return OS.str();
+    OS.flush();
+    return Str;
   }
 
   // TODO: IntSymExpr doesn't appear in practice.
   // Add the relevant code once it does.
 
   std::string VisitSymSymExpr(const SymSymExpr *S) {
     return "(" + Visit(S->getLHS()) + ") " +
            std::string(BinaryOperator::getOpcodeStr(S->getOpcode())) +
std::string VisitElementRegion(const ElementRegion *R) {
     OS << "element of type '" << R->getElementType().getAsString()
        << "' with index ";
     // For concrete index: omit type of the index integer.
     if (auto I = R->getIndex().getAs<nonloc::ConcreteInt>())
       OS << I->getValue();
     else
       OS << "'" << Visit(R->getIndex()) << "'";
     OS << " of " + Visit(R->getSuperRegion());
-    return OS.str();
+    OS.flush();
+    return Str;
   }
 
   std::string VisitNonParamVarRegion(const NonParamVarRegion *R) {
     const VarDecl *VD = R->getDecl();
     std::string Name = VD->getQualifiedNameAsString();
     if (isa<ParmVarDecl>(VD))
       return "parameter '" + Name + "'";
     else if (VD->hasAttr<BlocksAttr>())

clang/include/clang/Testing/TestClangConfig.h

std::vector<std::string> getCommandLineArgs() const {
     Result.push_back(Target);
     return Result;
   }
 
   std::string toString() const {
     std::string Result;
     llvm::raw_string_ostream OS(Result);
     OS << "{ Language=" << Language << ", Target=" << Target << " }";
-    return OS.str();
+    OS.flush();
+    return Result;
   }
 
   friend std::ostream &operator<<(std::ostream &OS,
                                   const TestClangConfig &ClangConfig) {
     return OS << ClangConfig.toString();
   }
 };
 
 } // end namespace clang
 
 #endif

clang/lib/AST/ASTDiagnostic.cpp

if (!Repeated) {
     // type and element count.
     if (const auto *VTy = Ty->getAs<VectorType>()) {
       std::string DecoratedString;
       llvm::raw_string_ostream OS(DecoratedString);
       const char *Values = VTy->getNumElements() > 1 ? "values" : "value";
       OS << "'" << S << "' (vector of " << VTy->getNumElements() << " '"
          << VTy->getElementType().getAsString(Context.getPrintingPolicy())
          << "' " << Values << ")";
-      return OS.str();
+      OS.flush();
+      return DecoratedString;
     }
   }
 
   S = "'" + S + "'";
   return S;
 }
 
 static bool FormatTemplateTypeDiff(ASTContext &Context, QualType FromType,

clang/lib/AST/AttrImpl.cpp

else if (state == FixedWidth || state == ScalableWidth) {
     OS << "enable";
   else if (state == Full)
     OS << "full";
   else if (state == AssumeSafety)
     OS << "assume_safety";
   else
     OS << "disable";
   OS << ")";
-  return OS.str();
+  OS.flush();
+  return ValueName;
 }
 
 // Return a string suitable for identifying this attribute in diagnostics.
 std::string
 LoopHintAttr::getDiagnosticName(const PrintingPolicy &Policy) const {
   unsigned SpellingIndex = getAttributeSpellingListIndex();
   if (SpellingIndex == Pragma_nounroll)
     return "#pragma nounroll";

clang/lib/AST/Decl.cpp

 void NamedDecl::printName(raw_ostream &os) const {
   os << Name;
 }
 
 std::string NamedDecl::getQualifiedNameAsString() const {
   std::string QualName;
   llvm::raw_string_ostream OS(QualName);
   printQualifiedName(OS, getASTContext().getPrintingPolicy());
-  return OS.str();
+  OS.flush();
+  return QualName;
 }
 
 void NamedDecl::printQualifiedName(raw_ostream &OS) const {
   printQualifiedName(OS, getASTContext().getPrintingPolicy());
 }
 
 void NamedDecl::printQualifiedName(raw_ostream &OS,
                                    const PrintingPolicy &P) const {

clang/lib/AST/DeclarationName.cpp

bool DeclarationName::isDependentName() const {
   auto *TD = getCXXDeductionGuideTemplate();
   if (TD && TD->getDeclContext()->isDependentContext())
     return true;
 
   return false;
 }
 
 std::string DeclarationName::getAsString() const {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << *this;
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 void *DeclarationName::getFETokenInfoSlow() const {
   switch (getNameKind()) {
   case Identifier:
     llvm_unreachable("case Identifier already handled by getFETokenInfo!");
   case CXXConstructorName:
   case CXXDestructorName:
bool DeclarationNameInfo::isInstantiationDependent() const {
   }
   llvm_unreachable("All name kinds handled.");
 }
 
 std::string DeclarationNameInfo::getAsString() const {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << *this;
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 raw_ostream &clang::operator<<(raw_ostream &OS, DeclarationNameInfo DNInfo) {
   LangOptions LO;
   DNInfo.printName(OS, PrintingPolicy(LangOptions()));
   return OS;
 }
 

clang/lib/AST/OpenMPClause.cpp

for (const OMPTraitSelector &Selector : Set.Selectors) {
         continue;
 
       for (const OMPTraitProperty &Property : Selector.Properties)
         OS << '$' << 'P'
            << getOpenMPContextTraitPropertyName(Property.Kind,
                                                 Property.RawString);
     }
   }
-  return OS.str();
+  OS.flush();
+  return MangledName;
 }
 
 OMPTraitInfo::OMPTraitInfo(StringRef MangledName) {
   unsigned long U;
   do {
     if (!MangledName.consume_front("$S"))
       break;
     if (MangledName.consumeInteger(10, U))

clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp

for (size_t i = 0, e = Errors.size(); i != e; ++i) {
     printErrorContentToStream(Errors[i], OS);
   }
 }
 
 std::string Diagnostics::toString() const {
   std::string S;
   llvm::raw_string_ostream OS(S);
   printToStream(OS);
-  return OS.str();
+  OS.flush();
+  return S;
 }
 
 void Diagnostics::printToStreamFull(llvm::raw_ostream &OS) const {
   for (size_t i = 0, e = Errors.size(); i != e; ++i) {
     if (i != 0) OS << "\n";
     const ErrorContent &Error = Errors[i];
     for (size_t i = 0, e = Error.ContextStack.size(); i != e; ++i) {
       printContextFrameToStream(Error.ContextStack[i], OS);
       OS << "\n";
     }
     printErrorContentToStream(Error, OS);
   }
 }
 
 std::string Diagnostics::toStringFull() const {
   std::string S;
   llvm::raw_string_ostream OS(S);
   printToStreamFull(OS);
-  return OS.str();
+  OS.flush();
+  return S;
 }
 
 }  // namespace dynamic
 }  // namespace ast_matchers
 }  // namespace clang

clang/lib/Analysis/AnalysisDeclContext.cpp

if (const auto *OID = dyn_cast<ObjCImplementationDecl>(DC)) {
            << OC->getIdentifier()->getNameStart() << ')';
       }
     } else if (const auto *OCD = dyn_cast<ObjCCategoryImplDecl>(DC)) {
       OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() << ')';
     }
     OS << ' ' << OMD->getSelector().getAsString() << ']';
   }
 
-  return OS.str();
+  OS.flush();
+  return Str;
 }
 
 LocationContextManager &AnalysisDeclContext::getLocationContextManager() {
   assert(
       ADCMgr &&
       "Cannot create LocationContexts without an AnalysisDeclContextManager!");
   return ADCMgr->getLocationContextManager();
 }

clang/lib/Basic/SourceLocation.cpp

void SourceLocation::print(raw_ostream &OS, const SourceManager &SM)const{
   OS << '>';
 }
 
 LLVM_DUMP_METHOD std::string
 SourceLocation::printToString(const SourceManager &SM) const {
   std::string S;
   llvm::raw_string_ostream OS(S);
   print(OS, SM);
-  return OS.str();
+  OS.flush();
+  return S;
 }
 
 LLVM_DUMP_METHOD void SourceLocation::dump(const SourceManager &SM) const {
   print(llvm::errs(), SM);
   llvm::errs() << '\n';
 }
 
 LLVM_DUMP_METHOD void SourceRange::dump(const SourceManager &SM) const {
void SourceRange::print(raw_ostream &OS, const SourceManager &SM) const {
   OS << '>';
 }
 
 LLVM_DUMP_METHOD std::string
 SourceRange::printToString(const SourceManager &SM) const {
   std::string S;
   llvm::raw_string_ostream OS(S);
   print(OS, SM);
-  return OS.str();
+  OS.flush();
+  return S;
 }
 
 //===----------------------------------------------------------------------===//
 // FullSourceLoc
 //===----------------------------------------------------------------------===//
 
 FileID FullSourceLoc::getFileID() const {
   assert(isValid());

clang/lib/Basic/Version.cpp

std::string getClangFullRepositoryVersion() {
   std::string LLVMRev = getLLVMRevision();
   if (!LLVMRev.empty() && LLVMRev != Revision) {
     OS << " (";
     std::string LLVMRepo = getLLVMRepositoryPath();
     if (!LLVMRepo.empty())
       OS << LLVMRepo << ' ';
     OS << LLVMRev << ')';
   }
-  return OS.str();
+  OS.flush();
+  return buf;
 }
 
 std::string getClangFullVersion() {
   return getClangToolFullVersion("clang");
 }
 
 std::string getClangToolFullVersion(StringRef ToolName) {
   std::string buf;
   llvm::raw_string_ostream OS(buf);
 #ifdef CLANG_VENDOR
   OS << CLANG_VENDOR;
 #endif
   OS << ToolName << " version " CLANG_VERSION_STRING;
 
   std::string repo = getClangFullRepositoryVersion();
   if (!repo.empty()) {
     OS << " " << repo;
   }
 
-  return OS.str();
+  OS.flush();
+  return buf;
 }
 
 std::string getClangFullCPPVersion() {
   // The version string we report in __VERSION__ is just a compacted version of
   // the one we report on the command line.
   std::string buf;
   llvm::raw_string_ostream OS(buf);
 #ifdef CLANG_VENDOR
   OS << CLANG_VENDOR;
 #endif
   OS << "Clang " CLANG_VERSION_STRING;
 
   std::string repo = getClangFullRepositoryVersion();
   if (!repo.empty()) {
     OS << " " << repo;
   }
 
-  return OS.str();
+  OS.flush();
+  return buf;
 }
 
 } // end namespace clang

clang/lib/Frontend/CompilerInvocation.cpp

 }
 
 static std::string serializeXRayInstrumentationBundle(const XRayInstrSet &S) {
   llvm::SmallVector<StringRef, 2> BundleParts;
   serializeXRayInstrValue(S, BundleParts);
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
   llvm::interleave(BundleParts, OS, [&OS](StringRef Part) { OS << Part; }, ",");
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 // Set the profile kind using fprofile-instrument-use-path.
 static void setPGOUseInstrumentor(CodeGenOptions &Opts,
                                   const Twine &ProfileName) {
   auto ReaderOrErr = llvm::IndexedInstrProfReader::create(ProfileName);
   // In error, return silently and let Clang PGOUse report the error message.
   if (auto E = ReaderOrErr.takeError()) {

clang/lib/Frontend/TestModuleFileExtension.cpp

return std::unique_ptr<ModuleFileExtensionReader>(
                                                     new TestModuleFileExtension::Reader(this, Stream));
 }
 
 std::string TestModuleFileExtension::str() const {
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
   OS << BlockName << ":" << MajorVersion << ":" << MinorVersion << ":" << Hashed
      << ":" << UserInfo;
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }

clang/lib/Rewrite/HTMLRewrite.cpp

case '\t':
 
       break;
 
     case '<': os << "&lt;"; break;
     case '>': os << "&gt;"; break;
     case '&': os << "&amp;"; break;
     }
   }
-
-  return os.str();
+  os.flush();
+  return Str;
 }
 
 static void AddLineNumber(RewriteBuffer &RB, unsigned LineNo,
                           unsigned B, unsigned E) {
   SmallString<256> Str;
   llvm::raw_svector_ostream OS(Str);
 
   OS << "<tr class=\"codeline\" data-linenumber=\"" << LineNo << "\">"

clang/lib/Sema/CodeCompleteConsumer.cpp

for (const Chunk &C : *this) {
     case CK_CurrentParameter:
       OS << "<#" << C.Text << "#>";
       break;
     default:
       OS << C.Text;
       break;
     }
   }
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 const char *CodeCompletionString::getTypedText() const {
   for (const Chunk &C : *this)
     if (C.Kind == CK_TypedText)
       return C.Text;
 
   return nullptr;
for (auto &C : CCS) {
     case CodeCompletionString::CK_Optional:
       break;
 
     default:
       OS << C.Text;
       break;
     }
   }
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 void PrintingCodeCompleteConsumer::ProcessOverloadCandidates(
     Sema &SemaRef, unsigned CurrentArg, OverloadCandidate *Candidates,
     unsigned NumCandidates, SourceLocation OpenParLoc) {
   OS << "OPENING_PAREN_LOC: ";
   OpenParLoc.print(OS, SemaRef.getSourceManager());
   OS << "\n";

clang/lib/Sema/SemaAttr.cpp

 attrMatcherRuleListToString(ArrayRef<attr::SubjectMatchRule> Rules) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   for (const auto &I : llvm::enumerate(Rules)) {
     if (I.index())
       OS << (I.index() == Rules.size() - 1 ? ", and " : ", ");
     OS << "'" << attr::getSubjectMatchRuleSpelling(I.value()) << "'";
   }
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 } // end anonymous namespace
 
 void Sema::ActOnPragmaAttributeAttribute(
     ParsedAttr &Attribute, SourceLocation PragmaLoc,
     attr::ParsedSubjectMatchRuleSet Rules) {
   Attribute.setIsPragmaClangAttribute();

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

if (hasFuchsiaAttr<AcquireHandleAttr>(FuncDecl)) {
     SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
     Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
       auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
       if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) {
         std::string SBuf;
         llvm::raw_string_ostream OS(SBuf);
         OS << "Function '" << FuncDecl->getDeclName()
            << "' returns an open handle";
-        return OS.str();
+        OS.flush();
+        return SBuf;
       } else
         return "";
     });
     State =
         State->set<HStateMap>(RetSym, HandleState::getMaybeAllocated(nullptr));
   } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(FuncDecl)) {
     // Function returns an unowned handle
     SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
     Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
       auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
       if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) {
         std::string SBuf;
         llvm::raw_string_ostream OS(SBuf);
         OS << "Function '" << FuncDecl->getDeclName()
            << "' returns an unowned handle";
-        return OS.str();
+        OS.flush();
+        return SBuf;
       } else
         return "";
     });
     State = State->set<HStateMap>(RetSym, HandleState::getUnowned());
   }
 
   for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
     if (Arg >= FuncDecl->getNumParams())
for (SymbolRef Handle : Handles) {
         } else {
           Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
             auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
             if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
               std::string SBuf;
               llvm::raw_string_ostream OS(SBuf);
               OS << "Handle released through " << ParamDiagIdx
                  << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
-              return OS.str();
+              OS.flush();
+              return SBuf;
             } else
               return "";
           });
           State = State->set<HStateMap>(Handle, HandleState::getReleased());
         }
       } else if (hasFuchsiaAttr<AcquireHandleAttr>(PVD)) {
         Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
           auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
           if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
             std::string SBuf;
             llvm::raw_string_ostream OS(SBuf);
             OS << "Handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
-            return OS.str();
+            OS.flush();
+            return SBuf;
           } else
             return "";
         });
         State = State->set<HStateMap>(
             Handle, HandleState::getMaybeAllocated(ResultSymbol));
       } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(PVD)) {
         Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
           auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
           if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
             std::string SBuf;
             llvm::raw_string_ostream OS(SBuf);
             OS << "Unowned handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
-            return OS.str();
+            OS.flush();
+            return SBuf;
           } else
             return "";
         });
         State = State->set<HStateMap>(Handle, HandleState::getUnowned());
       } else if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
                  PVD->getType()->isIntegerType()) {
         // Working around integer by-value escapes.
         // The by-value escape would not be captured in checkPointerEscape.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

std::string HTMLDiagnostics::GenerateHTML(const PathDiagnostic& D, Rewriter &R,
   const FileEntry* Entry = SMgr.getFileEntryForID(FID);
   FinalizeHTML(D, R, SMgr, path, FileIDs[0], Entry, declName);
 
   std::string file;
   llvm::raw_string_ostream os(file);
   for (auto BI : *Buf)
     os << BI;
 
-  return os.str();
+  os.flush();
+  return file;
 }
 
 void HTMLDiagnostics::dumpCoverageData(
     const PathDiagnostic &D,
     const PathPieces &path,
     llvm::raw_string_ostream &os) {
 
   const FilesToLineNumsMap &ExecutedLines = D.getExecutedLines();
<form>
     <input type="checkbox" name="showArrows"
            id="showArrows" style="margin-left: 10px" />
     <label for="showArrows">
        Show control flow arrows
     </label>
 </form>
 )<<<";
 
-  return os.str();
+  os.flush();
+  return s;
 }
 
 void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
     const SourceManager& SMgr, const PathPieces& path, FileID FID,
     const FileEntry *Entry, const char *declName) {
   // This is a cludge; basically we want to append either the full
   // working directory if we have no directory information.  This is
   // a work in progress.
const arrowIndices = )<<<";
   R.InsertTextBefore(R.getSourceMgr().getLocForStartOfFile(BugFileID),
                      OS.str());
 }
 
 std::string getSpanBeginForControl(const char *ClassName, unsigned Index) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   OS << "<span id=\"" << ClassName << Index << "\">";
-  return OS.str();
+  OS.flush();
+  return Result;
 }
 
 std::string getSpanBeginForControlStart(unsigned Index) {
   return getSpanBeginForControl("start", Index);
 }
 
 std::string getSpanBeginForControlEnd(unsigned Index) {
   return getSpanBeginForControl("end", Index);

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

 LLVM_DUMP_METHOD void MemRegion::dump() const {
   dumpToStream(llvm::errs());
 }
 
 std::string MemRegion::getString() const {
   std::string s;
   llvm::raw_string_ostream os(s);
   dumpToStream(os);
-  return os.str();
+  os.flush();
+  return s;
 }
 
 void MemRegion::dumpToStream(raw_ostream &os) const {
   os << "<Unknown Region>";
 }
 
 void AllocaRegion::dumpToStream(raw_ostream &os) const {
   os << "alloca{S" << Ex->getID(getContext()) << ',' << Cnt << '}';

clang/lib/Tooling/Syntax/Tokens.cpp

for (auto &M : File.Mappings) {
           M.EndSpelled == File.SpelledTokens.size()
               ? "<eof>"
               : PrintToken(File.SpelledTokens[M.EndSpelled]),
           M.EndSpelled, PrintToken(ExpandedTokens[M.BeginExpanded]),
           M.BeginExpanded, PrintToken(ExpandedTokens[M.EndExpanded]),
           M.EndExpanded);
     }
   }
-  return OS.str();
+  OS.flush();
+  return Dump;
 }

clang/lib/Tooling/Syntax/Tree.cpp

std::string syntax::Node::dumpTokens(const SourceManager &SM) const {
   std::string Storage;
   llvm::raw_string_ostream OS(Storage);
   traverse(this, [&](const syntax::Node *N) {
     if (const auto *L = dyn_cast<syntax::Leaf>(N)) {
       dumpLeaf(OS, L, SM);
       OS << " ";
     }
   });
-  return OS.str();
+  OS.flush();
+  return Storage;
 }
 
 void syntax::Node::assertInvariants() const {
 #ifndef NDEBUG
   if (isDetached())
     assert(getParent() == nullptr);
   else
     assert(getParent() != nullptr);

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

static std::string OptLLVM(const std::string &IR, CodeGenOpt::Level OLvl) {
 
   // Add a pass that writes the optimized IR to an output stream
   std::string outString;
   raw_string_ostream OS(outString);
   Passes.add(createPrintModulePass(OS, "", false));
 
   Passes.run(*M);
 
-  return OS.str();
+  OS.flush();
+  return outString;
 }
 
 // Takes a function and runs it on a set of inputs
 // First determines whether f is the optimized or unoptimized function
 static void RunFuncOnInputs(LLVMFunc f, int Arr[kNumArrays][kArraySize]) {
   for (int i = 0; i < kNumArrays / 3; i++)
     f(Arr[i], Arr[i + (kNumArrays / 3)], Arr[i + (2 * kNumArrays / 3)],
       kArraySize);

clang/unittests/AST/ASTTraverserTest.cpp

template <typename... NodeType> std::string dumpASTString(NodeType &&... N) {
   llvm::raw_string_ostream OS(Buffer);
 
   TestASTDumper Dumper(OS);
 
   OS << "\n";
 
   Dumper.Visit(std::forward<NodeType &&>(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 template <typename... NodeType>
 std::string dumpASTString(TraversalKind TK, NodeType &&... N) {
   std::string Buffer;
   llvm::raw_string_ostream OS(Buffer);
 
   TestASTDumper Dumper(OS);
   Dumper.SetTraversalKind(TK);
 
   OS << "\n";
 
   Dumper.Visit(std::forward<NodeType &&>(N)...);
 
-  return OS.str();
+  OS.flush();
+  return Buffer;
 }
 
 const FunctionDecl *getFunctionNode(clang::ASTUnit *AST,
                                     const std::string &Name) {
   auto Result = ast_matchers::match(functionDecl(hasName(Name)).bind("fn"),
                                     AST->getASTContext());
   EXPECT_EQ(Result.size(), 1u);
   return Result[0].getNodeAs<FunctionDecl>("fn");

clang/unittests/Analysis/MacroExpansionContextTest.cpp

SourceLocation Loc =
         SourceMgr.translateLineCol(SourceMgr.getMainFileID(), row, col);
     return SourceMgr.getExpansionLoc(Loc);
   }
 
   static std::string dumpExpandedTexts(const MacroExpansionContext &Ctx) {
     std::string Buf;
     llvm::raw_string_ostream OS{Buf};
     Ctx.dumpExpandedTextsToStream(OS);
-    return OS.str();
+    OS.flush();
+    return Buf;
   }
 
   static std::string dumpExpansionRanges(const MacroExpansionContext &Ctx) {
     std::string Buf;
     llvm::raw_string_ostream OS{Buf};
     Ctx.dumpExpansionRangesToStream(OS);
-    return OS.str();
+    OS.flush();
+    return Buf;
   }
 };
 
 TEST_F(MacroExpansionContextTest, IgnoresPragmas) {
   // No-crash during lexing.
   const auto Ctx = getMacroExpansionContextFor(R"code(
   _Pragma("pack(push, 1)")
   _Pragma("pack(pop, 1)")

clang/unittests/Tooling/Syntax/TreeTest.cpp

dumpElementsAndDelimiters(ArrayRef<List::ElementAndDelimiter<Node>> EDs) {
     llvm::interleaveComma(
         EDs, OS, [&OS, this](const List::ElementAndDelimiter<Node> &ED) {
           OS << "(" << dumpQuotedTokensOrNull(ED.element) << ", "
              << dumpQuotedTokensOrNull(ED.delimiter) << ")";
         });
 
     OS << "]";
 
-    return OS.str();
+    OS.flush();
+    return Storage;
   }
 
   std::string dumpNodes(ArrayRef<Node *> Nodes) {
     std::string Storage;
     llvm::raw_string_ostream OS(Storage);
 
     OS << "[";
 
     llvm::interleaveComma(Nodes, OS, [&OS, this](const Node *N) {
       OS << dumpQuotedTokensOrNull(N);
     });
 
     OS << "]";
 
-    return OS.str();
+    OS.flush();
+    return Storage;
   }
 };
 
 INSTANTIATE_TEST_SUITE_P(TreeTests, ListTest,
                         ::testing::ValuesIn(allTestClangConfigs()) );
 
 /// "a, b, c"  <=> [("a", ","), ("b", ","), ("c", null)]
 TEST_P(ListTest, List_Separated_WellFormed) {