[cfe-commits] [Patch] Add handling of APSInt to Template Type Diffing - Fixes PR14015

[Richard Trieu]

On Mon, Oct 22, 2012 at 2:37 PM, Matthew Curtis <mcurtis at codeaurora.org>wrote:

>  On 10/8/2012 8:55 PM, Richard Trieu wrote:
>
> An assumption during the creation of Template Type Diffing expected that
> integral arguments would be available as Expr's (e.g. IntegralLiteral).
>  However, it appears that in some cases, the TemplateArgument gives an
> Expr, and in others, TemplateArgument only gives an APSInt.  This patch
> allows Template Type Diffing to handle APSInt by storing in inside the tree
> nodes, as well as other bits of information needed.  Also, in cases where
> one argument is given as an Expr and the other as an APSInt, evaluate the
> Expr and store the result as an APSInt.
>
> _______________________________________________
> cfe-commits mailing listcfe-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
> A couple of minor things related to comments:
>
>  @@ -566,6 +581,12 @@
>               (FlatTree[ReadNode].FromTD || FlatTree[ReadNode].ToTD);
>      }
>
> +    /// NodeIsAPSInt - Returns true if the *arugments* are stored in
> APSInt's.
>
>                                               *^** misspelling*
>
> +    bool NodeIsAPSInt() {
> +      return FlatTree[ReadNode].IsValidFromInt ||
> +             FlatTree[ReadNode].IsValidToInt;
> +    }
> +
>
>
>        // Handle Expressions
>
>            *^ should update comment now that we handle Integrals*
>
>       if (NonTypeTemplateParmDecl *DefaultNTTPD =
>               dyn_cast<NonTypeTemplateParmDecl>(ParamND)) {
>         Expr *FromExpr, *ToExpr;
>         llvm::APSInt FromInt, ToInt;
>         bool HasFromInt = !FromIter.isEnd() &&
>                           FromIter->getKind() ==
> TemplateArgument::Integral;
>         bool HasToInt = !ToIter.isEnd() &&
>                         ToIter->getKind() == TemplateArgument::Integral;
>
>
>
> And an observation:
>
> The patch favors converting Exprs to Ints which may lose information that
> might be helpful to the user. For example, compiling this:
>
> template <int i> struct A { };
> A<1+6> a1;
> A<5+3> a2;
> void f() { a1 = a2; }
>
> produces:
>
> error: no viable overloaded '='
> void f() { a1 = a2; }
>            ~~ ^ ~~
> note: candidate function (the implicit copy assignment operator) not
> viable: no known conversion from *'A<8>' to 'A<7>'* for 1st argument
> template <int i> struct A { };
>                         ^
> 1 error generated.
>
>
> If we preserve Exprs in, we get:
>
> error: no viable overloaded '='
> void f() { a1 = a2; }
>            ~~ ^ ~~
> note: candidate function (the implicit copy assignment operator) not
> viable: no known conversion from *'A<5 + 3>' to 'A<7>'* for 1st argument
> template <int i> struct A { };
>                         ^
> 1 error generated.
>
>
> Not sure how important this is, just something I noticed. In this simple
> example it's obvious that '8' is '5 + 3', but I could see a more
> complicated expression causing confusion.
>

It doesn't seem as useful to have the whole expression on one side when the
other side already has 1+6 changed to 7.  This also allows for the
assumption that if the two templates have an argument, they are of the same
type.

>
> Otherwise LGTM (though I'm perhaps too much of a newbie here for my
> approval to carry much weight).
>
> Cheers,
> Matthew Curtis.
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121022/ab0c77e7/attachment.html>