This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] integer overflow checking builtins in constant expressions
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>, Martin Sebor <msebor at gmail dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Tue, 7 Jun 2016 18:34:14 +0200
- Subject: Re: [PATCH] integer overflow checking builtins in constant expressions
- Authentication-results: sourceware.org; auth=none
- References: <20160602072316 dot GY28550 at tucnak dot redhat dot com> <5750BF82 dot 5030203 at gmail dot com> <5750CF25 dot 1080801 at gmail dot com> <20160603070637 dot GA7387 at tucnak dot redhat dot com> <5751A26C dot 1060201 at gmail dot com> <20160603153235 dot GK7387 at tucnak dot redhat dot com> <5751E408 dot 1010707 at gmail dot com> <20160606123617 dot GW7387 at tucnak dot redhat dot com> <18c82043-baa5-8ae1-e551-70b3b8241056 at redhat dot com> <5756ED69 dot 1030608 at gmail dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Jun 07, 2016 at 09:51:05AM -0600, Martin Sebor wrote:
> On 06/07/2016 08:32 AM, Jason Merrill wrote:
> >On 06/06/2016 08:36 AM, Jakub Jelinek wrote:
> >>@@ -352,6 +352,35 @@ builtin_valid_in_constant_expr_p (const_
> >> case BUILT_IN_FUNCTION:
> >> case BUILT_IN_LINE:
> >>
> >>+ /* The following built-ins are valid in constant expressions
> >>+ when their arguments are. */
> >>+ case BUILT_IN_ADD_OVERFLOW:
> >>+ case BUILT_IN_ADD_OVERFLOW_P:
> >....
> >
> >Why is this necessary? builtin_valid_in_constant_expr_p is only needed
> >for builtins that can have constant values even if their operands are
> >non-constant, which doesn't apply here.
> >
> >For that matter, I don't see why we needed to add _FUNCTION et al, either.
>
> I don't remember what prompted me to change the function in either
> patch. Perhaps it was the comment above the function that says
> this:
>
> /* Test whether DECL is a builtin that may appear in a
> constant-expression. */
>
> But removing the change doesn't seem to affect the C++ test results
> for the two features so it looks like the change may not be needed.
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.
Is the patch ok for trunk without the cp/tree.c hunk?
Jakub