<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Guoping,<div><br></div><div>This looks great to me. My main concern is that we are copy-pasting TopologicallySortedCFG and CFGBlockSet yet again. This is a very bad trend. Before this patch goes in, I'd like to see that common code refactored. I'll look into fixing that today.</div><div><br></div><div>Cheers,</div><div>Ted</div><div><br><div><div>On Oct 20, 2011, at 11:57 AM, Guoping Long wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi<div><br></div><div> Thanks Anna. Thanks for all who help point issues in my code!</div><div><br></div><div> The FileCheck logic has been added. Now I believe the code is at least in a reasonable shape. Attached is the revised patch. Please let me know if there are additional issues. I shall fix them.</div>
<div><br></div><div>----</div><div>Guoping</div><div><br><div class="gmail_quote">2011/10/18 Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com">ganna@apple.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div>Hi Guoping,</div><div><br></div><div>To automate testing, you should check the expected output of the dominators analysis checker with FileCheck. Inside the test, you specify the command which should be applied on it as well as the expected output. It will get tested when you run the following from the clang build directory (see the Testing section on <a href="http://clang.llvm.org/hacking.html" target="_blank">http://clang.llvm.org/hacking.html</a>):</div>
<div>$ make test</div><div>or if you only want to test the tests from the analysis folder:</div><div>$ TESTDIRS=Analysis make test</div><div><br></div><div>Your test is probably going to look similar to the following example, which tests CFG builder:</div>
test/Analysis/initializers-cfg-output.cpp<br><div><div><br></div><div>You can find the documentation on FileCheck here: <a href="http://llvm.org/docs/TestingGuide.html" target="_blank">http://llvm.org/docs/TestingGuide.html</a> The document might be a bit outdated but it does a good job explaining the concepts.</div>
<div><br></div><div>Cheers,</div><div>Anna.</div><div><div></div><div class="h5"><div>On Oct 18, 2011, at 9:08 AM, Guoping Long wrote:</div><br></div></div><blockquote type="cite"><div><div></div><div class="h5"><span style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)">Hi, clang</span><div style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)">
<br></div><div style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)"> Thanks Joerg and Arash. Attached is the new patch with everything contained. Please let me know if there are any further issues. I shall continue working to improve it.</div>
<span style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)">
</span><div style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)"><br></div><div style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)"><font color="#888888">--Guoping</font></div>
<br><div class="gmail_quote">2011/10/18 Joerg Sonnenberger <span dir="ltr"><<a href="mailto:joerg@britannica.bec.de" target="_blank">joerg@britannica.bec.de</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Mon, Oct 17, 2011 at 10:59:27PM -0700, Guoping Long wrote:<br>
> 1 The patch file is generated by running svn diff under the llvm/tools/clang<br>
> directory. I do not know why this patch only contains information for<br>
> existing file modifications, not including new added files (dominators.cpp<br>
> (should be in clang/lib/Analysis/), dominators.h (should be in<br>
> clang/include/clang/Analysis/Analyses/), domtest.c (should be in<br>
> clang/test/Analysis)). So I included the patch in together with the three<br>
> new files as attached.<br>
<br>
</div>Did you run "svn add" on them first?<br>
<font color="#888888"><br>
Joerg<br>
</font></blockquote></div><br>
</div></div><span><dominators.patch></span>_______________________________________________<div class="im"><br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br></div></blockquote></div><br></div></blockquote></div><br></div>
<span><dominators.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></body></html>