<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 10/8/2012 8:55 PM, Richard Trieu
wrote:<br>
</div>
<blockquote
cite="mid:CAPUYKWn3Y6R1QY4rOuy0vMFS799YRGhfAatHLv08Abk2mcDvRw@mail.gmail.com"
type="cite">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.
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
cfe-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a>
</pre>
</blockquote>
<br>
A couple of minor things related to comments:<br>
<tt><br>
</tt>
<blockquote type="cite"><tt>@@ -566,6 +581,12 @@</tt><tt><br>
</tt><tt> (FlatTree[ReadNode].FromTD ||
FlatTree[ReadNode].ToTD);</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt> </tt><tt><br>
</tt><tt>+ /// NodeIsAPSInt - Returns true if the </tt><tt><b>arugments</b></tt><tt>
are stored in APSInt's.</tt><tt><br>
</tt></blockquote>
<tt> </tt><b><tt>^</tt></b><b><tt>
misspelling</tt></b><tt><br>
</tt>
<blockquote type="cite"><tt>+ bool NodeIsAPSInt() {</tt><tt><br>
</tt><tt>+ return FlatTree[ReadNode].IsValidFromInt ||</tt><tt><br>
</tt><tt>+ FlatTree[ReadNode].IsValidToInt;</tt><tt><br>
</tt><tt>+ }</tt><tt><br>
</tt><tt>+</tt><tt><br>
</tt></blockquote>
<tt><br>
</tt>
<blockquote type="cite"><tt> // Handle Expressions</tt><tt><br>
</tt></blockquote>
<tt> <b>^ should update comment now that we handle
Integrals</b></tt><tt><br>
</tt>
<blockquote type="cite"><tt> if (NonTypeTemplateParmDecl
*DefaultNTTPD =</tt><tt><br>
</tt><tt>
dyn_cast<NonTypeTemplateParmDecl>(ParamND)) {</tt><tt><br>
</tt><tt> Expr *FromExpr, *ToExpr;</tt><tt><br>
</tt><tt> llvm::APSInt FromInt, ToInt;</tt><tt><br>
</tt><tt> bool HasFromInt = !FromIter.isEnd() &&</tt><tt><br>
</tt><tt> FromIter->getKind() ==
TemplateArgument::Integral;</tt><tt><br>
</tt><tt> bool HasToInt = !ToIter.isEnd() &&</tt><tt><br>
</tt><tt> ToIter->getKind() ==
TemplateArgument::Integral;</tt><br>
</blockquote>
<br>
<br>
And an observation:<br>
<br>
The patch favors converting Exprs to Ints which may lose information
that might be helpful to the user. For example, compiling this:<br>
<blockquote><tt>template <int i> struct A { };</tt><br>
<tt>A<1+6> a1;</tt><br>
<tt>A<5+3> a2;</tt><br>
<tt>void f() { a1 = a2; }</tt><br>
</blockquote>
produces:<br>
<blockquote><tt>error: no viable overloaded '='</tt><tt><br>
</tt><tt>void f() { a1 = a2; }</tt><tt><br>
</tt><tt> ~~ ^ ~~</tt><tt><br>
</tt><tt>note: candidate function (the implicit copy assignment
operator) not viable: no known conversion from <b>'A<8>'
to 'A<7>'</b> for 1st argument</tt><tt><br>
</tt><tt>template <int i> struct A { };</tt><tt><br>
</tt><tt> ^</tt><tt><br>
</tt><tt>1 error generated.</tt><tt><br>
</tt></blockquote>
<br>
If we preserve Exprs in, we get:<br>
<br>
<blockquote><tt>error: no viable overloaded '='</tt><br>
<tt>void f() { a1 = a2; }</tt><br>
<tt> ~~ ^ ~~</tt><br>
<tt>note: candidate function (the implicit copy assignment
operator) not viable: no known conversion from <b>'A<5 +
3>' to 'A<7>'</b> for 1st argument</tt><br>
<tt>template <int i> struct A { };</tt><br>
<tt> ^</tt><br>
<tt>1 error generated.</tt><br>
</blockquote>
<br>
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.<br>
<br>
Otherwise LGTM (though I'm perhaps too much of a newbie here for my
approval to carry much weight).<br>
<br>
Cheers,<br>
Matthew Curtis. <br>
<br>
<br>
<pre class="moz-signature" cols="72">--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation</pre>
</body>
</html>