Re: [C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

2017-02-22 Thread Jakub Jelinek 
On Wed, Feb 22, 2017 at 01:58:28PM -0800, Jason Merrill wrote:
> On Wed, Feb 22, 2017 at 1:50 PM, Jakub Jelinek  wrote:
> > This patch does a few things:
> > 2) all but one error in potential_constant_expression_1 to error_at with
> > location_of (t)
> 
> I'd prefer to calculate the location once at the top of the function
> rather than at every call to error_at.  Given that, I think
> EXPR_LOC_OR_LOC is better than location_of, as the latter will give
> the wrong result for a decl.  OK with that change.

Ok, here is what I've committed then:

2017-02-22  Jakub Jelinek  

PR c++/79664
* parser.c (cp_parser_omp_teams, cp_parser_omp_target): Use
SET_EXPR_LOCATION on OMP_TARGET/OMP_TEAMS tree.
* constexpr.c (potential_constant_expression_1): Handle
OMP_*, OACC_* and CILK_* trees.  Use error_at with
EXPR_LOC_OR_LOC (t, input_location) computed early
instead of error, or error_at with location_of (t).

* g++.dg/gomp/teams-1.C: Adjust expected diagnostic location.
* g++.dg/cpp1y/constexpr-throw.C: Likewise.
* g++.dg/gomp/pr79664.C: New test.

--- gcc/cp/parser.c.jj  2017-02-22 22:32:36.659695471 +0100
+++ gcc/cp/parser.c 2017-02-22 23:00:21.472093702 +0100
@@ -35526,6 +35526,7 @@ cp_parser_omp_teams (cp_parser *parser,
  OMP_TEAMS_CLAUSES (ret) = clauses;
  OMP_TEAMS_BODY (ret) = body;
  OMP_TEAMS_COMBINED (ret) = 1;
+ SET_EXPR_LOCATION (ret, loc);
  return add_stmt (ret);
}
 }
@@ -35547,6 +35548,7 @@ cp_parser_omp_teams (cp_parser *parser,
   TREE_TYPE (stmt) = void_type_node;
   OMP_TEAMS_CLAUSES (stmt) = clauses;
   OMP_TEAMS_BODY (stmt) = cp_parser_omp_structured_block (parser, if_p);
+  SET_EXPR_LOCATION (stmt, loc);
 
   return add_stmt (stmt);
 }
@@ -35965,6 +35967,7 @@ cp_parser_omp_target (cp_parser *parser,
  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
  OMP_TARGET_BODY (stmt) = body;
  OMP_TARGET_COMBINED (stmt) = 1;
+ SET_EXPR_LOCATION (stmt, pragma_tok->location);
  add_stmt (stmt);
  pc = _TARGET_CLAUSES (stmt);
  goto check_clauses;
--- gcc/cp/constexpr.c.jj   2017-02-22 22:32:34.490723479 +0100
+++ gcc/cp/constexpr.c  2017-02-22 23:03:12.472864242 +0100
@@ -5001,10 +5001,11 @@ potential_constant_expression_1 (tree t,
 return false;
   if (t == NULL_TREE)
 return true;
+  location_t loc = EXPR_LOC_OR_LOC (t, input_location);
   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
 {
   if (flags & tf_error)
-error ("expression %qE has side-effects", t);
+error_at (loc, "expression %qE has side-effects", t);
   return false;
 }
   if (CONSTANT_CLASS_P (t))
@@ -5086,8 +5087,7 @@ potential_constant_expression_1 (tree t,
  {
/* fold_call_expr can't do anything with IFN calls.  */
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
-   "call to internal function %qE", t);
+ error_at (loc, "call to internal function %qE", t);
return false;
  }
  }
@@ -5105,8 +5105,8 @@ potential_constant_expression_1 (tree t,
  {
if (flags & tf_error)
  {
-   error_at (EXPR_LOC_OR_LOC (t, input_location),
- "call to non-constexpr function %qD", fun);
+   error_at (loc, "call to non-constexpr function %qD",
+ fun);
explain_invalid_constexpr_fn (fun);
  }
return false;
@@ -5199,8 +5199,7 @@ potential_constant_expression_1 (tree t,
&& !integer_zerop (from))
  {
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
-   "reinterpret_cast from integer to pointer");
+ error_at (loc, "reinterpret_cast from integer to pointer");
return false;
  }
 return (RECUR (from, TREE_CODE (t) != VIEW_CONVERT_EXPR));
@@ -5266,7 +5265,7 @@ potential_constant_expression_1 (tree t,
&& !DECL_DECLARED_CONSTEXPR_P (DECL_CONTEXT (x)))
  {
if (flags & tf_error)
- error ("use of % in a constant expression");
+ error_at (loc, "use of % in a constant expression");
return false;
  }
return true;
@@ -5354,10 +5353,40 @@ potential_constant_expression_1 (tree t,
 case DELETE_EXPR:
 case VEC_DELETE_EXPR:
 case THROW_EXPR:
+case OMP_PARALLEL:
+case OMP_TASK:
+case OMP_FOR:
+case OMP_DISTRIBUTE:
+case OMP_TASKLOOP:
+case OMP_TEAMS:
+case OMP_TARGET_DATA:
+case OMP_TARGET:
+case OMP_SECTIONS:
+case OMP_ORDERED:

Re: [C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

2017-02-22 Thread Jason Merrill 
On Wed, Feb 22, 2017 at 1:50 PM, Jakub Jelinek  wrote:
> This patch does a few things:
> 2) all but one error in potential_constant_expression_1 to error_at with
> location_of (t)

I'd prefer to calculate the location once at the top of the function
rather than at every call to error_at.  Given that, I think
EXPR_LOC_OR_LOC is better than location_of, as the latter will give
the wrong result for a decl.  OK with that change.

Jason

[C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

2017-02-22 Thread Jakub Jelinek 
Hi!

This patch does a few things:
1) replaces ICEs with diagnostics + return false in
   potential_constant_expression_1 for OpenMP/OpenACC/Cilk+ constructs,
   I believe unless the upcoming versions of those standards say otherwise,
   we should treat the constructs as invalid in constexpr
When writing a testcase for 1), I've noticed very odd locus for all the
error messages, so I've changed
2) all but one error in potential_constant_expression_1 to error_at with
location_of (t)
and finally
3) #pragma omp target/teams in some cases had wrong locus too

