On Wed, Nov 2, 2011 at 12:36 AM, Enea Zaffanella <[email protected]>wrote:
> Il 02/11/2011 03:21, Richard Trieu ha scritto: > > On Mon, Oct 24, 2011 at 7:30 AM, Douglas Gregor <[email protected] > > <mailto:[email protected]>> wrote: > > > > > > On Oct 20, 2011, at 5:36 PM, Chandler Carruth wrote: > > > >> On Thu, Oct 20, 2011 at 5:12 PM, Richard Trieu <[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> Should Clang be printing suffixes that are accepted only with > >> certain flags? > >> > >> > >> I think this is an interesting policy decision. I'd love to hear > >> Doug's thoughts on it. > >> > >> It seems fine to me for Clang, when running with -fms-extensions, > >> to suggest fixes even if only valid for -fms-extensions. Clearly > >> if there is a generic suggestion that could be made, that would be > >> a preferred alternative. For example, '__asm__' should be > >> suggested before 'asm'. > > > > I think it's fine for Clang to print suffixes that are only accepted > > with certain flags. Presumably, you should never get an > > IntegerLiteral of type __int128_t unless you're in a dialect that > > supports parsing it. > > > > … except that we cheat when we're building template arguments, > > because it was convenient. That cheating could be eliminated by > > encoding integer literal values directly within > > SubstNonTypeTemplateParmExpr. > > > > - Doug > > > > > > New patch. Changes as follows: > > Add int128 and uint128 suffixes (i128 and Ui128) to StmtPrinter. short > > and unsigned short will get to llvm_unreachable > > Add assert to IntergerLiteral to prevent creation with type short or > > unsigned short > > Fix comment in IntegerLiteral to say that int128 and uint128 are > > acceptable types > > Change BuildExpressFromIntegralTemplateArgument to give a proper Expr. > > For negative numbers, use UnaryOperator of IntegerLiteral. For short > > and unsigned short, ImplicitCastExpr from int. > > > The case of explictly signed/unsigned char template arguments should be > considered too. Currently clang produces a character literal having a > signed/unsigned char type in those cases, but such a character literal > should typically have plain char type. Would it be possible to improve > the current patch so that an integer literal is used instead (with the > appropriate cast)? > > Yes, that is certainly possible to do. I will play around with it a little and see how it works out. > > Regarding the kind of cast: I am fully aware that here it is mainly a > matter of taste ... but wouldn't it be slightly better to use explicit > casts, so that the required conversions are more clearly readable from > the pretty printed statement? > > For instance, consider the following: > > template <short v> struct S { > static const long a = v; > }; > > S<0> s; > > Here, when initializing the static member `a', the short value `v' is > implicitly converted to long. If we go for implicit casts, we will > obtain a template instance where the 0 integer literal is first > implicitly converted to short and then implicitly converted to long, > which is somehow strange looking. An explicit cast from the integer > literal to short may serve as a better hint for the reader regarding the > fact that such a 0 constant value was originally having type short. > This is what the AST dump looks like: template <short v = 0> struct S { struct S; static const long a = (ImplicitCastExpr 0x4664d48 <unsignedtemplate.cc:24:26> 'const long' <IntegralCast> (SubstNonTypeTemplateParmExpr 0x4664d20 <col:26> 'short' (ImplicitCastExpr 0x4664d08 <col:26> 'short' <IntegralCast> (IntegerLiteral 0x4664ce0 <col:26> 'int' 0)))) ; inline S() throw() (CompoundStmt 0x4664ff8 <unsignedtemplate.cc:23:28>) inline S(const S<0> &) throw(); } The two implicit casts are separated by a SubstNonTypeTemplateParamExpr. It shows the integer literal cast to short to match the non-type template parameter before being case to long for assignment. > > Cheers, > Enea. > > > > Basically, protect IntegerLiteral being shorts, fix the only case of > > short IntegerLiterals, and add support for printing int128 > > IntegerLiterals. > > > > PR: > > http://llvm.org/bugs/show_bug.cgi?id=11179 > > > > Patch also located at: > > http://codereview.appspot.com/5309045/ > > > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
