<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 24, 2008, at 8:08 AM, Csaba Hruska wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">Hi!<br>Here is a patch what replaces std::ostream with llvm::raw_ostream.<br>This patch covers the AST library, but ignores Analysis lib. That will be ported later.<br><br>Cheers,<br>Csaba<br></div> <span><raw_ostreamport.patch></span>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev<br></blockquote></div><br><div>Hi Csaba,</div><div><br></div><div>This looks like a pretty good patch, although it introduces a "flushing" bug when using "raw_string_ostream" that appears over and over. Here are some specific comments:</div><div><br></div><div><blockquote type="cite" class=""><div><span class="Apple-style-span" style="-webkit-text-stroke-width: -1; ">===================================================================</span></div><div><font class="Apple-style-span" color="#000000">--- Driver/SerializationTest.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55284)</font></div><div><font class="Apple-style-span" color="#000000">+++ Driver/SerializationTest.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -64,12 +64,15 @@</font></div><div><font class="Apple-style-span" color="#000000"> TranslationUnit& TU) {</font></div><div><font class="Apple-style-span" color="#000000"> { </font></div><div><font class="Apple-style-span" color="#000000"> // Pretty-print the decls to a temp file.</font></div><div><font class="Apple-style-span" color="#000000">- std::ofstream DeclPP(FNameDeclPrint.c_str());</font></div><div><font class="Apple-style-span" color="#000000">- assert (DeclPP && "Could not open file for printing out decls.");</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_fd_ostream DeclPP(FNameDeclPrint.c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000">+ assert (Err.empty() && "Could not open file for printing out decls.");</font></div><div><font class="Apple-style-span" color="#000000"> llvm::OwningPtr<ASTConsumer> FilePrinter(CreateASTPrinter(&DeclPP));</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> for (TranslationUnit::iterator I=TU.begin(), E=TU.end(); I!=E; ++I)</font></div><div><font class="Apple-style-span" color="#000000"> FilePrinter->HandleTopLevelDecl(*I);</font></div><div><font class="Apple-style-span" color="#000000">+</font></div><div><font class="Apple-style-span" color="#000000">+ DeclPP.flush();</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>I don't think the DeclPP.flush() is needed here, since DecpPP is stack allocated. Its destructor (which is called immediately afterwards) will flush the output. Having the explicit flush in this case just adds a little more clutter.</div><div><br></div><div>Here is the destructor for raw_fd_ostream:</div><div><br></div><div><div>raw_fd_ostream::~raw_fd_ostream() {</div> <div> flush();</div> <div> if (ShouldClose)</div> <div> close(FD);</div> <div>}</div></div><div><br></div><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Serialize the translation unit.</font></div><div><font class="Apple-style-span" color="#000000">@@ -87,12 +90,15 @@</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> {</font></div><div><font class="Apple-style-span" color="#000000"> // Pretty-print the deserialized decls to a temp file.</font></div><div><font class="Apple-style-span" color="#000000">- std::ofstream DeclPP(FNameDeclPrint.c_str());</font></div><div><font class="Apple-style-span" color="#000000">- assert (DeclPP && "Could not open file for printing out decls.");</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_fd_ostream DeclPP(FNameDeclPrint.c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000">+ assert (Err.empty() && "Could not open file for printing out decls.");</font></div><div><font class="Apple-style-span" color="#000000"> llvm::OwningPtr<ASTConsumer> FilePrinter(CreateASTPrinter(&DeclPP));</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> for (TranslationUnit::iterator I=NewTU->begin(), E=NewTU->end(); I!=E; ++I)</font></div><div><font class="Apple-style-span" color="#000000"> FilePrinter->HandleTopLevelDecl(*I);</font></div><div><font class="Apple-style-span" color="#000000">+</font></div><div><font class="Apple-style-span" color="#000000">+ DeclPP.flush();</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Same thing here. No need to explicitly call DeclPP.flush().</div><div><br></div><blockquote type="cite" class=""><div><span class="Apple-style-span" style="-webkit-text-stroke-width: -1; ">Index: Driver/RewriteObjC.cpp</span></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- Driver/RewriteObjC.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55284)</font></div><div><font class="Apple-style-span" color="#000000">+++ Driver/RewriteObjC.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -25,9 +25,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/MemoryBuffer.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/CommandLine.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/Streams.h"</font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/System/Path.h"</font></div><div><font class="Apple-style-span" color="#000000">-#include <sstream></font></div><div><font class="Apple-style-span" color="#000000">-#include <fstream></font></div><div><font class="Apple-style-span" color="#000000"> using namespace clang;</font></div><div><font class="Apple-style-span" color="#000000"> using llvm::utostr;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -453,20 +452,20 @@</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Create the output file.</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- std::ostream *OutFile;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_ostream *OutFile;</font></div><div><font class="Apple-style-span" color="#000000"> if (OutFileName == "-") {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = llvm::cout.stream();</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = &llvm::outs();</font></div><div><font class="Apple-style-span" color="#000000"> } else if (!OutFileName.empty()) {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = new std::ofstream(OutFileName.c_str(), </font></div><div><font class="Apple-style-span" color="#000000">- std::ios_base::binary|std::ios_base::out);</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = new llvm::raw_fd_ostream(OutFileName.c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000"> } else if (InFileName == "-") {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = llvm::cout.stream();</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = &llvm::outs();</font></div><div><font class="Apple-style-span" color="#000000"> } else {</font></div><div><font class="Apple-style-span" color="#000000"> llvm::sys::Path Path(InFileName);</font></div><div><font class="Apple-style-span" color="#000000"> Path.eraseSuffix();</font></div><div><font class="Apple-style-span" color="#000000"> Path.appendSuffix("cpp");</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = new std::ofstream(Path.toString().c_str(), </font></div><div><font class="Apple-style-span" color="#000000">- std::ios_base::binary|std::ios_base::out);</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = new llvm::raw_fd_ostream(Path.toString().c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> RewriteInclude();</font></div><div><font class="Apple-style-span" color="#000000">@@ -489,6 +488,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> // Emit metadata.</font></div><div><font class="Apple-style-span" color="#000000"> *OutFile << ResultStr;</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile->flush();</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>I don't think you introduced this bug, but I think that there is a memory leak here. In the case where OutFile is not &llvm::outs(), the stream is never deallocated. One possible solution is to introduced an extra llvm::OwningPtr<> here. For example:</div><div><br></div><div> llvm::OwningPtr<llvm::raw_ostream> OwnedStream;</div><div> llvm::raw_ostream *OutFile;</div><div> ...</div><div> else if (!OutFileName.empty()) {</div><div> OutFile = new llvm::raw_fd_ostream(...);</div><div> OwnedStream.reset(OutFile);</div><div> }</div><div> ..</div><div><br></div><div>Thus, with just a little more code, the dynamically allocated raw_fd_ostream is deallocated at the end of the function.</div><div><br></div><div><br></div><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000">Index: Driver/RewriteMacros.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- Driver/RewriteMacros.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55284)</font></div><div><font class="Apple-style-span" color="#000000">+++ Driver/RewriteMacros.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -17,8 +17,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> #include "clang/Lex/Preprocessor.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "clang/Basic/SourceManager.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/Streams.h"</font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/System/Path.h"</font></div><div><font class="Apple-style-span" color="#000000">-#include <fstream></font></div><div><font class="Apple-style-span" color="#000000"> using namespace clang;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> /// isSameToken - Return true if the two specified tokens start have the same</font></div><div><font class="Apple-style-span" color="#000000">@@ -205,20 +205,20 @@</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Create the output file.</font></div><div><font class="Apple-style-span" color="#000000">- std::ostream *OutFile;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_ostream *OutFile;</font></div><div><font class="Apple-style-span" color="#000000"> if (OutFileName == "-") {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = llvm::cout.stream();</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = &llvm::outs();</font></div><div><font class="Apple-style-span" color="#000000"> } else if (!OutFileName.empty()) {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = new std::ofstream(OutFileName.c_str(), </font></div><div><font class="Apple-style-span" color="#000000">- std::ios_base::binary|std::ios_base::out);</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = new llvm::raw_fd_ostream(OutFileName.c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000"> } else if (InFileName == "-") {</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = llvm::cout.stream();</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = &llvm::outs();</font></div><div><font class="Apple-style-span" color="#000000"> } else {</font></div><div><font class="Apple-style-span" color="#000000"> llvm::sys::Path Path(InFileName);</font></div><div><font class="Apple-style-span" color="#000000"> Path.eraseSuffix();</font></div><div><font class="Apple-style-span" color="#000000"> Path.appendSuffix("cpp");</font></div><div><font class="Apple-style-span" color="#000000">- OutFile = new std::ofstream(Path.toString().c_str(), </font></div><div><font class="Apple-style-span" color="#000000">- std::ios_base::binary|std::ios_base::out);</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Err;</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile = new llvm::raw_fd_ostream(Path.toString().c_str(), Err);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Get the buffer corresponding to MainFileID. If we haven't changed it, then</font></div><div><font class="Apple-style-span" color="#000000">@@ -230,4 +230,5 @@</font></div><div><font class="Apple-style-span" color="#000000"> } else {</font></div><div><font class="Apple-style-span" color="#000000"> fprintf(stderr, "No changes\n");</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000">+ OutFile->flush();</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Same problem as before: possible memory leak of OutFile (this looks like a copy-paste).</div><div><br></div><blockquote type="cite" class=""><font class="Apple-style-span" color="#000000"><br></font><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">Index: Driver/ASTConsumers.h</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- Driver/ASTConsumers.h</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55284)</font></div><div><font class="Apple-style-span" color="#000000">+++ Driver/ASTConsumers.h</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -14,6 +14,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> #ifndef DRIVER_ASTCONSUMERS_H</font></div><div><font class="Apple-style-span" color="#000000"> #define DRIVER_ASTCONSUMERS_H</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include <string></font></div><div><font class="Apple-style-span" color="#000000"> #include <iosfwd></font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -30,7 +31,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> class Preprocessor;</font></div><div><font class="Apple-style-span" color="#000000"> class PreprocessorFactory;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">-ASTConsumer *CreateASTPrinter(std::ostream* OS = NULL);</font></div><div><font class="Apple-style-span" color="#000000">+ASTConsumer *CreateASTPrinter(llvm::raw_ostream* OS = NULL);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> ASTConsumer *CreateASTDumper();</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">Index: lib/Rewrite/Rewriter.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- lib/Rewrite/Rewriter.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55282)</font></div><div><font class="Apple-style-span" color="#000000">+++ lib/Rewrite/Rewriter.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -16,7 +16,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> #include "clang/AST/Stmt.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "clang/Lex/Lexer.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "clang/Basic/SourceManager.h"</font></div><div><font class="Apple-style-span" color="#000000">-#include <sstream></font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> using namespace clang;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> void RewriteBuffer::RemoveText(unsigned OrigOffset, unsigned Size) {</font></div><div><font class="Apple-style-span" color="#000000">@@ -166,9 +166,9 @@</font></div><div><font class="Apple-style-span" color="#000000"> return true;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Get the new text.</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream S;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Str;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream S(Str);</font></div><div><font class="Apple-style-span" color="#000000"> To->printPretty(S);</font></div><div><font class="Apple-style-span" color="#000000">- const std::string &Str = S.str();</font></div></blockquote><div><br></div><div><br></div><div><div>Looks like we're missing a "flush" here before using the value of Str.</div><div><br></div><div>Here's a thought. I added a "str()" method to raw_string_stream that does an implicit flush:</div><div><br></div><div>std::string& raw_string_ostream::str() {</div><div> flush();</div><div> return OS;</div><div>}</div><div><br></div><div>It's in this commit: <a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080825/066409.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080825/066409.html</a></div><div><br></div><div>This behavior more closely matches ostringstream, and I like it because it forces clients to think about retrieving the latest version of the string. Using str() removes a whole bunch of "flush" errors that your patch introduces (see the remainder of my comments to see how frequently this happens). Consequently, the current code that uses ostringstream can basically be left unmodified (except for swapping in raw_string_ostream), the necessary flushes get introduced, and we're all happy.</div><div><br></div></div><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> ReplaceText(From->getLocStart(), Size, &Str[0], Str.size());</font></div><div><font class="Apple-style-span" color="#000000"> return false;</font></div><div><font class="Apple-style-span" color="#000000">Index: lib/Rewrite/HTMLRewrite.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- lib/Rewrite/HTMLRewrite.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55282)</font></div><div><font class="Apple-style-span" color="#000000">+++ lib/Rewrite/HTMLRewrite.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -20,7 +20,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/ADT/SmallString.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/ADT/OwningPtr.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/MemoryBuffer.h"</font></div><div><font class="Apple-style-span" color="#000000">-#include <sstream></font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> using namespace clang;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -161,7 +161,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> bool ReplaceTabs) {</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> unsigned len = s.size();</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Str;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(Str);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> for (unsigned i = 0 ; i < len; ++i) {</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -195,7 +196,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- return os.str();</font></div><div><font class="Apple-style-span" color="#000000">+ return Str;</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Definitely missing a flush. Use os.str() (which is nice, because the change on the last line is not needed).</div><div><br></div><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> static void AddLineNumber(RewriteBuffer &RB, unsigned LineNo,</font></div><div><font class="Apple-style-span" color="#000000">@@ -272,7 +273,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> SourceLocation StartLoc = SourceLocation::getFileLoc(FileID, 0);</font></div><div><font class="Apple-style-span" color="#000000"> SourceLocation EndLoc = SourceLocation::getFileLoc(FileID, FileEnd-FileStart);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> os << "<!doctype html>\n" // Use HTML 5 doctype</font></div><div><font class="Apple-style-span" color="#000000"> "<html>\n<head>\n";</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -327,7 +329,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> "</style>\n</head>\n<body>";</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Generate header</font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(StartLoc, os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(StartLoc, s);</font></div></blockquote><div><br></div><div>Missing flush. Just use os.str().</div><div><br></div><blockquote type="cite" class=""><font class="Apple-style-span" color="#000000"><br></font><div><font class="Apple-style-span" color="#000000">Index: lib/AST/CFG.cpp</font></div><div><font class="Apple-style-span" color="#000000"><br></font></div><div><font class="Apple-style-span" color="#000000"> static std::string getNodeLabel(const CFGBlock* Node, const CFG* Graph) {</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> #ifndef NDEBUG</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream Out;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string OutStr;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream Out(OutStr);</font></div><div><font class="Apple-style-span" color="#000000"> print_block(Out,Graph, *Node, GraphHelper, false);</font></div><div><font class="Apple-style-span" color="#000000">- std::string OutStr = Out.str();</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> if (OutStr[0] == '\n') OutStr.erase(OutStr.begin());</font></div></blockquote><div><br></div><div>Another missing flush. Jut use os.str().</div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">Index: lib/AST/StmtViz.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- lib/AST/StmtViz.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55282)</font></div><div><font class="Apple-style-span" color="#000000">+++ lib/AST/StmtViz.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -33,14 +33,14 @@</font></div><div><font class="Apple-style-span" color="#000000"> static std::string getNodeLabel(const Stmt* Node, const Stmt* Graph) {</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> #ifndef NDEBUG</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream Out;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string OutStr;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream Out(OutStr);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> if (Node)</font></div><div><font class="Apple-style-span" color="#000000"> Out << Node->getStmtClassName();</font></div><div><font class="Apple-style-span" color="#000000"> else</font></div><div><font class="Apple-style-span" color="#000000"> Out << "<NULL>";</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- std::string OutStr = Out.str(); </font></div><div><font class="Apple-style-span" color="#000000"> if (OutStr[0] == '\n') OutStr.erase(OutStr.begin());</font></div></blockquote><div><br></div><div>Another missing flush.</div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Process string output to make it nicer...</font></div><div><font class="Apple-style-span" color="#000000">Index: lib/AST/Type.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- lib/AST/Type.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55282)</font></div><div><font class="Apple-style-span" color="#000000">+++ lib/AST/Type.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -870,9 +870,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> S += '*';</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> if (getSizeExpr()) {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream s(S);</font></div><div><font class="Apple-style-span" color="#000000"> getSizeExpr()->printPretty(s);</font></div><div><font class="Apple-style-span" color="#000000">- S += s.str();</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> S += ']';</font></div></blockquote><div><br></div><div>This is okay. The destructor for raw_string_stream does a flush to S before we hit the statement "S += ']'";</div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -897,9 +896,10 @@</font></div><div><font class="Apple-style-span" color="#000000"> void TypeOfExpr::getAsStringInternal(std::string &InnerString) const {</font></div><div><font class="Apple-style-span" color="#000000"> if (!InnerString.empty()) // Prefix the basic type, e.g. 'typeof(e) X'.</font></div><div><font class="Apple-style-span" color="#000000"> InnerString = ' ' + InnerString;</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream s;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string Str;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream s(Str);</font></div><div><font class="Apple-style-span" color="#000000"> getUnderlyingExpr()->printPretty(s);</font></div><div><font class="Apple-style-span" color="#000000">- InnerString = "typeof(" + s.str() + ")" + InnerString;</font></div><div><font class="Apple-style-span" color="#000000">+ InnerString = "typeof(" + Str + ")" + InnerString;</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Another missing flush. Use s.str().</div><div><br></div><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000">Index: lib/Driver/HTMLDiagnostics.cpp</font></div><div><font class="Apple-style-span" color="#000000">===================================================================</font></div><div><font class="Apple-style-span" color="#000000">--- lib/Driver/HTMLDiagnostics.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(revision 55282)</font></div><div><font class="Apple-style-span" color="#000000">+++ lib/Driver/HTMLDiagnostics.cpp</font><span class="Apple-tab-span" style="white-space:pre"><font class="Apple-style-span" color="#000000"> </font></span><font class="Apple-style-span" color="#000000">(working copy)</font></div><div><font class="Apple-style-span" color="#000000">@@ -23,9 +23,9 @@</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/Compiler.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/MemoryBuffer.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/Support/Streams.h"</font></div><div><font class="Apple-style-span" color="#000000">+#include "llvm/Support/raw_ostream.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include "llvm/System/Path.h"</font></div><div><font class="Apple-style-span" color="#000000"> #include <fstream></font></div><div><font class="Apple-style-span" color="#000000">-#include <sstream></font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> using namespace clang;</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">@@ -220,7 +220,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> // Add the name of the file as an <h1> tag. </font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> os << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"</font></div><div><font class="Apple-style-span" color="#000000"> "<tr><td class=\"rowname\">File:</td><td>"</font></div><div><font class="Apple-style-span" color="#000000">@@ -244,7 +245,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> os << "</table>\n<h3>Annotated Source Code</h3>\n"; </font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), s);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Missing flush. Use os.str().</div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Embed meta-data tags.</font></div><div><font class="Apple-style-span" color="#000000">@@ -252,28 +253,32 @@</font></div><div><font class="Apple-style-span" color="#000000"> const std::string& BugDesc = D.getDescription();</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> if (!BugDesc.empty()) {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> os << "\n<!-- BUGDESC " << BugDesc << " -->\n";</font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), s);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div>Missing flush. Use os.str().</div><div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> os << "\n<!-- BUGFILE " << DirName << Entry->getName() << " -->\n";</font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), s);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div><div>Same thing.</div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> os << "\n<!-- BUGLINE " << D.back()->getLocation().getLogicalLineNumber()</font></div><div><font class="Apple-style-span" color="#000000"> << " -->\n";</font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), s);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div>Same thing.</div><div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> {</font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> os << "\n<!-- BUGPATHLENGTH " << D.size() << " -->\n";</font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, 0), s);</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div></blockquote><div><br></div>Same thing.</div><div><br><blockquote type="cite" class=""><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Add CSS, header, and footer.</font></div><div><font class="Apple-style-span" color="#000000">@@ -365,7 +370,8 @@</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> // Create the html for the message.</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- std::ostringstream os;</font></div><div><font class="Apple-style-span" color="#000000">+ std::string s;</font></div><div><font class="Apple-style-span" color="#000000">+ llvm::raw_string_ostream os(s);</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000"> os << "\n<tr><td class=\"num\"></td><td class=\"line\">"</font></div><div><font class="Apple-style-span" color="#000000"> << "<div id=\"";</font></div><div><font class="Apple-style-span" color="#000000">@@ -398,7 +404,7 @@</font></div><div><font class="Apple-style-span" color="#000000"> assert (false && "Unhandled hint.");</font></div><div><font class="Apple-style-span" color="#000000"> }</font></div><div><font class="Apple-style-span" color="#000000"> </font></div><div><font class="Apple-style-span" color="#000000">- R.InsertStrBefore(SourceLocation::getFileLoc(FileID, DisplayPos), os.str());</font></div><div><font class="Apple-style-span" color="#000000">+ R.InsertStrBefore(SourceLocation::getFileLoc(FileID, DisplayPos), s);</font></div></blockquote><div><br></div>Same thing.</div><div><br></div><div>For the uses of raw_string_ostream, I think the right thing to do is just remove your changes where your removed the call to "str()". Other than fixing the two memory leaks I pointed out, the rest of the patch looks pretty good!</div></body></html>