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

[Richard Trieu]

On Mon, Oct 22, 2012 at 3:24 PM, Richard Trieu <rtrieu at google.com> wrote:

>
>
> 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
>>
>>
> Committed at r167252.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121101/028e488b/attachment.html>