<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>21 nov 2007 kl. 23.16 skrev Ted Kremenek:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Anders,<div><br class="webkit-block-placeholder"></div><div>The serialization code looks good to me. A few comments (inline):</div><div><br><div><div>On Nov 21, 2007, at 5:36 PM, Anders Carlsson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0; ">Modified: cfe/trunk/AST/StmtSerialization.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/StmtSerialization.cpp?rev=44266&r1=44265&r2=44266&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/StmtSerialization.cpp?rev=44266&r1=44265&r2=44266&view=diff</a><br><br>==============================================================================<br>--- cfe/trunk/AST/StmtSerialization.cpp (original)<br>+++ cfe/trunk/AST/StmtSerialization.cpp Wed Nov 21 19:36:19 2007<br>@@ -197,8 +197,27 @@<br><br>void AsmStmt::EmitImpl(Serializer& S) const {<br> S.Emit(AsmLoc);<br>+ <br> getAsmString()->EmitImpl(S);</span></blockquote><div><br class="webkit-block-placeholder"></div><div>Directly emitting the StringLiteral object is okay if you believe that no object outside of the AsmStmt will EVER hold a reference to it. Otherwise, I would use EmitOwnedPtr. This reasoning applies to all the places where StringLiteral appears in AsmStmt. If you don't use EmitOwnedPtr (since I assume AsmStmt owns the references to these StringLiterals), the serializer won't be able to backpatch or otherwise fix up other references to these objects upon deserialization.</div></div></div></div></blockquote><div><br class="webkit-block-placeholder"></div>That should not be a problem. Eventually all the uses of StringLiteral should be replaced by regular std::strings.</div><div><br class="webkit-block-placeholder"></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><blockquote type="cite"><br></blockquote><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-size: 14px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0; "><br>+ Stmt->Names.reserve(size);<br>+ for (unsigned i = 0; i < size; ++i) {<br>+ std::vector<char> data;</span></blockquote><div><br class="webkit-block-placeholder"></div><div>One recommendation is to move the vector "data" outside of the loop, and invoke "data.clear()" at the beginning of the loop body. This will reduce the number of allocations made when the vector is resized.</div></div></div></div></blockquote><div><br class="webkit-block-placeholder"></div>Good point, I'll do that.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br class="webkit-block-placeholder"></div><div>Looks good! I'm glad you could successfully de-obfuscate my completely undocumented serialization API.</div></div></div></blockquote></div><br><div>I've seen much much worse APIs ;)</div><div><br class="webkit-block-placeholder"></div><div>Anders</div></body></html>