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

[Richard Trieu]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120827/2f38aad2/attachment.html>