logan-5 created this revision.
logan-5 added a reviewer: aaron.ballman.
logan-5 added a project: clang.
Herald added subscribers: abrachet, ctetreau, dexonsmith, martong.
logan-5 requested review of this revision.
Herald added a subscriber: cfe-commits.
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::string`s
Implementing those two ideas would make simply `return Underlying;` the
natural, correct, and efficient choice. They are, however, out of scope for
this patch.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115374
Files:
clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
clang/include/clang/Testing/TestClangConfig.h
clang/lib/AST/ASTDiagnostic.cpp
clang/lib/AST/AttrImpl.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclarationName.cpp
clang/lib/AST/OpenMPClause.cpp
clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
clang/lib/Analysis/AnalysisDeclContext.cpp
clang/lib/Basic/SourceLocation.cpp
clang/lib/Basic/Version.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Frontend/TestModuleFileExtension.cpp
clang/lib/Rewrite/HTMLRewrite.cpp
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/lib/Tooling/Syntax/Tokens.cpp
clang/lib/Tooling/Syntax/Tree.cpp
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
clang/unittests/AST/ASTTraverserTest.cpp
clang/unittests/Analysis/MacroExpansionContextTest.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp
Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===================================================================
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -203,7 +203,8 @@
OS << "]";
- return OS.str();
+ OS.flush();
+ return Storage;
}
std::string dumpNodes(ArrayRef<Node *> Nodes) {
@@ -218,7 +219,8 @@
OS << "]";
- return OS.str();
+ OS.flush();
+ return Storage;
}
};
Index: clang/unittests/Analysis/MacroExpansionContextTest.cpp
===================================================================
--- clang/unittests/Analysis/MacroExpansionContextTest.cpp
+++ clang/unittests/Analysis/MacroExpansionContextTest.cpp
@@ -95,14 +95,16 @@
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;
}
};
Index: clang/unittests/AST/ASTTraverserTest.cpp
===================================================================
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -111,7 +111,8 @@
Dumper.Visit(std::forward<NodeType &&>(N)...);
- return OS.str();
+ OS.flush();
+ return Buffer;
}
template <typename... NodeType>
@@ -126,7 +127,8 @@
Dumper.Visit(std::forward<NodeType &&>(N)...);
- return OS.str();
+ OS.flush();
+ return Buffer;
}
const FunctionDecl *getFunctionNode(clang::ASTUnit *AST,
Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
===================================================================
--- clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -139,7 +139,8 @@
Passes.run(*M);
- return OS.str();
+ OS.flush();
+ return outString;
}
// Takes a function and runs it on a set of inputs
Index: clang/lib/Tooling/Syntax/Tree.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -263,7 +263,8 @@
OS << " ";
}
});
- return OS.str();
+ OS.flush();
+ return Storage;
}
void syntax::Node::assertInvariants() const {
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -927,5 +927,6 @@
M.EndExpanded);
}
}
- return OS.str();
+ OS.flush();
+ return Dump;
}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -444,7 +444,8 @@
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 {
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -437,7 +437,8 @@
for (auto BI : *Buf)
os << BI;
- return os.str();
+ os.flush();
+ return file;
}
void HTMLDiagnostics::dumpCoverageData(
@@ -534,7 +535,8 @@
</form>
)<<<";
- return os.str();
+ os.flush();
+ return s;
}
void HTMLDiagnostics::FinalizeHTML(const PathDiagnostic& D, Rewriter &R,
@@ -1202,7 +1204,8 @@
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) {
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -389,7 +389,8 @@
llvm::raw_string_ostream OS(SBuf);
OS << "Function '" << FuncDecl->getDeclName()
<< "' returns an open handle";
- return OS.str();
+ OS.flush();
+ return SBuf;
} else
return "";
});
@@ -405,7 +406,8 @@
llvm::raw_string_ostream OS(SBuf);
OS << "Function '" << FuncDecl->getDeclName()
<< "' returns an unowned handle";
- return OS.str();
+ OS.flush();
+ return SBuf;
} else
return "";
});
@@ -439,7 +441,8 @@
llvm::raw_string_ostream OS(SBuf);
OS << "Handle released through " << ParamDiagIdx
<< llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
- return OS.str();
+ OS.flush();
+ return SBuf;
} else
return "";
});
@@ -453,7 +456,8 @@
llvm::raw_string_ostream OS(SBuf);
OS << "Handle allocated through " << ParamDiagIdx
<< llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
- return OS.str();
+ OS.flush();
+ return SBuf;
} else
return "";
});
@@ -467,7 +471,8 @@
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 "";
});
Index: clang/lib/Sema/SemaAttr.cpp
===================================================================
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -792,7 +792,8 @@
OS << (I.index() == Rules.size() - 1 ? ", and " : ", ");
OS << "'" << attr::getSubjectMatchRuleSpelling(I.value()) << "'";
}
- return OS.str();
+ OS.flush();
+ return Result;
}
} // end anonymous namespace
Index: clang/lib/Sema/CodeCompleteConsumer.cpp
===================================================================
--- clang/lib/Sema/CodeCompleteConsumer.cpp
+++ clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -335,7 +335,8 @@
break;
}
}
- return OS.str();
+ OS.flush();
+ return Result;
}
const char *CodeCompletionString::getTypedText() const {
@@ -640,7 +641,8 @@
break;
}
}
- return OS.str();
+ OS.flush();
+ return Result;
}
void PrintingCodeCompleteConsumer::ProcessOverloadCandidates(
Index: clang/lib/Rewrite/HTMLRewrite.cpp
===================================================================
--- clang/lib/Rewrite/HTMLRewrite.cpp
+++ clang/lib/Rewrite/HTMLRewrite.cpp
@@ -202,8 +202,8 @@
case '&': os << "&"; break;
}
}
-
- return os.str();
+ os.flush();
+ return Str;
}
static void AddLineNumber(RewriteBuffer &RB, unsigned LineNo,
Index: clang/lib/Frontend/TestModuleFileExtension.cpp
===================================================================
--- clang/lib/Frontend/TestModuleFileExtension.cpp
+++ clang/lib/Frontend/TestModuleFileExtension.cpp
@@ -133,5 +133,6 @@
llvm::raw_string_ostream OS(Buffer);
OS << BlockName << ":" << MajorVersion << ":" << MinorVersion << ":" << Hashed
<< ":" << UserInfo;
- return OS.str();
+ OS.flush();
+ return Buffer;
}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1285,7 +1285,8 @@
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.
Index: clang/lib/Basic/Version.cpp
===================================================================
--- clang/lib/Basic/Version.cpp
+++ clang/lib/Basic/Version.cpp
@@ -82,7 +82,8 @@
OS << LLVMRepo << ' ';
OS << LLVMRev << ')';
}
- return OS.str();
+ OS.flush();
+ return buf;
}
std::string getClangFullVersion() {
@@ -102,7 +103,8 @@
OS << " " << repo;
}
- return OS.str();
+ OS.flush();
+ return buf;
}
std::string getClangFullCPPVersion() {
@@ -120,7 +122,8 @@
OS << " " << repo;
}
- return OS.str();
+ OS.flush();
+ return buf;
}
} // end namespace clang
Index: clang/lib/Basic/SourceLocation.cpp
===================================================================
--- clang/lib/Basic/SourceLocation.cpp
+++ clang/lib/Basic/SourceLocation.cpp
@@ -90,7 +90,8 @@
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 {
@@ -149,7 +150,8 @@
std::string S;
llvm::raw_string_ostream OS(S);
print(OS, SM);
- return OS.str();
+ OS.flush();
+ return S;
}
//===----------------------------------------------------------------------===//
Index: clang/lib/Analysis/AnalysisDeclContext.cpp
===================================================================
--- clang/lib/Analysis/AnalysisDeclContext.cpp
+++ clang/lib/Analysis/AnalysisDeclContext.cpp
@@ -387,7 +387,8 @@
OS << ' ' << OMD->getSelector().getAsString() << ']';
}
- return OS.str();
+ OS.flush();
+ return Str;
}
LocationContextManager &AnalysisDeclContext::getLocationContextManager() {
Index: clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
===================================================================
--- clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
+++ clang/lib/ASTMatchers/Dynamic/Diagnostics.cpp
@@ -204,7 +204,8 @@
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 {
@@ -223,7 +224,8 @@
std::string S;
llvm::raw_string_ostream OS(S);
printToStreamFull(OS);
- return OS.str();
+ OS.flush();
+ return S;
}
} // namespace dynamic
Index: clang/lib/AST/OpenMPClause.cpp
===================================================================
--- clang/lib/AST/OpenMPClause.cpp
+++ clang/lib/AST/OpenMPClause.cpp
@@ -2454,7 +2454,8 @@
Property.RawString);
}
}
- return OS.str();
+ OS.flush();
+ return MangledName;
}
OMPTraitInfo::OMPTraitInfo(StringRef MangledName) {
Index: clang/lib/AST/DeclarationName.cpp
===================================================================
--- clang/lib/AST/DeclarationName.cpp
+++ clang/lib/AST/DeclarationName.cpp
@@ -236,7 +236,8 @@
std::string Result;
llvm::raw_string_ostream OS(Result);
OS << *this;
- return OS.str();
+ OS.flush();
+ return Result;
}
void *DeclarationName::getFETokenInfoSlow() const {
@@ -460,7 +461,8 @@
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) {
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1589,7 +1589,8 @@
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 {
Index: clang/lib/AST/AttrImpl.cpp
===================================================================
--- clang/lib/AST/AttrImpl.cpp
+++ clang/lib/AST/AttrImpl.cpp
@@ -60,7 +60,8 @@
else
OS << "disable";
OS << ")";
- return OS.str();
+ OS.flush();
+ return ValueName;
}
// Return a string suitable for identifying this attribute in diagnostics.
Index: clang/lib/AST/ASTDiagnostic.cpp
===================================================================
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -312,7 +312,8 @@
OS << "'" << S << "' (vector of " << VTy->getNumElements() << " '"
<< VTy->getElementType().getAsString(Context.getPrintingPolicy())
<< "' " << Values << ")";
- return OS.str();
+ OS.flush();
+ return DecoratedString;
}
}
Index: clang/include/clang/Testing/TestClangConfig.h
===================================================================
--- clang/include/clang/Testing/TestClangConfig.h
+++ clang/include/clang/Testing/TestClangConfig.h
@@ -73,7 +73,8 @@
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,
Index: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
+++ clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
@@ -32,7 +32,8 @@
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) {
@@ -69,7 +70,8 @@
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) {
@@ -82,7 +84,8 @@
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) {
@@ -123,7 +126,8 @@
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.
@@ -177,7 +181,8 @@
else
OS << "'" << Visit(R->getIndex()) << "'";
OS << " of " + Visit(R->getSuperRegion());
- return OS.str();
+ OS.flush();
+ return Str;
}
std::string VisitNonParamVarRegion(const NonParamVarRegion *R) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
