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

Timur Iskhodzhanov timurrrr at google.com
Mon Sep 3 06:15:17 PDT 2012


and r163111 for the new test file - blame svn for that :)

On Mon, Sep 3, 2012 at 1:09 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> 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



More information about the cfe-dev mailing list