<div>Hi Richard!</div><div><br></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><br></div>
<div>Missing some indentation here.</div><div><br></div><div><div>+ FromType = FromIter[-1].second;</div><div>+ ToType = ToIter[-1].second;</div><div>+</div><div>+ // Remove all the qualifiers that are already in the type. This gets</div>
<div>+ // all the qualifiers that are added between this qualifier and the</div><div>+ // original type passed into this function.</div><div>+ FromQual -= FromType.getLocalUnqualifiedType().getQualifiers();</div>
<div><br></div></div><div>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()")?</div>
<div><br></div><div>Thanks!</div><br><div class="gmail_quote">On Thu, Sep 13, 2012 at 4:44 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Mon, Aug 27, 2012 at 4:29 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><div>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><div><div class="gmail_extra"></div></div></div></blockquote>
<div> </div></div></div><div>Good idea. I'll make a new patch for the Qualifiers stuff.</div></div>
</blockquote></div><br></div></div><div>Qualifiers functions pulled out and committed in a separate patch. Updated patch attached.</div>
</blockquote></div><br>