Using 6.0.0 20151208 the last two asserts fire, whereas they don't with GCC 5.2.1 (as expected). The issue seems constexpr related as not marking from_value and from_aggr constexpr makes it go away. Compiled using: g++-trunk -std=c++11 main.cpp -std=c++14 and -std=c++1z result in the same behaviour. ---------------------------------------- #include <cassert> struct holder { int& value; }; constexpr holder from_value(int& value) { return { value }; } struct aggr { int i; }; constexpr holder from_aggr(aggr& a) { return from_value(a.i); } int main() { aggr a { 42 }; // these don't fire assert( &from_value(a.i).value != nullptr ); assert( &a.i == &from_value(a.i).value ); // those do assert( &from_aggr(a).value != nullptr ); assert( &a.i == &from_aggr(a).value ); } ----------------------------------------
Started with r230365 aka merge of the delayed folding branch. struct B { int &b; }; constexpr B foo (int &x) { return { x }; } struct C { int c; }; constexpr B bar (C &y) { return foo (y.c); } int main () { C a { 42 }; if (&foo (a.c).b == nullptr) __builtin_abort (); if (&a.c != &foo (a.c).b) __builtin_abort (); if (&bar (a).b == nullptr) __builtin_abort (); if (&a.c != &bar (a).b) __builtin_abort (); } shows IMHO better what's going on, already in *.original dump the PARM_DECL y leaks into the main function (supposedly it should have been replaced by &a.
I think the problem is constexpr.c creates invalid trees, in particular CONSTRUCTORs that have non-TREE_CONSTANT elements, yet they have TREE_CONSTANT set on them (and possibly also TREE_SIDE_EFFECTS similarly). build_constructor with NULL vals creates TREE_CONSTANT and !TREE_SIDE_EFFECTS CONSTRUCTOR, and constexpr.c later modifies the elts without updating those flags. --- constexpr.c.jj4 2015-12-07 12:17:56.000000000 +0100 +++ constexpr.c 2015-12-14 19:40:24.054131253 +0100 @@ -2174,6 +2174,8 @@ cxx_eval_bare_aggregate (const constexpr { vec<constructor_elt, va_gc> *v = CONSTRUCTOR_ELTS (t); bool changed = false; + bool constant_p = true; + bool side_effects_p = false; gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (t)); verify_ctor_sanity (ctx, TREE_TYPE (t)); @@ -2222,7 +2224,13 @@ cxx_eval_bare_aggregate (const constexpr (*p)->last().value = elt; } else - CONSTRUCTOR_APPEND_ELT (*p, index, elt); + { + CONSTRUCTOR_APPEND_ELT (*p, index, elt); + if (!TREE_CONSTANT (elt)) + constant_p = false; + if (TREE_SIDE_EFFECTS (elt)) + side_effects_p = true; + } } if (*non_constant_p || !changed) return t; @@ -2230,6 +2238,10 @@ cxx_eval_bare_aggregate (const constexpr /* We're done building this CONSTRUCTOR, so now we can interpret an element without an explicit initializer as value-initialized. */ CONSTRUCTOR_NO_IMPLICIT_ZERO (t) = false; + if (!constant_p) + TREE_CONSTANT (t) = false; + if (side_effects_p) + TREE_SIDE_EFFECTS (t) = true; if (VECTOR_TYPE_P (TREE_TYPE (t))) t = fold (t); return t; @@ -2847,9 +2859,13 @@ cxx_eval_store_expression (const constex /* The hash table might have moved since the get earlier. */ valp = ctx->values->get (object); if (TREE_CODE (init) == CONSTRUCTOR) - /* An outer ctx->ctor might be pointing to *valp, so just replace - its contents. */ - CONSTRUCTOR_ELTS (*valp) = CONSTRUCTOR_ELTS (init); + { + /* An outer ctx->ctor might be pointing to *valp, so just replace + its contents. */ + CONSTRUCTOR_ELTS (*valp) = CONSTRUCTOR_ELTS (init); + TREE_CONSTANT (*valp) = TREE_CONSTANT (init); + TREE_SIDE_EFFECTS (*valp) = TREE_SIDE_EFFECTS (init); + } else *valp = init; } is untested fix that cures this testcase, but the cxx_eval_bare_aggregate changes are incomplete, there are other ways how a new elt is stored too. Or perhaps it could just in another loop go over all elements and compute those flags. Jason, any thoughts on this?
On 12/14/2015 01:45 PM, jakub at gcc dot gnu.org wrote: > I think the problem is constexpr.c creates invalid trees, in particular > CONSTRUCTORs that have non-TREE_CONSTANT elements, yet they have TREE_CONSTANT > set on them (and possibly also TREE_SIDE_EFFECTS similarly). Hmm, any element without TREE_CONSTANT should have caused us to return the original CONSTRUCTOR. > build_constructor with NULL vals creates TREE_CONSTANT and !TREE_SIDE_EFFECTS > CONSTRUCTOR, and constexpr.c later modifies the elts without updating those > flags. > > is untested fix that cures this testcase, but the cxx_eval_bare_aggregate > changes are incomplete, there are other ways how a new elt is stored too. Or > perhaps it could just in another loop go over all elements and compute those > flags. Jason, any thoughts on this? I thought there was already a function to recompute these flags, but I'm not finding it. Jason
(In reply to Jason Merrill from comment #3) > Hmm, any element without TREE_CONSTANT should have caused us to return > the original CONSTRUCTOR. Perhaps the TREE_SIDE_EFFECTS stuff is not needed, but for TREE_CONSTANT perhaps the reason is that constexpr.c has different POV on what is a constant compared to ../tree.c - at least it seems that cxx_eval_constant_expression happily accepts &y->c as *non_constant_p = false, but if CONSTRUCTOR containing that is marked TREE_CONSTANT (which is IMHO wrong, because in that case all elements should be TREE_CONSTANT), then we e.g. trigger: case CONSTRUCTOR: if (TREE_CONSTANT (t)) /* Don't re-process a constant CONSTRUCTOR, but do fold it to VECTOR_CST if applicable. */ return fold (t); r = cxx_eval_bare_aggregate (ctx, t, lval, non_constant_p, overflow_p); break; and just fold it instead of calling cxx_eval_bare_aggregate on it. > I thought there was already a function to recompute these flags, but I'm > not finding it. I can't find it either, we have that only for ADDR_EXPR it seems - recompute_tree_invariant_for_addr_expr.
Note, I gave up on this PR, so if Jason or somebody more familiar with C++ FE than myself could pick it up, it would be greatly appreciated.
*** Bug 69327 has been marked as a duplicate of this bug. ***
Author: jason Date: Tue Jan 26 21:34:10 2016 New Revision: 232847 URL: https://gcc.gnu.org/viewcvs?rev=232847&root=gcc&view=rev Log: PR c++/68782 gcc/ * tree.c (recompute_constructor_flags): Split out from build_constructor. (verify_constructor_flags): New. * tree.h: Declare them. gcc/cp/ * constexpr.c (cxx_eval_bare_aggregate): Update TREE_CONSTANT and TREE_SIDE_EFFECTS. (cxx_eval_constant_expression) [CONSTRUCTOR]: Call verify_constructor_flags. Added: trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-aggr2.C Modified: trunk/gcc/ChangeLog trunk/gcc/cp/ChangeLog trunk/gcc/cp/constexpr.c trunk/gcc/tree.c trunk/gcc/tree.h
Fixed.