Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor <msebor@gmail.com>]

On 06/07/2016 01:42 PM, Jakub Jelinek wrote:
> On Tue, Jun 07, 2016 at 03:35:28PM -0400, Jason Merrill wrote:
>>> Indeed, confirming removal of the cp/tree.c hunk doesn't affect anything in
>>> the testsuite, nor e.g.
>>> enum A {
>>>   B = 1,
>>>   C = 2,
>>>   D = __builtin_add_overflow_p (B, C, C)
>>> };
>>> int e[__builtin_add_overflow_p (B, C, C) + 1];
>>> template <int N> int foo (int);
>>> void
>>> bar ()
>>> {
>>>   foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
>>> }
>>> That said, builtin_valid_in_constant_expr_p is used in lots of spots, and in
>>> various of them I see that without the change it actually sets
>>> *non_integral_constant_expression_p = true;
>>> or something similar, but I have no idea why it still works despite of that.
>>
>> Ah, that's the C++98 constant expression handling, which is a lot more
>> restricted.  C++11 and up don't care about that flag.
>
> Oops, actually, it seems even the cp/tree.c hunks are significant:
>
> I've only compiled the above testcase with the default -std=c++14, with
> -std=c++98 without the cp/tree.c bits I get:
>
> a.C:4:7: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
> constant-expression
>     D = __builtin_add_overflow_p (B, C, C)
>         ^~~~~~~~~~~~~~~~~~~~~~~~
> a.C:4:40: error: a function call cannot appear in a constant-expression
>     D = __builtin_add_overflow_p (B, C, C)
>                                          ^
> a.C:6:45: error: array bound is not an integer constant before ‘]’ token
>   int e[__builtin_add_overflow_p (B, C, C) + 1];
>                                               ^
> a.C: In function ‘void bar()’:
> a.C:11:8: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
> constant-expression
>     foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
>          ^~~~~~~~~~~~~~~~~~~~~~~~
> a.C:11:41: error: a function call cannot appear in a constant-expression
>     foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
>                                           ^
> a.C:11:50: error: no matching function for call to ‘foo(int)’
>     foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
>                                                    ^
> a.C:7:22: note: candidate: template<int N> int foo(int)
>   template <int N> int foo (int);
>                        ^~~
> a.C:7:22: note:   template argument deduction/substitution failed:
> a.C:11:50: error: template argument 1 is invalid
>     foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
>                                                    ^
>
> Though, maybe it is only worth supporting the __builtin_*_overflow_p
> builtins for C++98 integer constant expression contexts and
> thus only handle there the 3 builtins instead of all the others.
>
> I guess I should add this testcase to the testsuite then.

I've also since discovered that the handling of (at least)
BUILTIN_LINE is important in C++ 98 mode and if I had run
the c-c++-common tests instead of just the strict C++ subset
I would have seen the c-c++-common/builtin_location.c test
fail with the same error.

I suspect I added the other built-ins to the function to get
the Clang-compatibility typed built-ins to work in C++ 98
(with the default argument).  When it was decided that
the Clang built-ins shouldn't change I removed the tests
but not this change.

Martin