On Mon, Oct 22, 2012 at 2:37 PM, Matthew Curtis <[email protected]>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 > [email protected]http://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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
