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 <[email protected]> 
>>>> 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.


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to