On Mon, Oct 22, 2012 at 3:24 PM, Richard Trieu <rtr...@google.com> wrote:
> > > On Mon, Oct 22, 2012 at 2:37 PM, Matthew Curtis <mcur...@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-comm...@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.
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits