Hi,<div><br></div><div>I was not on vacation hence delay in response. </div><div><br></div><div><div>+</div><div>+  void CollectCXXFriends(const CXXRecordDecl *Decl,</div><div>+                       llvm::DIFile F,</div><div>
+                       llvm::SmallVectorImpl<llvm::DIDescriptor> &EltTys,</div><div>+                       llvm::DICompositeType &RecordTy);</div><div>+</div></div><div><br></div><div>There is no need to pass DICompositeType by reference. Just pass it by value.</div>
<div><br></div><div><div>+        llvm::DIType t=getOrCreateType(tsi->getType(),Unit);</div></div><div><br></div><div>Please use 'Ty' instead of 't' for local variable name here, add space around '=' and add space before Unit.</div>
<div><br></div><div><div> llvm::DIType CGDebugInfo::CreateType(const VectorType *Ty,</div><div>-<span class="Apple-tab-span" style="white-space:pre">                               </span>     llvm::DIFile Unit) {</div><div>+                     llvm::DIFile Unit) {</div>
<div><br></div><div>Watch out. I see tabs at few places!</div><div><br></div>I am not able to apply patch cleanly against mainline svn sources. Please refresh the patch and add a test case.</div><div>Thanks for working on this.</div>
<div>-</div><div>Devang<br><div class="gmail_quote">On Mon, Aug 23, 2010 at 2:21 AM, Alexander Herz <span dir="ltr"><<a href="mailto:alexander.herz@mytum.de">alexander.herz@mytum.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">



  

<div text="#000000" bgcolor="#ffffff">
Any update on whether the corrected version of the patch will be
accepted?<br>
<br>
Thx.<div class="im"><br>
Alex<br>
<br>
On 08/12/2010 07:21 PM, Devang Patel wrote:
</div><div><div></div><div class="h5"><blockquote type="cite">Hi Alex,<br>
  <br>
I'll apply these patches once following issues are resolved. I see few
tabs sneaked into diffs. clang and llvm code is tabs free. Few other
comments...<br>
  <br>
  <br>
  <blockquote style="border:medium none;margin:0pt 0pt 0pt 40px;padding:0px">-
 // Static methods do not need "this" pointer argument.<br>
-  if (Method->isStatic())<br>
-    return FnTy;</blockquote>
  <br>
Why are you removing this check ? All the changes
to getOrCreateMethodType() function are unnecessary and not related to
this patch.
  <div><br>
  <br>
  <br>
  </div>
  <blockquote style="border:medium none;margin:0pt 0pt 0pt 40px;padding:0px">
    <div>+/// CollectCXXFriends - A helper function to collect debug
info for</div>
    <div>+/// C++ base classes. This is used while creating debug info
entry for </div>
    <div>+/// a Record.</div>
    <div>+void CGDebugInfo::</div>
    <div>+CollectCXXFriends(const CXXRecordDecl *RD, llvm::DIFile Unit,</div>
    <div>+              
 llvm::SmallVectorImpl<llvm::DIDescriptor> &EltTys,</div>
    <div>+                llvm::DICompositeType &RecordTy) {</div>
    <div>+</div>
    <div>+  for (CXXRecordDecl::friend_iterator BI =
 RD->friend_begin(),</div>
    <div>+         BE = RD->friend_end(); BI != BE; ++BI) {</div>
    <div>+    unsigned BFlags = 0;</div>
  </blockquote>
  <div><br>
  </div>
  <div>This BFlags is never assigned any other value. Intentional ?</div>
  <div><br>
  </div>
  <div>llvm diffs looks fine.</div>
  <div>Thanks for working on this.</div>
  <div>-</div>
  <div>Devang</div>
</blockquote>
<br>
</div></div></div>

</blockquote></div><br><br clear="all"><br>-- <br>-<br>Devang<br>
</div>