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

[Charles Davis]

On Jun 11, 2012, at 7:22 PM, Richard Smith wrote:

> 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!
Thanks! r158376
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120612/7d012ab1/attachment.html>