[cfe-commits] [Patch] Handle qualifiers for template types in during template type diffing.

[Richard Trieu]

On Mon, Aug 27, 2012 at 4:29 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Mon, Aug 27, 2012 at 11:04 AM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> Cool, a couple of high level comments:
>>
>> @@ -778,17 +793,19 @@
>>            if (Context.hasSameType(FromType, ToType)) {
>>              Tree.SetSame(true);
>>            } else {
>> -            const TemplateSpecializationType *FromArgTST =
>> -                GetTemplateSpecializationType(Context, FromType);
>> -            const TemplateSpecializationType *ToArgTST =
>> -                GetTemplateSpecializationType(Context, ToType);
>> -
>> -            if (FromArgTST && ToArgTST) {
>> -              bool SameTemplate = hasSameTemplate(FromArgTST, ToArgTST);
>> -              if (SameTemplate) {
>> -
>>  Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),
>> -
>> ToArgTST->getTemplateName().getAsTemplateDecl());
>> -                DiffTemplate(FromArgTST, ToArgTST);
>> +            Qualifiers FromQual, ToQual;
>> +            bool SameTemplate = hasSameTemplate(Context, FromType,
>> ToType,
>> +                                                FromQual, ToQual);
>> +            if (SameTemplate) {
>> +              const TemplateSpecializationType *FromArgTST =
>> +                  GetTemplateSpecializationType(Context, FromType);
>> +              const TemplateSpecializationType *ToArgTST =
>> +                  GetTemplateSpecializationType(Context, ToType);
>> +              if (FromArgTST && ToArgTST) {
>> +
>>  Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),
>> +
>> ToArgTST->getTemplateName().getAsTemplateDecl());
>> +              Tree.SetNode(FromQual, ToQual);
>> +              DiffTemplate(FromArgTST, ToArgTST);
>>                }
>>              }
>>            }
>>
>> The indent here is now off... and this is crazily nested.
>>
>> Can we factor this function into smaller pieces with less nesting? I
>> think that would make it much easier to read.
>>
>> @@ -824,62 +841,120 @@
>>      }
>>    }
>>
>> +  static void RemoveQualifiers(Qualifiers &Qual, Qualifiers RemoveQual) {
>>
>> I feel like this should be a method on Qualifiers?
>>
>> +  // Removes any common qualifiers from ToQual and FromQual and places
>> them
>> +  // in CommonQual.
>> +  void DiffQualifiers(Qualifiers &FromQual, Qualifiers &ToQual,
>>
>> I feel like this should also be provided by Qualifiers -- probably as a
>> static factory function?
>>
>
> Good idea.  I'll make a new patch for the Qualifiers stuff.
>

Qualifiers functions pulled out and committed in a separate patch.  Updated
patch attached.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120913/505ea7f8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qualifers-type-diffing3.patch
Type: application/octet-stream
Size: 25457 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120913/505ea7f8/attachment.obj>