[cfe-dev] Fix PR13444 - wrong mangling of "const char * const *" and friends

[Timur Iskhodzhanov]

r163110, thanks!

On Mon, Sep 3, 2012 at 12:54 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> On Fri, Aug 31, 2012 at 8:11 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Fri, Aug 31, 2012 at 7:55 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>>> Can you please review this patch?
>>> This fixes http://llvm.org/PR13444 as well as a large part of
>>> http://llvm.org/PR13182 plus adds quite a few tests.
>>>
>>>
>>> +void MicrosoftCXXNameMangler::manglePointerQualifiers(Qualifiers Quals) {
>>> +  // <pointer-cvr-qualifiers> ::= P  # no qualifiers
>>> +  //                          ::= Q  # const
>>> +  //                          ::= R  # volatile
>>> +  //                          ::= S  # const volatile
>>> +  if (!Quals.hasVolatile()) {
>>> +    if (!Quals.hasConst()) {
>>> +      Out << 'P';
>>> +    } else {
>>> +      Out << 'Q';
>>> +    }
>>> +  } else {
>>> +    if (!Quals.hasConst()) {
>>> +      Out << 'R';
>>> +    } else {
>>> +      Out << 'S';
>>> +    }
>>> +  }
>>> +}
>>
>> I wonder if this could be made a bit more clear, a la something like this:
>>
>> bool HasConst = Quals.hasConst();
>> bool HasVolatile = Quals.hasVolatile();
>>
>> if (hasVolatile && hasConst)
>>   Out << 'S';
>> else if (hasConst)
>>   Out << 'Q';
>> else if (hasVolatile)
>>   Out << 'R';
>> else
>>   Out << 'P';
>>
>> As it stands, I had to think about the logic due to the nesting.
> Actually what you're suggesting is exactly how I initially wrote it :)
> But then I saw MicrosoftCXXNameMangler::mangleQualifiers() and decided
> to take that approach.
> Let me fix both functions then...
>
>> Other than that, it LGTM.  Thanks!
> I suppose your LGTM is enough to commit.
>
>> ~Aaron