ping
On Fri, May 3, 2013 at 12:07 PM, Warren Hunt <[email protected]> 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 <[email protected]> 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 [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
