Page MenuHomePhabricator

Handle cases where the value is too large to fit into the underlying type.
Needs ReviewPublic

Authored by v.g.vassilev on Jun 30 2017, 2:59 PM.

Details

Reviewers
rsmith
Summary

Patch by Axel Naumann!

Diff Detail

Event Timeline

v.g.vassilev planned changes to this revision.Jun 30 2017, 2:59 PM
v.g.vassilev created this revision.

Waiting for @karies for a test case.

karies added a comment.Jul 3 2017, 2:46 AM

Two comments:

  • I think the part Val.getBitWidth() == 64 likely needs rewording, although I don't know how
  • I can show the motivation, not sure whether that qualifies as a test case:
$ echo 'template <unsigned long long> struct S{}; S<(unsigned long long)-1> s = 42;' | clang++ -fsyntax-only -std=c++14 -x c++ -
<stdin>:1:69: error: no viable conversion from 'int' to 'S<(unsigned long long)-1>'
template <unsigned long long> struct S{}; S<(unsigned long long)-1> s = 42;
                                                                    ^   ~~
<stdin>:1:38: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const S<18446744073709551615> &' for 1st argument
template <unsigned long long> struct S{}; S<(unsigned long long)-1> s = 42;
                                     ^                                  ~~
<stdin>:1:38: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'S<18446744073709551615> &&' for 1st argument
template <unsigned long long> struct S{}; S<(unsigned long long)-1> s = 42;
                                     ^                                  ~~
1 error generated.

The following shows that the name clang usew for diagnostics (S<18446744073709551615> &) is actually invalid:

$ echo 'template <unsigned long long> struct S{}; S<18446744073709551615> a;' | clang++ -fsyntax-only -std=c++14 -x c++ -
<stdin>:1:45: warning: integer literal is too large to be represented in a signed integer type, interpreting as unsigned [-Wimplicitly-unsigned-literal]
template <unsigned long long> struct S{}; S<18446744073709551615> a;
                                            ^
1 warning generated.

My goal is for clang to print a valid class name, which means this should be spelled S<18446744073709551615ull>`.

Add test case.

Sorry, undo ping... we have a test case.

rsmith added inline comments.Aug 27 2017, 1:56 PM
lib/AST/TemplateBase.cpp
64

If Val is LLONG_MIN, this will still produce an integer too large for any signed type.

68

We shouldn't be assuming that long long is exactly 64 bits here. Perhaps we could produce an U/L/UL/LL/ULL suffix based on the particular builtin type, regardless of the value itself?

68–69

This is not sufficient to ensure a correct round-trip in all cases; for template<auto>, we would need the type of the template argument to exactly match the type of the printed expression. But if you don't want to address that case now, that's fine.

69

This should be uppercase, to match how we pretty-print IntegerLiterals.