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

[Richard Smith]

Hi Richard!

@@ -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);

Missing some indentation here.

+    FromType = FromIter[-1].second;
+    ToType = ToIter[-1].second;
+
+    // Remove all the qualifiers that are already in the type.  This gets
+    // all the qualifiers that are added between this qualifier and the
+    // original type passed into this function.
+    FromQual -= FromType.getLocalUnqualifiedType().getQualifiers();

It looks like that's the same as QualType(FromIter[-1].first,
0).getQualifiers(). Using that would avoid the need to track QualTypes as
you descend through alias templates. Maybe also move this computation out
into the caller (where it's "QualType(FromArgTST, 0).getQualifiers()")?

Thanks!

On Thu, Sep 13, 2012 at 4:44 PM, Richard Trieu <rtrieu at google.com> wrote:

> 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/20120926/c9916380/attachment.html>