[cfe-commits] [Windows] Properly mangle templates

[Charles Davis]

Thanks, r157524.

Chip

On May 25, 2012, at 7:56 PM, John McCall wrote:

> On May 24, 2012, at 10:03 PM, Charles Davis wrote:
>> On May 24, 2012, at 5:53 AM, John McCall wrote:
>>> On May 24, 2012, at 4:35 AM, Timur Iskhodzhanov wrote:
>>>> On Thu, May 24, 2012 at 2:44 PM, João Matos <ripzonetriton at gmail.com> wrote:
>>>>>>> Attached is a patch that introduces template mangling (at least in the
>>>>>>> simple cases) as well as some tests for the code.
>>>>>>> 
>>>>>>> Can you please review it?
>>>>> I'm only wondering if there is any other template argument kind we need to handle.
>>>> We'll find out as we compile more code with Clang.
>>>> Currently, even simple programs with templates are miscompiled, the
>>>> suggested patch should cover most of the cases.
>>>> I hope all other cases will hit the assertion and then we can deal
>>>> with minimized test cases.
>>>> 
>>>> The current patch (plus one more patch to handle back referenes)
>>>> mangles templates from googletest and iostream just fine (at least
>>>> without obvious problems).
>>>> 
>>>>> The patch looks fine to me.
>>>> Can I commit or should I wait for John's review?
>>> 
>>> Please wait for my review;  I'll get to it before the end of the week.
>> Here's a patch of my own that has the various pieces of Timur's patch factored into their own functions. (I think it's nicer this way, and it matches the layout of ItaniumMangle.cpp more closely. Your call.)
> 
> I like it factored like this, thank you.
> 
> +void MicrosoftCXXNameMangler::mangleNumber(llvm::APSInt Value) {
> 
> Take this by reference, please.
> 
> +    default:
> +      llvm_unreachable("Don't know how to mangle this kind of template argument yet!");
> 
> Never use unreachable for as-yet-unsupported features.  Report an error instead;  ItaniumMangle.cpp has some examples of how to do that from here.  If an error is completely blocking work, at least emit a warning.
> 
> With these changes, this is approved.
> 
> John.