| Message ID | 20181211163925.GY12380@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix up __builtin_is_constant_evaluated handling in array type sizes (PR c++/88446) | expand |
On Tue, Dec 11, 2018 at 05:39:25PM +0100, Jakub Jelinek wrote: > Hi! > > As mentioned in the PR, while we allow VLAs in some contexts in C++ as > an extension, they aren't standard and the standard requires in those spots > constant expressions, thus __builtin_is_constant_evaluated () needs to be > true if those sizes are indeed constant expressions. > > Fixed by calling cxx_eval_outermost_constant_expr with > pretend_const_required too in that case. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-12-11 Jakub Jelinek <jakub@redhat.com> > > PR c++/88446 > * cp-tree.h (maybe_constant_value): Add pretend_const_required > argument. > * constexpr.c (maybe_constant_value): Likewise. If true, don't > cache and call cxx_eval_outermost_constant_expr with true as > pretend_const_required. > * decl.c (compute_array_index_type_loc): Call maybe_constant_value > with true as pretend_const_required. > > * g++.dg/cpp2a/is-constant-evaluated3.C: New test. > > --- gcc/cp/cp-tree.h.jj 2018-12-07 00:23:15.024998595 +0100 > +++ gcc/cp/cp-tree.h 2018-12-11 13:55:51.933845503 +0100 > @@ -7663,7 +7663,7 @@ extern bool require_rvalue_constant_expr > extern bool require_potential_rvalue_constant_expression (tree); > extern tree cxx_constant_value (tree, tree = NULL_TREE); > extern tree cxx_constant_init (tree, tree = NULL_TREE); > -extern tree maybe_constant_value (tree, tree = NULL_TREE); > +extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false); > extern tree maybe_constant_init (tree, tree = NULL_TREE, bool = false); > extern tree fold_non_dependent_expr (tree, tsubst_flags_t = tf_warning_or_error); > extern tree fold_simple (tree); > --- gcc/cp/constexpr.c.jj 2018-12-11 12:01:27.968941683 +0100 > +++ gcc/cp/constexpr.c 2018-12-11 13:56:50.382890876 +0100 > @@ -5244,7 +5244,7 @@ fold_simple (tree t) > static GTY((deletable)) hash_map<tree, tree> *cv_cache; > > tree > -maybe_constant_value (tree t, tree decl) > +maybe_constant_value (tree t, tree decl, bool pretend_const_required) > { > tree r; > Could you please describe the new param in the comment? > --- gcc/cp/decl.c.jj 2018-12-07 00:23:15.000000000 +0100 > +++ gcc/cp/decl.c 2018-12-11 13:57:30.779231098 +0100 > @@ -9645,7 +9645,10 @@ compute_array_index_type_loc (location_t > { > size = instantiate_non_dependent_expr_sfinae (size, complain); > size = build_converted_constant_expr (size_type_node, size, complain); > - size = maybe_constant_value (size); > + /* Pedantically a constant expression is required here and so > + __builtin_is_constant_evaluated () should fold to true if it > + is successfully folded into a constant. */ > + size = maybe_constant_value (size, NULL_TREE, true); Perhaps use /*pretend_const_required=*/true? Marek
On 12/11/18 3:35 PM, Marek Polacek wrote: > On Tue, Dec 11, 2018 at 05:39:25PM +0100, Jakub Jelinek wrote: >> Hi! >> >> As mentioned in the PR, while we allow VLAs in some contexts in C++ as >> an extension, they aren't standard and the standard requires in those spots >> constant expressions, thus __builtin_is_constant_evaluated () needs to be >> true if those sizes are indeed constant expressions. >> >> Fixed by calling cxx_eval_outermost_constant_expr with >> pretend_const_required too in that case. >> >> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> 2018-12-11 Jakub Jelinek <jakub@redhat.com> >> >> PR c++/88446 >> * cp-tree.h (maybe_constant_value): Add pretend_const_required >> argument. >> * constexpr.c (maybe_constant_value): Likewise. If true, don't >> cache and call cxx_eval_outermost_constant_expr with true as >> pretend_const_required. >> * decl.c (compute_array_index_type_loc): Call maybe_constant_value >> with true as pretend_const_required. >> >> * g++.dg/cpp2a/is-constant-evaluated3.C: New test. >> >> --- gcc/cp/cp-tree.h.jj 2018-12-07 00:23:15.024998595 +0100 >> +++ gcc/cp/cp-tree.h 2018-12-11 13:55:51.933845503 +0100 >> @@ -7663,7 +7663,7 @@ extern bool require_rvalue_constant_expr >> extern bool require_potential_rvalue_constant_expression (tree); >> extern tree cxx_constant_value (tree, tree = NULL_TREE); >> extern tree cxx_constant_init (tree, tree = NULL_TREE); >> -extern tree maybe_constant_value (tree, tree = NULL_TREE); >> +extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false); >> extern tree maybe_constant_init (tree, tree = NULL_TREE, bool = false); >> extern tree fold_non_dependent_expr (tree, tsubst_flags_t = tf_warning_or_error); >> extern tree fold_simple (tree); >> --- gcc/cp/constexpr.c.jj 2018-12-11 12:01:27.968941683 +0100 >> +++ gcc/cp/constexpr.c 2018-12-11 13:56:50.382890876 +0100 >> @@ -5244,7 +5244,7 @@ fold_simple (tree t) >> static GTY((deletable)) hash_map<tree, tree> *cv_cache; >> >> tree >> -maybe_constant_value (tree t, tree decl) >> +maybe_constant_value (tree t, tree decl, bool pretend_const_required) >> { >> tree r; >> > > Could you please describe the new param in the comment? > >> --- gcc/cp/decl.c.jj 2018-12-07 00:23:15.000000000 +0100 >> +++ gcc/cp/decl.c 2018-12-11 13:57:30.779231098 +0100 >> @@ -9645,7 +9645,10 @@ compute_array_index_type_loc (location_t >> { >> size = instantiate_non_dependent_expr_sfinae (size, complain); >> size = build_converted_constant_expr (size_type_node, size, complain); >> - size = maybe_constant_value (size); >> + /* Pedantically a constant expression is required here and so >> + __builtin_is_constant_evaluated () should fold to true if it >> + is successfully folded into a constant. */ >> + size = maybe_constant_value (size, NULL_TREE, true); > > Perhaps use /*pretend_const_required=*/true? Please. And the name of the parameter should change to match the other patches. OK with those changes. Jason
On Thu, Dec 13, 2018 at 03:05:10PM -0500, Jason Merrill wrote: > > Perhaps use /*pretend_const_required=*/true? > > Please. And the name of the parameter should change to match the other > patches. OK with those changes. You've already acked a newer version of that patch that has that in and current trunk uses /*manifestly_const_required=*/true. Jakub
--- gcc/cp/cp-tree.h.jj 2018-12-07 00:23:15.024998595 +0100 +++ gcc/cp/cp-tree.h 2018-12-11 13:55:51.933845503 +0100 @@ -7663,7 +7663,7 @@ extern bool require_rvalue_constant_expr extern bool require_potential_rvalue_constant_expression (tree); extern tree cxx_constant_value (tree, tree = NULL_TREE); extern tree cxx_constant_init (tree, tree = NULL_TREE); -extern tree maybe_constant_value (tree, tree = NULL_TREE); +extern tree maybe_constant_value (tree, tree = NULL_TREE, bool = false); extern tree maybe_constant_init (tree, tree = NULL_TREE, bool = false); extern tree fold_non_dependent_expr (tree, tsubst_flags_t = tf_warning_or_error); extern tree fold_simple (tree); --- gcc/cp/constexpr.c.jj 2018-12-11 12:01:27.968941683 +0100 +++ gcc/cp/constexpr.c 2018-12-11 13:56:50.382890876 +0100 @@ -5244,7 +5244,7 @@ fold_simple (tree t) static GTY((deletable)) hash_map<tree, tree> *cv_cache; tree -maybe_constant_value (tree t, tree decl) +maybe_constant_value (tree t, tree decl, bool pretend_const_required) { tree r; @@ -5261,6 +5261,9 @@ maybe_constant_value (tree t, tree decl) /* No caching or evaluation needed. */ return t; + if (pretend_const_required) + return cxx_eval_outermost_constant_expr (t, true, true, true, decl); + if (cv_cache == NULL) cv_cache = hash_map<tree, tree>::create_ggc (101); if (tree *cached = cv_cache->get (t)) --- gcc/cp/decl.c.jj 2018-12-07 00:23:15.000000000 +0100 +++ gcc/cp/decl.c 2018-12-11 13:57:30.779231098 +0100 @@ -9645,7 +9645,10 @@ compute_array_index_type_loc (location_t { size = instantiate_non_dependent_expr_sfinae (size, complain); size = build_converted_constant_expr (size_type_node, size, complain); - size = maybe_constant_value (size); + /* Pedantically a constant expression is required here and so + __builtin_is_constant_evaluated () should fold to true if it + is successfully folded into a constant. */ + size = maybe_constant_value (size, NULL_TREE, true); if (!TREE_CONSTANT (size)) size = osize; --- gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated3.C.jj 2018-12-11 14:11:34.235458615 +0100 +++ gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated3.C 2018-12-11 14:09:55.319073715 +0100 @@ -0,0 +1,26 @@ +// P0595R1 +// { dg-do run { target c++14 } } + +struct false_type { static constexpr bool value = false; }; +struct true_type { static constexpr bool value = true; }; +template<class T, class U> +struct is_same : false_type {}; +template<class T> +struct is_same<T, T> : true_type {}; + +int a[__builtin_is_constant_evaluated () ? 1 : 2]; +int b[1]; +static_assert (is_same<decltype (a), decltype (b)>::value, ""); + +int +main () +{ + int c[__builtin_is_constant_evaluated () ? 3 : 4]; + int d[3]; + static_assert (is_same<decltype (c), decltype (d)>::value, ""); + int (*e)[7][9] = new int[__builtin_is_constant_evaluated () ? -1 : 5] + [__builtin_is_constant_evaluated () ? 7 : 8] + [__builtin_is_constant_evaluated () ? 9 : 10]; + e[0][0][0] = 6; + delete[] e; +}