* [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934)
@ 2018-12-18 20:45 Jakub Jelinek
2018-12-18 22:40 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-12-18 20:45 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
The following testcase FAILs, because parsing creates a TREE_CONSTANT
CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that,
but constexpr evaluation doesn't touch those CONSTRUCTORs.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-12-18 Jakub Jelinek <jakub@redhat.com>
PR c++/87934
* constexpr.c (cxx_eval_constant_expression) <case CONSTRUCTOR>: Do
re-process TREE_CONSTANT CONSTRUCTORs if they aren't reduced constant
expressions.
* g++.dg/cpp0x/constexpr-87934.C: New test.
--- gcc/cp/constexpr.c.jj 2018-12-12 23:43:57.263128844 +0100
+++ gcc/cp/constexpr.c 2018-12-18 14:43:33.460553853 +0100
@@ -4681,7 +4681,7 @@ cxx_eval_constant_expression (const cons
break;
case CONSTRUCTOR:
- if (TREE_CONSTANT (t))
+ if (TREE_CONSTANT (t) && reduced_constant_expression_p (t))
{
/* Don't re-process a constant CONSTRUCTOR, but do fold it to
VECTOR_CST if applicable. */
--- gcc/testsuite/g++.dg/cpp0x/constexpr-87934.C.jj 2018-12-18 15:05:56.318886878 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-87934.C 2018-12-18 15:02:10.652524999 +0100
@@ -0,0 +1,9 @@
+// PR c++/87934
+// { dg-do compile { target c++11 } }
+
+struct Foo
+{
+ enum { BAR } bar = BAR;
+};
+
+constexpr Foo foo{};
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) 2018-12-18 20:45 [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) Jakub Jelinek @ 2018-12-18 22:40 ` Jason Merrill 2018-12-18 23:19 ` Jakub Jelinek 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2018-12-18 22:40 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches On 12/18/18 3:45 PM, Jakub Jelinek wrote: > The following testcase FAILs, because parsing creates a TREE_CONSTANT > CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that, > but constexpr evaluation doesn't touch those CONSTRUCTORs. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? OK. I also wonder if store_init_value should use cp_fold_r rather than just cp_fully_fold. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) 2018-12-18 22:40 ` Jason Merrill @ 2018-12-18 23:19 ` Jakub Jelinek 2018-12-19 3:28 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Jakub Jelinek @ 2018-12-18 23:19 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches On Tue, Dec 18, 2018 at 05:40:03PM -0500, Jason Merrill wrote: > On 12/18/18 3:45 PM, Jakub Jelinek wrote: > > The following testcase FAILs, because parsing creates a TREE_CONSTANT > > CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that, > > but constexpr evaluation doesn't touch those CONSTRUCTORs. > > > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > > trunk? > > OK. I also wonder if store_init_value should use cp_fold_r rather than just > cp_fully_fold. I've been thinking about that already when working on the PR88410 bug. Do you mean something like following completely untested patch? Perhaps I could add a helper inline so that there is no code repetition between cp_fully_fold and this new function. Note, it doesn't fix this PR, as store_init_value is called only after we emit the error, so the constexpr.c patch is needed too. --- gcc/cp/cp-tree.h.jj 2018-12-12 23:43:57.211129676 +0100 +++ gcc/cp/cp-tree.h 2018-12-19 00:12:59.795154220 +0100 @@ -7542,6 +7542,7 @@ extern bool cxx_omp_privatize_by_referen extern bool cxx_omp_disregard_value_expr (tree, bool); extern void cp_fold_function (tree); extern tree cp_fully_fold (tree); +extern tree cp_fully_fold_init (tree); extern void clear_fold_cache (void); extern tree lookup_hotness_attribute (tree); extern tree process_stmt_hotness_attribute (tree); --- gcc/cp/typeck2.c.jj 2018-12-01 00:25:09.340988953 +0100 +++ gcc/cp/typeck2.c 2018-12-19 00:14:19.306875071 +0100 @@ -750,7 +750,7 @@ split_nonconstant_init (tree dest, tree init = TARGET_EXPR_INITIAL (init); if (TREE_CODE (init) == CONSTRUCTOR) { - init = cp_fully_fold (init); + init = cp_fully_fold_init (init); code = push_stmt_list (); if (split_nonconstant_init_1 (dest, init)) init = NULL_TREE; @@ -858,7 +858,7 @@ store_init_value (tree decl, tree init, if (!const_init) value = oldval; } - value = cp_fully_fold (value); + value = cp_fully_fold_init (value); /* Handle aggregate NSDMI in non-constant initializers, too. */ value = replace_placeholders (value, decl); --- gcc/cp/cp-gimplify.c.jj 2018-12-17 22:54:02.736416699 +0100 +++ gcc/cp/cp-gimplify.c 2018-12-19 00:12:05.862021875 +0100 @@ -2171,6 +2171,32 @@ cp_fully_fold (tree x) return cp_fold_rvalue (x); } +/* Likewise, but also fold recursively. */ + +tree +cp_fully_fold_init (tree x) +{ + if (processing_template_decl) + return x; + /* FIXME cp_fold ought to be a superset of maybe_constant_value so we don't + have to call both. */ + if (cxx_dialect >= cxx11) + { + x = maybe_constant_value (x); + /* Sometimes we are given a CONSTRUCTOR but the call above wraps it into + a TARGET_EXPR; undo that here. */ + if (TREE_CODE (x) == TARGET_EXPR) + x = TARGET_EXPR_INITIAL (x); + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR + && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR + && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x)) + x = TREE_OPERAND (x, 0); + } + hash_set<tree> pset; + cp_walk_tree (&x, cp_fold_r, &pset, NULL); + return cp_fold_rvalue (x); +} + /* c-common interface to cp_fold. If IN_INIT, this is in a static initializer and certain changes are made to the folding done. Or should be (FIXME). We never touch maybe_const, as it is only used for the C front-end Jakub ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) 2018-12-18 23:19 ` Jakub Jelinek @ 2018-12-19 3:28 ` Jason Merrill 2018-12-19 23:14 ` Jakub Jelinek 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2018-12-19 3:28 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches On 12/18/18 6:19 PM, Jakub Jelinek wrote: > On Tue, Dec 18, 2018 at 05:40:03PM -0500, Jason Merrill wrote: >> On 12/18/18 3:45 PM, Jakub Jelinek wrote: >>> The following testcase FAILs, because parsing creates a TREE_CONSTANT >>> CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that, >>> but constexpr evaluation doesn't touch those CONSTRUCTORs. >>> >>> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for >>> trunk? >> >> OK. I also wonder if store_init_value should use cp_fold_r rather than just >> cp_fully_fold. > > I've been thinking about that already when working on the PR88410 bug. > > Do you mean something like following completely untested patch? > Perhaps I could add a helper inline so that there is no code repetition > between cp_fully_fold and this new function. Something like that, yes. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) 2018-12-19 3:28 ` Jason Merrill @ 2018-12-19 23:14 ` Jakub Jelinek 2018-12-20 16:01 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Jakub Jelinek @ 2018-12-19 23:14 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches On Tue, Dec 18, 2018 at 10:27:56PM -0500, Jason Merrill wrote: > On 12/18/18 6:19 PM, Jakub Jelinek wrote: > > On Tue, Dec 18, 2018 at 05:40:03PM -0500, Jason Merrill wrote: > > > On 12/18/18 3:45 PM, Jakub Jelinek wrote: > > > > The following testcase FAILs, because parsing creates a TREE_CONSTANT > > > > CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that, > > > > but constexpr evaluation doesn't touch those CONSTRUCTORs. > > > > > > > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > > > > trunk? > > > > > > OK. I also wonder if store_init_value should use cp_fold_r rather than just > > > cp_fully_fold. > > > > I've been thinking about that already when working on the PR88410 bug. > > > > Do you mean something like following completely untested patch? > > Perhaps I could add a helper inline so that there is no code repetition > > between cp_fully_fold and this new function. > > Something like that, yes. The following does the job too (even the PR88410 ICE is gone with the cp-gimplify.c change from that patch reverted) and is shorter. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-12-19 Jakub Jelinek <jakub@redhat.com> * cp-tree.h (cp_fully_fold_init): Declare. * cp-gimplify.c (cp_fully_fold_init): New function. * typeck2.c (split_nonconstant_init, store_init_value): Use it instead of cp_fully_fold. --- gcc/cp/cp-tree.h.jj 2018-12-19 09:09:28.251543416 +0100 +++ gcc/cp/cp-tree.h 2018-12-19 14:57:54.719812330 +0100 @@ -7542,6 +7542,7 @@ extern bool cxx_omp_privatize_by_referen extern bool cxx_omp_disregard_value_expr (tree, bool); extern void cp_fold_function (tree); extern tree cp_fully_fold (tree); +extern tree cp_fully_fold_init (tree); extern void clear_fold_cache (void); extern tree lookup_hotness_attribute (tree); extern tree process_stmt_hotness_attribute (tree); --- gcc/cp/cp-gimplify.c.jj 2018-12-19 09:09:28.335542037 +0100 +++ gcc/cp/cp-gimplify.c 2018-12-19 15:00:28.214293053 +0100 @@ -2171,6 +2171,20 @@ cp_fully_fold (tree x) return cp_fold_rvalue (x); } +/* Likewise, but also fold recursively, which cp_fully_fold doesn't perform + in some cases. */ + +tree +cp_fully_fold_init (tree x) +{ + if (processing_template_decl) + return x; + x = cp_fully_fold (x); + hash_set<tree> pset; + cp_walk_tree (&x, cp_fold_r, &pset, NULL); + return x; +} + /* c-common interface to cp_fold. If IN_INIT, this is in a static initializer and certain changes are made to the folding done. Or should be (FIXME). We never touch maybe_const, as it is only used for the C front-end --- gcc/cp/typeck2.c.jj 2018-12-19 09:09:28.401540956 +0100 +++ gcc/cp/typeck2.c 2018-12-19 14:57:54.736812061 +0100 @@ -750,7 +750,7 @@ split_nonconstant_init (tree dest, tree init = TARGET_EXPR_INITIAL (init); if (TREE_CODE (init) == CONSTRUCTOR) { - init = cp_fully_fold (init); + init = cp_fully_fold_init (init); code = push_stmt_list (); if (split_nonconstant_init_1 (dest, init)) init = NULL_TREE; @@ -858,7 +858,7 @@ store_init_value (tree decl, tree init, if (!const_init) value = oldval; } - value = cp_fully_fold (value); + value = cp_fully_fold_init (value); /* Handle aggregate NSDMI in non-constant initializers, too. */ value = replace_placeholders (value, decl); Jakub ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) 2018-12-19 23:14 ` Jakub Jelinek @ 2018-12-20 16:01 ` Jason Merrill 0 siblings, 0 replies; 6+ messages in thread From: Jason Merrill @ 2018-12-20 16:01 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches On 12/19/18 6:14 PM, Jakub Jelinek wrote: > On Tue, Dec 18, 2018 at 10:27:56PM -0500, Jason Merrill wrote: >> On 12/18/18 6:19 PM, Jakub Jelinek wrote: >>> On Tue, Dec 18, 2018 at 05:40:03PM -0500, Jason Merrill wrote: >>>> On 12/18/18 3:45 PM, Jakub Jelinek wrote: >>>>> The following testcase FAILs, because parsing creates a TREE_CONSTANT >>>>> CONSTRUCTOR that contains CONST_DECL elts. cp_fold_r can handle that, >>>>> but constexpr evaluation doesn't touch those CONSTRUCTORs. >>>>> >>>>> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for >>>>> trunk? >>>> >>>> OK. I also wonder if store_init_value should use cp_fold_r rather than just >>>> cp_fully_fold. >>> >>> I've been thinking about that already when working on the PR88410 bug. >>> >>> Do you mean something like following completely untested patch? >>> Perhaps I could add a helper inline so that there is no code repetition >>> between cp_fully_fold and this new function. >> >> Something like that, yes. > > The following does the job too (even the PR88410 ICE is gone with the > cp-gimplify.c change from that patch reverted) and is shorter. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK, thanks. Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-20 16:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-18 20:45 [C++ PATCH] Constexpr fold even some TREE_CONSTANT ctors (PR c++/87934) Jakub Jelinek 2018-12-18 22:40 ` Jason Merrill 2018-12-18 23:19 ` Jakub Jelinek 2018-12-19 3:28 ` Jason Merrill 2018-12-19 23:14 ` Jakub Jelinek 2018-12-20 16:01 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).