On Thu, Sep 13, 2012 at 4:44 PM, Richard Trieu <[email protected]> wrote:

> On Mon, Aug 27, 2012 at 4:29 PM, Richard Trieu <[email protected]> wrote:
>
>> On Mon, Aug 27, 2012 at 11:04 AM, Chandler Carruth 
>> <[email protected]>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.
>

Another patch.  Fix some issues with handling of templates in typedefs and
added a test case for them.

Attachment: qualifers-type-diffing4.patch
Description: Binary data

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

Reply via email to