<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 9, 2008, at 5:14 AM, Csaba Hruska wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi!<br>Here are the cutted patch files of indepenent changes:</blockquote><div><br></div><div>Great!</div><br><blockquote type="cite"><br>TranslationUnit.patch - adds support (de)serialization (from)to (in memory) buffer.</blockquote><div><br></div><div>Ted, can you look at this?</div><br><blockquote type="cite"><br>Preprocessor.patch - adds a new method (getPredefines) needed for passing predefines list to clangserver.</blockquote><div><br></div><div>Looks good to me, applied!</div><br><blockquote type="cite"><br> ASTConsumers.patch - adds functions what supports capruing ASTConsumer's output.</blockquote><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">-ASTConsumer *clang::CreateASTDumper() { return new ASTDumper(); }</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+ASTConsumer *clang::CreateASTDumper(std::ostream* OS) { return new ASTDumper(OS); }</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 10.0px Monaco; min-height: 14.0px"> <br class="webkit-block-placeholder"></p><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;">Please stay in 80 cols.</span></font></div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><br></span></font></div><div><font class="Apple-style-span" face="Monaco" size="2"><span class="Apple-style-span" style="font-size: 10px;"><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> ASTConsumer* CreateHTMLPrinter(const std::string &OutFile, Diagnostic &D,</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> Preprocessor *PP, PreprocessorFactory* PPF);</div><p style="margin: 0.0px 0.0px 0.0px 0.0px; font: 10.0px Monaco; min-height: 14.0px"> <br class="webkit-block-placeholder"></p><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+ASTConsumer* CreateHTMLPrinter(std::ostream &OutStream, Diagnostic &D,</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+ Preprocessor *PP, PreprocessorFactory* PPF);</div><div><br></div><div>It's unclear to me that duplicating each of these is really the right approach. Would it work to make the *only* entry points take an ostream, and then just have the caller create the ofstream etc and pass that in as the OutStream?</div><div><br></div><div><br></div></span></font></div></div><div><blockquote type="cite">I've also attached a skeleton of distclang documentation page (no info yet, it will be added after the initial commit).<br></blockquote><br></div><div>Applied.</div><div><br></div><blockquote type="cite"><br>I haven't attached clang.cpp, clang.h NetSession.cpp, NetSession.h patch because they place in source structure is unknown.</blockquote><blockquote type="cite">We have to refactor the Driver directory content first.</blockquote><div><br></div>Sounds good. Thank you for taking this in steps, it makes it far easier to review.</div><div><br></div><div><br><blockquote type="cite"><br> - platform independent thread and network support. Should these added to llvm/Support or System library ?</blockquote><div><br></div><div>The general rule is that really low level things that are highly system specific should go in libsystem. libsupport can contain target independent abstractions that may optionally be built on libsystem components.</div><br><blockquote type="cite">We also have to decide if we want to be distcc compatible or not. And what's the benefit of distcc compatibility?<br></blockquote></div><br><div>I don't really have an opinion or an answer here. :) Are there benefits? anyone have an opinion?</div><div><br></div><div><br></div><div><div><blockquote type="cite"><span class="Apple-style-span" style="-webkit-text-stroke-width: -1; "> - capture output of PrintPreprocessedOutput.cpp. Ted: openfd is not acceptable, because we have to know the output size before we send it. (required by net packet header)</span></blockquote></div><div><div><br></div><div>Lets talk about this one specifically. The "Simple buffered I/O" code in PrintPreprocessedOutput.cpp was written with extensive profiling, tuning and other tweaking and it is quite effective, though very ugly, code. The basic algorithm is that it buffers up chunks of 64K, then writes them out to disk with open/write/close. The API for it is very simple, and is basically built around not copying strings unnecessarily.</div><div><br></div><div>This is pretty simple stuff and would be very useful elsewhere in LLVM (e.g. the .s printer, the bc writer, ...). It has been on my todo list for a long time, and seems required for the distcc project, to refactor this out to be a more general interface. I think something like this would be a great interface:</div><div><br></div><div>class outstream {</div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><span style="color: #aa0d91"> </span> <span style="color: #aa0d91">char</span> *OutBufStart, *OutBufEnd, *OutBufCur;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> outstream(const outstream&); // can't be copied.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> void operator=(const outstream&); // or assigned.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">protected:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span class="Apple-style-span" style="font-family: Helvetica; font-size: 12px; ">outstream() {} // may only be subclassed</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px;"> virtual void FlushBuffer() = 0;</span></font></div><div>public:</div><div> virtual ~outstream();</div></div><div> </div><div> void OutputChar(char c) { ...} </div><div> void OutputString(<span class="Apple-style-span" style="color: rgb(170, 13, 145); font-family: Monaco; font-size: 10px; ">const<span style="color: #000000"> </span>char<span style="color: #000000"> *Ptr, </span>unsigned<span style="color: #000000"> Size) { ... }</span></span></div><div> void Close();</div><div><br></div><div> static outstream* CreateFile(...);</div><div> static outstream* CreateOStream(std::ostream &O);</div><div> ...</div><div>};</div><div><br></div><div>The basic idea here is that all the buffering of simple things (e.g. strings and chars) is inline and trivial, and the flushing happens via a virtual method that does the write or whatever else is needed. This is a very simple and useful API that is mostly target-independent (and should thus live in libsupport) but does rely on a few system specific things (which could live in libsystem).</div><div><br></div><div>Is this something you could take on? It should be pretty straight-forward. Once it exists, switching the -E printer over to it should be easy, and this will make it easy to get it to output to an std::ostream or whatever else is desired (you could even make an outstream for a socket or whatever, to avoid the extra std::ostream overhead).</div><div><br></div></div></div><div>-Chris</div></body></html>