While 1) and 3) I think I can commit with my OpenMP maintainer hat on,
I'm seeking approval for 2).

The only spot in potential_constant_expression_1 I've kept error
is for division by zero, which breaks several g++.dg/warn/overflow-warn*.C
tests and I have no idea what's going on in dejagnu there (both patched
and unpatched compilers emit 4 different diagnostics on a single line
there, the difference is just in one of those diagnostics (different
column), but the testcases actually try to match only 3 diagnostics and
the wording of some of them is very similar and one of the expected
diagnostics is pruned somewhere during the dejagnu processing).
After fighting with that for a while I gave up.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-02-22  Jakub Jelinek  

PR c++/79664
* parser.c (cp_parser_omp_teams, cp_parser_omp_target): Use
SET_EXPR_LOCATION on OMP_TARGET/OMP_TEAMS tree.
* constexpr.c (potential_constant_expression_1): Handle
OMP_*, OACC_* and CILK_* trees.  Use error_at with location_of (t)
instead of error, use location_of (t) instead of
EXPR_LOC_OR_LOC (t, input_location).

* g++.dg/gomp/teams-1.C: Adjust expected diagnostic location.
* g++.dg/cpp1y/constexpr-throw.C: Likewise.
* g++.dg/gomp/pr79664.C: New test.

--- gcc/cp/parser.c.jj  2017-02-20 13:43:21.0 +0100
+++ gcc/cp/parser.c 2017-02-22 17:17:39.176918357 +0100
@@ -35519,6 +35519,7 @@ cp_parser_omp_teams (cp_parser *parser,
  OMP_TEAMS_CLAUSES (ret) = clauses;
  OMP_TEAMS_BODY (ret) = body;
  OMP_TEAMS_COMBINED (ret) = 1;
+ SET_EXPR_LOCATION (ret, loc);
  return add_stmt (ret);
}
 }
@@ -35540,6 +35541,7 @@ cp_parser_omp_teams (cp_parser *parser,
   TREE_TYPE (stmt) = void_type_node;
   OMP_TEAMS_CLAUSES (stmt) = clauses;
   OMP_TEAMS_BODY (stmt) = cp_parser_omp_structured_block (parser, if_p);
+  SET_EXPR_LOCATION (stmt, loc);
 
   return add_stmt (stmt);
 }
@@ -35958,6 +35960,7 @@ cp_parser_omp_target (cp_parser *parser,
  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
  OMP_TARGET_BODY (stmt) = body;
  OMP_TARGET_COMBINED (stmt) = 1;
+ SET_EXPR_LOCATION (stmt, pragma_tok->location);
  add_stmt (stmt);
  pc = _TARGET_CLAUSES (stmt);
  goto check_clauses;
--- gcc/cp/constexpr.c.jj   2017-02-21 18:56:43.0 +0100
+++ gcc/cp/constexpr.c  2017-02-22 17:16:53.413507546 +0100
@@ -5004,7 +5004,8 @@ potential_constant_expression_1 (tree t,
   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
 {
   if (flags & tf_error)
-error ("expression %qE has side-effects", t);
+error_at (location_of (t),
+ "expression %qE has side-effects", t);
   return false;
 }
   if (CONSTANT_CLASS_P (t))
@@ -5086,7 +5087,7 @@ potential_constant_expression_1 (tree t,
  {
/* fold_call_expr can't do anything with IFN calls.  */
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
+ error_at (location_of (t),
"call to internal function %qE", t);
return false;
  }
@@ -5105,7 +5106,7 @@ potential_constant_expression_1 (tree t,
  {
if (flags & tf_error)
  {
-   error_at (EXPR_LOC_OR_LOC (t, input_location),
+   error_at (location_of (t),
  "call to non-constexpr function %qD", fun);
explain_invalid_constexpr_fn (fun);
  }
@@ -5199,7 +5200,7 @@ potential_constant_expression_1 (tree t,
&& !integer_zerop (from))
  {
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
+ error_at (location_of (t),
"reinterpret_cast from integer to pointer");
return false;
  }
@@ -5266,7 +5267,8 @@ potential_constant_expression_1 (tree t,
&& !DECL_DECLARED_CONSTEXPR_P (DECL_CONTEXT (x)))
  {
if (flags & tf_error)
- error ("use of % in a constant expression");
+ error_at (location_of (t),
+