Microsoft X64 Mangling

[Richard Smith]

@@ -1277,6 +1284,8 @@ void
MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
         else
           Out << 'Q';
     }
+  if (PointersAre64Bit && !MD->isStatic())
+    Out << 'E';
   } else
     Out << 'Y';
 }

Needs more indentation.

You have special cases for pointers/references to functions (where no E is
added) but I don't see any test cases for that.

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.

On Wed, May 8, 2013 at 1:36 PM, Warren Hunt <whunt at google.com> wrote:

> ping
>
>
> On Fri, May 3, 2013 at 12:07 PM, Warren Hunt <whunt at google.com> wrote:
>
>> Sounds good.  I added a local that caches the 64-bitness of pointers.
>>
>> -Warren
>>
>>
>> On Thu, May 2, 2013 at 2:51 PM, Charles Davis <cdavis5x at gmail.com> wrote:
>>
>>>
>>> On May 2, 2013, at 3:19 PM, Warren Hunt wrote:
>>>
>>> > 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.
>>> >
>>> > Please review.
>>> >
>>> > Thanks,
>>> > -Warren
>>> >
>>> > (Note: This is my first patch.)
>>> Welcome to Clang development!
>>>
>>> > @@ -1277,6 +1278,9 @@ void
>>> MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
>>> >          else
>>> >            Out << 'Q';
>>> >      }
>>> > +  if (getASTContext().getTargetInfo().getPointerWidth(0) == 64 &&
>>> > +      !MD->isStatic())
>>> > +    Out << 'E';
>>> >    } else
>>> >      Out << 'Y';
>>> >  }
>>> 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.)
>>>
>>> > @@ -1390,6 +1394,8 @@ void
>>> MicrosoftCXXNameMangler::mangleDecayedArrayType(const ArrayType *T,
>>> >      manglePointerQualifiers(T->getElementType().getQualifiers());
>>> >    } else {
>>> >      Out << 'Q';
>>> > +    if (getASTContext().getTargetInfo().getPointerWidth(0) == 64)
>>> > +      Out << 'E';
>>> >    }
>>> >    mangleType(T->getElementType(), SourceRange());
>>> >  }
>>> Why only here? What does VC do in the global case?
>>>
>>> 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().
>>>
>>> Someone else wrote a patch a while back. You can find it here:
>>>
>>> http://llvm-reviews.chandlerc.com/D101
>>>
>>> I didn't finish reviewing it because real life got in the way. :\. You
>>> might find some ideas in there.
>>>
>>> Chip
>>>
>>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130508/9a1282b3/attachment.html>