<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Dec 30, 2008, at 2:54 PM, Talin wrote:</div><blockquote type="cite"><div>OK changes made and new patch attached.</div></blockquote><div><br></div>Nice</div><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; ">+++ utils/unittest/Makefile<span class="Apple-tab-span" style="white-space:pre">   </span>(revision 0)</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;">...</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; ">+# This has to come after Makefile.common, since it doesn't allow us to</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+# override the VPATH value unless we set PROJECT_NAME, which we don't want</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+# to do.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+VPATH = $(LLVM_SRC_ROOT)/utils/unittest/googletest/src/</div><div><br></div><div><br></div><div>Why play with VPATH here?  What is this doing?  Can this be handled a different way?  Similarly in makefile.unittest.</div><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; ">+++ unittests/ADT/DenseMapTest.cpp<span class="Apple-tab-span" style="white-space:pre">     </span>(revision 0)</div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+namespace {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">+using namespace llvm;</div><div><br></div><div><br></div><div>Please put the 'using' right after the #includes.</div><div><br></div><div>Otherwise, the patch looks good to me.  Please verify that it works with both a SRCDIR=OBJDIR and SRCDIR!=OBJDIR build.</div><div><br></div><div>-Chris</div></div></div></span></font></div></div></body></html>