Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2180)

Issue 5309045: Change the printing of integer literal to handle more types

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 1 month ago by rtrieu
Modified:
8 years ago
Reviewers:
Richard Smith
Base URL:
https://llvm.org/svn/llvm-project/cfe/trunk/
Visibility:
Public.

Description

http://llvm.org/bugs/show_bug.cgi?id=11179 The StmtPrinter has trouble printing IntegerLiteral if the type is short or uint128, or their unsigned variants. This change will prevent Clang from complaining on these types.

Patch Set 1 #

Patch Set 2 : Modify IntegerLiteral to not accept shorts and fix int128 printing #

Total comments: 7

Patch Set 3 : Handle min values better. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+79 lines, -6 lines) Patch
M include/clang/AST/Expr.h View 1 2 2 chunks +5 lines, -2 lines 0 comments Download
M lib/AST/StmtPrinter.cpp View 1 1 chunk +2 lines, -0 lines 0 comments Download
M lib/Sema/SemaTemplate.cpp View 1 2 4 chunks +51 lines, -4 lines 0 comments Download
A test/Misc/integer-literal-printing.cpp View 1 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 2
Richard Smith
http://codereview.appspot.com/5309045/diff/2001/include/clang/AST/Expr.h File include/clang/AST/Expr.h (right): http://codereview.appspot.com/5309045/diff/2001/include/clang/AST/Expr.h#newcode1087 include/clang/AST/Expr.h:1087: "Interger type cannot be short or unsigned short"); Typo ...
8 years ago (2011-11-02 05:37:32 UTC) #1
rtrieu
8 years ago (2011-11-03 01:30:38 UTC) #2
http://codereview.appspot.com/5309045/diff/2001/lib/Sema/SemaTemplate.cpp
File lib/Sema/SemaTemplate.cpp (right):

http://codereview.appspot.com/5309045/diff/2001/lib/Sema/SemaTemplate.cpp#new...
lib/Sema/SemaTemplate.cpp:4204: bool IsNegative = Value.isNegative();
On 2011/11/02 05:37:33, Richard Smith wrote:
> APInt::isNegative just checks the top bit, which won't do the right thing for
> unsigned values. You could grab the full APSInt from the TemplateArgument and
> check isSigned() too.

Done.

http://codereview.appspot.com/5309045/diff/2001/lib/Sema/SemaTemplate.cpp#new...
lib/Sema/SemaTemplate.cpp:4206: Value = Value.abs();
On 2011/11/02 05:37:33, Richard Smith wrote:
> I think this will still be negative if Value is SHRT_MIN etc. In such cases,
you
> may need to switch to the next wider integer type. For LLONG_MIN, I think
you'll
> need something nasty like -LLONG_MAX - 1.

Done.

http://codereview.appspot.com/5309045/diff/2001/lib/Sema/SemaTemplate.cpp#new...
lib/Sema/SemaTemplate.cpp:4212: }
On 2011/11/02 05:37:33, Richard Smith wrote:
> It looks like we'll build a short literal if an enum has an underlying type of
> 'short' or 'unsigned short'. Can we move this check after the enum check
below?

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b