On Thu, May 9, 2013 at 11:32 AM, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div>I updated the white space.</div><div><br></div><div>Are these are the checks for pointers/references to functions that you were looking for? (lines 128-134 of mangle-ms-arg-qualifiers.cpp)</div></div>
</div></blockquote><div><br></div><div>Ah, yes, thanks :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>void foo_p6ahxz(int x()) {}</div>
<div>// CHECK: "\01?foo_p6ahxz@@YAXP6AHXZ@Z"</div><div>// X64: "\01?foo_p6ahxz@@YAXP6AHXZ@Z"</div><div><br></div><div>void foo_a6ahxz(int (&x)()) {}</div>
<div>// CHECK: "\01?foo_a6ahxz@@YAXA6AHXZ@Z"</div><div>// X64: "\01?foo_a6ahxz@@YAXA6AHXZ@Z"</div><div><br></div><div>void foo_q6ahxz(int (&&x)()) {}</div><div>// CHECK: "\01?foo_q6ahxz@@YAX$$Q6AHXZ@Z"</div>
<div>// X64: "\01?foo_q6ahxz@@YAX$$Q6AHXZ@Z"</div></div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, May 8, 2013 at 3:39 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>@@ -1277,6 +1284,8 @@ void MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {</div><div> else</div>
<div> Out << 'Q';</div><div> }</div><div>+ if (PointersAre64Bit && !MD->isStatic())</div><div>
<div>+ Out << 'E';</div><div> } else</div><div> Out << 'Y';</div><div> }</div><div><br></div></div><div>Needs more indentation.</div><div><br></div><div>You have special cases for pointers/references to functions (where no E is added) but I don't see any test cases for that.</div>
<div><br></div><div>The BNF comments should be updated in various places to indicate the presence of this specifier. It might also be useful to factor out the "if (PointersAre64Bit) Out << 'E';" code into a function, and make up a name for it to use in the BNF descriptions.</div>
</blockquote></div></div></div></div></blockquote><div><br></div><div>I'd still like these comments updated. Other than that, patch looks fine.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><div>
On Wed, May 8, 2013 at 1:36 PM, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>
<div dir="ltr">ping</div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 3, 2013 at 12:07 PM, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sounds good. I added a local that caches the 64-bitness of pointers.<span><font color="#888888"><div>
<br></div><div>-Warren</div></font></span></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, May 2, 2013 at 2:51 PM, Charles Davis <span dir="ltr"><<a href="mailto:cdavis5x@gmail.com" target="_blank">cdavis5x@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
On May 2, 2013, at 3:19 PM, Warren Hunt wrote:<br>
<br>
> I've put together a patch that fixes all of the issues I found with Microsoft name mangling in 64-bit mode. Mostly it involves adding an 'E' note to all pointers that are 64-bit (including 'this' pointers). This is done in MicrosoftMangle.cpp. I've updated 3 of the mangling test files with X64 tests, which provides some reasonable amount of coverage.<br>
><br>
> Please review.<br>
><br>
> Thanks,<br>
> -Warren<br>
><br>
> (Note: This is my first patch.)<br>
</div></div>Welcome to Clang development!<br>
<br>
> @@ -1277,6 +1278,9 @@ void MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {<br>
> else<br>
> Out << 'Q';<br>
> }<br>
> + if (getASTContext().getTargetInfo().getPointerWidth(0) == 64 &&<br>
> + !MD->isStatic())<br>
> + Out << 'E';<br>
> } else<br>
> Out << 'Y';<br>
> }<br>
The indentation on this looks wrong. (You can't tell from here, but it's definitely wrong in the patch itself.) Also, I think we should handle this where we mangle in the 'this' qualifiers--the 'E' is because the 'this' pointer is Extended (__ptr64 instead of __ptr32). (The relevant line is 1172.)<br>
<br>
> @@ -1390,6 +1394,8 @@ void MicrosoftCXXNameMangler::mangleDecayedArrayType(const ArrayType *T,<br>
> manglePointerQualifiers(T->getElementType().getQualifiers());<br>
> } else {<br>
> Out << 'Q';<br>
> + if (getASTContext().getTargetInfo().getPointerWidth(0) == 64)<br>
> + Out << 'E';<br>
> }<br>
> mangleType(T->getElementType(), SourceRange());<br>
> }<br>
Why only here? What does VC do in the global case?<br>
<br>
There's a lot of code duplication (i.e. checks against the pointer size sprinkled throughout the code). Most of those can be collapsed into mangleQualifiers().<br>
<br>
Someone else wrote a patch a while back. You can find it here:<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D101" target="_blank">http://llvm-reviews.chandlerc.com/D101</a><br>
<br>
I didn't finish reviewing it because real life got in the way. :\. You might find some ideas in there.<br>
<br>
Chip<br>
<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div><br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br>