From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85333 invoked by alias); 23 Feb 2018 14:30:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 85277 invoked by uid 89); 23 Feb 2018 14:30:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Feb 2018 14:30:00 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1FB2641C9 for ; Fri, 23 Feb 2018 14:29:59 +0000 (UTC) Received: from redhat.com (ovpn-204-64.brq.redhat.com [10.40.204.64]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C96075D75F; Fri, 23 Feb 2018 14:29:58 +0000 (UTC) Date: Fri, 23 Feb 2018 14:30:00 -0000 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: Re: C++ PATCH to fix rejects-valid with constexpr ctor in C++17 (PR c++/83692) Message-ID: <20180223142955.GB2995@redhat.com> References: <20180125211639.GA2620@redhat.com> <20180205133752.GF2608@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) X-SW-Source: 2018-02/txt/msg01324.txt.bz2 On Fri, Feb 16, 2018 at 04:10:20PM -0500, Jason Merrill wrote: > On Mon, Feb 5, 2018 at 1:45 PM, Jason Merrill wrote: > > On Mon, Feb 5, 2018 at 8:37 AM, Marek Polacek wrote: > >> On Fri, Feb 02, 2018 at 02:11:27PM -0500, Jason Merrill wrote: > >>> On Thu, Jan 25, 2018 at 4:16 PM, Marek Polacek wrote: > >>> > This is a similar problem to 83116: we'd cached a constexpr call, but after a > >>> > store the result had become invalid, yet we used the wrong result again when > >>> > encountering the same call later. This resulted in evaluating a THROW_EXPR > >>> > which doesn't work. Details in > >>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83692#c5 > >>> > > >>> > The fix for 83116 didn't work here, because when evaluating the body of the > >>> > ctor via store_init_value -> cxx_constant_value we are in STRICT, so we do > >>> > cache. > >>> > >>> > It seems that we may no longer rely on the constexpr call table when we > >>> > do cxx_eval_store_expression, because that just rewrites *valp, i.e. the > >>> > value of an object. Might be too big a hammer again, but I couldn't think > >>> > of how I could guard the caching of a constexpr call. > >>> > >>> > This doesn't manifest in C++14 because build_special_member_call in C++17 is > >>> > more aggressive with copy elisions (as required by P0135 which changed how we > >>> > view prvalues). In C++14 build_special_member_call produces a CALL_EXPR, so > >>> > expand_default_init calls maybe_constant_init, for which STRICT is false, so > >>> > we avoid caching as per 83116. > >>> > >>> So it sounds like the problem is using cxx_constant_value for the > >>> diagnostic when it has different semantics from the > >>> maybe_constant_init that follows right after. I guess we want a > >>> cxx_constant_init function that is a hybrid of the two. > >> > >> So like the following? Thanks, > >> > >> Bootstrapped/regtested on x86_64-linux, ok for trunk? > >> > >> 2018-02-04 Marek Polacek > >> > >> PR c++/83692 > >> * constexpr.c (cxx_constant_init): New function. > >> * cp-tree.h (cxx_constant_init): Declare. > >> * typeck2.c (store_init_value): Call cxx_constant_init instead of > >> cxx_constant_value. > >> > >> +/* Like cxx_constant_value, but non-strict mode. */ > >> + > >> +tree > >> +cxx_constant_init (tree t, tree decl) > >> +{ > >> + return cxx_eval_outermost_constant_expr (t, false, false, decl); > >> +} > > > > Hmm, that doesn't do the TARGET_EXPR stripping that > > maybe_constant_init does. I was thinking of a version of > > maybe_constant_init that passes false to allow_non_constant. Probably > > by making "maybe_constant_init" and cxx_constant_init both call the > > current function with an additional parameter. And then the existing > > call to maybe_constant_init can move under an 'else' to avoid > > redundant constexpr evaluation. > > Want me to take this over? Sorry again for the delay. I tried to do what you suggested. There was one twist: it regressed constexpr-nullptr-2.C, in particular we lost diagnostics for constexpr int* pj0 = &((S*)0)->j; // { dg-error "not a constant expression" } constexpr int* pj1 = &((S*)nullptr)->j; // { dg-error "not a constant expression" } because when maybe_constant_init_1 saw a constant: 5142 else if (CONSTANT_CLASS_P (t)) 5143 /* No evaluation needed. */; so it didn't call cxx_eval_outermost_constant_expr which is supposed to give the error. I fixed it by adding "&& allow_non_constant" so now it gives the proper diagnostics. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-02-23 Marek Polacek PR c++/83692 * constexpr.c (maybe_constant_init_1): New function. (maybe_constant_init): Make it a wrapper around maybe_constant_init_1. (cxx_constant_init): New function. * cp-tree.h (cxx_constant_init): Declare. * typeck2.c (store_init_value): Call cxx_constant_init instead of cxx_constant_value. Move the maybe_constant_init call under an 'else'. * g++.dg/cpp1z/constexpr-83692.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 47ff90cb055..26d0d099a05 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -5123,8 +5123,8 @@ fold_non_dependent_expr (tree t) /* Like maybe_constant_value, but returns a CONSTRUCTOR directly, rather than wrapped in a TARGET_EXPR. */ -tree -maybe_constant_init (tree t, tree decl) +static tree +maybe_constant_init_1 (tree t, tree decl, bool allow_non_constant) { if (!t) return t; @@ -5139,10 +5139,10 @@ maybe_constant_init (tree t, tree decl) t = TARGET_EXPR_INITIAL (t); if (!is_nondependent_static_init_expression (t)) /* Don't try to evaluate it. */; - else if (CONSTANT_CLASS_P (t)) + else if (CONSTANT_CLASS_P (t) && allow_non_constant) /* No evaluation needed. */; else - t = cxx_eval_outermost_constant_expr (t, true, false, decl); + t = cxx_eval_outermost_constant_expr (t, allow_non_constant, false, decl); if (TREE_CODE (t) == TARGET_EXPR) { tree init = TARGET_EXPR_INITIAL (t); @@ -5152,6 +5152,22 @@ maybe_constant_init (tree t, tree decl) return t; } +/* Wrapper for maybe_constant_init_1 which permits non constants. */ + +tree +maybe_constant_init (tree t, tree decl) +{ + return maybe_constant_init_1 (t, decl, true); +} + +/* Wrapper for maybe_constant_init_1 which does not permit non constants. */ + +tree +cxx_constant_init (tree t, tree decl) +{ + return maybe_constant_init_1 (t, decl, false); +} + #if 0 /* FIXME see ADDR_EXPR section in potential_constant_expression_1. */ /* Return true if the object referred to by REF has automatic or thread diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h index 9038d677b2d..04c7b7ce3a9 100644 --- gcc/cp/cp-tree.h +++ gcc/cp/cp-tree.h @@ -7411,6 +7411,7 @@ extern bool require_potential_constant_expression (tree); extern bool require_constant_expression (tree); 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_init (tree, tree = NULL_TREE); extern tree fold_non_dependent_expr (tree); diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index 899d60e8535..153b46cca77 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -830,9 +830,10 @@ store_init_value (tree decl, tree init, vec** cleanups, int flags) if (!require_constant_expression (value)) value = error_mark_node; else - value = cxx_constant_value (value, decl); + value = cxx_constant_init (value, decl); } - value = maybe_constant_init (value, decl); + else + value = maybe_constant_init (value, decl); if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type)) /* Poison this CONSTRUCTOR so it can't be copied to another constexpr variable. */ diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C index e69de29bb2d..f6b61eeab85 100644 --- gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C +++ gcc/testsuite/g++.dg/cpp1z/constexpr-83692.C @@ -0,0 +1,21 @@ +// PR c++/83692 +// { dg-options -std=c++17 } + +struct integer { + constexpr int value() const { return m_value; } + int m_value; +}; + +struct outer { + integer m_x{0}; + constexpr outer() + { + if (m_x.value() != 0) + throw 0; + m_x.m_value = integer{1}.value(); + if (m_x.value() != 1) + throw 0; + } +}; + +constexpr outer o{}; Marek