<div class="gmail_quote">On Mon, Aug 27, 2012 at 11:04 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cool, a couple of high level comments:<div><br></div><div><div>@@ -778,17 +793,19 @@</div><div>           if (Context.hasSameType(FromType, ToType)) {</div><div>             Tree.SetSame(true);</div><div>           } else {</div>

<div>-            const TemplateSpecializationType *FromArgTST =</div><div>-                GetTemplateSpecializationType(Context, FromType);</div><div>-            const TemplateSpecializationType *ToArgTST =</div><div>
-                GetTemplateSpecializationType(Context, ToType);</div>
<div>-</div><div>-            if (FromArgTST && ToArgTST) {</div><div>-              bool SameTemplate = hasSameTemplate(FromArgTST, ToArgTST);</div><div>-              if (SameTemplate) {</div><div>-                Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),</div>

<div>-                             ToArgTST->getTemplateName().getAsTemplateDecl());</div><div>-                DiffTemplate(FromArgTST, ToArgTST);</div><div>+            Qualifiers FromQual, ToQual;</div><div>+            bool SameTemplate = hasSameTemplate(Context, FromType, ToType,</div>

<div>+                                                FromQual, ToQual);</div><div>+            if (SameTemplate) {</div><div>+              const TemplateSpecializationType *FromArgTST =</div><div>+                  GetTemplateSpecializationType(Context, FromType);</div>

<div>+              const TemplateSpecializationType *ToArgTST =</div><div>+                  GetTemplateSpecializationType(Context, ToType);</div><div>+              if (FromArgTST && ToArgTST) {</div><div>+              Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),</div>

<div>+                           ToArgTST->getTemplateName().getAsTemplateDecl());</div><div>+              Tree.SetNode(FromQual, ToQual);</div><div>+              DiffTemplate(FromArgTST, ToArgTST);</div><div>               }</div>

<div>             }</div><div>           }</div></div><div><br></div><div>The indent here is now off... and this is crazily nested.</div><div><br></div><div>Can we factor this function into smaller pieces with less nesting? I think that would make it much easier to read.</div>

<div><br></div><div><div>@@ -824,62 +841,120 @@</div><div>     }</div><div>   }</div><div> </div><div>+  static void RemoveQualifiers(Qualifiers &Qual, Qualifiers RemoveQual) {</div></div><div><br></div><div>I feel like this should be a method on Qualifiers?</div>

<div><br></div><div><div>+  // Removes any common qualifiers from ToQual and FromQual and places them</div><div>+  // in CommonQual.</div><div>+  void DiffQualifiers(Qualifiers &FromQual, Qualifiers &ToQual,</div>

</div><div><br></div><div>I feel like this should also be provided by Qualifiers -- probably as a static factory function?</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"></div></div></div></blockquote>
<div> </div><div>Good idea.  I'll make a new patch for the Qualifiers stuff.</div></div>