[cfe-commits] [PATCH] Grab bag of Microsoft C++ ABI Mangler fixes

[Richard Smith]

On Sun, Jun 10, 2012 at 10:51 PM, Charles Davis <cdavis at mymail.mines.edu>wrote:

>
> On Jun 9, 2012, at 5:49 PM, Richard Smith wrote:
>
> On Sat, Jun 9, 2012 at 2:41 PM, Charles Davis <cdavis at mymail.mines.edu>wrote:
>
>> Hi,
>>
>> This is a grab bag of various fixes to the VC++ name mangler.
>>
>> - The mangler is now capable of mangling the names of virtual function
>> tables--but not virtual base tables. That needs a serious refactoring of
>> the ManglerContext's interface.
>> - Local names are now mangled correctly--complete with anonymous numbered
>> scopes. I needed this to make the virtual table test case--which I copied
>> from the Itanium ABI test cases--work.
>> - Instead of throwing an llvm_unreachable(), unimplemented cases now
>> raise a compiler error. (This was the hardest part, since I needed source
>> locations to give to the DiagnosticsEngine.
>>
>> This is a big patch, so I wanted to run it past the list before I commit.
>> OK to commit?
>>
>
> @@ -1087,10 +1211,22 @@
>        Dimensions.push_back(CAT->getSize());
>        ElementTy = CAT->getElementType();
>      } else if (ElementTy->isVariableArrayType()) {
> -      llvm_unreachable("Don't know how to mangle VLAs!");
> +      const VariableArrayType *VAT =
> +        static_cast<const VariableArrayType *>(ElementTy.getTypePtr());
>
> Don't do this, use ASTContext::getAsVariableArrayType(ElementTy) instead.
>
> Now how did that get past? Fixed.
>
>
> Can you use a SmallVector<TemplateArgumentLoc> rather than allocating
> ASTTemplateArgumentListInfo objects in isTemplate?
>
> Done.
>
>
> Several of your "cannot mangle this yet" diagnostics have unwanted
> fallthrough. Should these be fatal?
>
> I've fixed all of them to return early.
>

I think there are now cases where two declarations mangle to the same name,
which could result in a crash in IRGen after your custom error is produced,
but I don't think that's a disaster given the context.

> I would remove "Oddly enough, " throughout.
>
> Done. It is too informal anyway.
>
>
> Is there a reason to create a new test file rather than adding tests to
> mangle-ms.cpp?
>
> Because the corresponding tests for the Itanium ABI (on which these are
> based) were in a separate file (cf.
> <clang>/test/CodeGenCXX/mangle-abi-examples.cpp).
>
> New patch attached. OK to commit?
>

Sure. Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120611/b4be2853/attachment.html>