On 06/07/2016 12:34 PM, Jakub Jelinek wrote:
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.

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.

Is the patch ok for trunk without the cp/tree.c hunk?

Yes.

Jason

Reply via email to