public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <patrick@parcs.ath.cx>
To: Jason Merrill <jason@redhat.com>
Cc: Patrick Palka <patrick@parcs.ath.cx>,
	    GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Fix PR c++/70332 (ICE due to aggregate initialization of NSDMI)
Date: Wed, 23 Mar 2016 13:25:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.20.11.1603230918240.1308@idea> (raw)
In-Reply-To: <56F292FA.5010200@redhat.com>

On Wed, 23 Mar 2016, Jason Merrill wrote:

> On 03/22/2016 07:12 PM, Patrick Palka wrote:
> > On Tue, Mar 22, 2016 at 6:12 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> > > On Tue, Mar 22, 2016 at 6:00 PM, Jason Merrill <jason@redhat.com> wrote:
> > > > On 03/22/2016 05:35 PM, Patrick Palka wrote:
> > > > > 
> > > > > +             if (cp_unevaluated_operand == 0
> > > > 
> > > > 
> > > > Why check this here?
> > > 
> > > Just so that the change doesn't affect the behavior of tsubst_decl()
> > > when cp_unevaluated_operand != 0.  Presumably the existing code (10
> > > lines below) handles that case just fine.
> > 
> > Turns out that without the check we can trigger the cxx_dialect >=
> > cxx14 assert because in c++11 mode we can reach the assert through
> > get_defaulted_eh_spec() which increments cp_unevaluated_operand and
> > then calls get_nsdmi (..., /*in_ctor=*/false) causing
> > current_class_ref to get set to a PLACEHOLDER_EXPR.
> > 
> > So for example g++.dg/cpp0x/nsdmi-template2.C regresses with an ICE.
> > So it seems the cp_unevaluated_operand != 0 check is necessary as long
> > as the assert stays.
> > 
> > There are no regressions if both the cp_unevaluated_operand check and
> > the assert are removed however.
> 
> I think that's my preference.
> 
> Jason
> 
> 
> 

Done.  Does this version look OK to commit after bootstrap + regtesting?

gcc/cp/ChangeLog:

	PR c++/70332
	* pt.c (tsubst_copy) [PARM_DECL]: Handle the use of 'this' in an
	NSDMI that's part of an aggregrate initialization.

gcc/testsuite/ChangeLog:

	PR c++/70332
	* g++.dg/cpp1y/nsdmi-aggr5.C: New test.
---
 gcc/cp/pt.c                              |  9 ++++++---
 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ebfc45b..231d112 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13877,10 +13877,13 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 
       if (r == NULL_TREE)
 	{
-	  /* We get here for a use of 'this' in an NSDMI.  */
+	  /* We get here for a use of 'this' in an NSDMI as part of a
+	     constructor call or as part of an aggregate initialization.  */
 	  if (DECL_NAME (t) == this_identifier
-	      && current_function_decl
-	      && DECL_CONSTRUCTOR_P (current_function_decl))
+	      && ((current_function_decl
+		   && DECL_CONSTRUCTOR_P (current_function_decl))
+		  || (current_class_ref
+		      && TREE_CODE (current_class_ref) == PLACEHOLDER_EXPR)))
 	    return current_class_ptr;
 
 	  /* This can happen for a parameter name used later in a function
diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C
new file mode 100644
index 0000000..fe377c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C
@@ -0,0 +1,24 @@
+// PR c++/70332
+// { dg-do run { target c++14 } }
+
+template <class T>
+struct C
+{
+ T m;
+ T *n = &m;
+};
+
+C<int> c { };
+
+int
+main ()
+{
+  *c.n = 5;
+  if (c.m != 5)
+    __builtin_abort ();
+
+  C<int> d { 10 };
+  *d.n = *d.n + 1;
+  if (d.m != 11)
+    __builtin_abort ();
+}
-- 
2.8.0.rc3.27.gade0865

  reply	other threads:[~2016-03-23 13:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-22 21:40 Patrick Palka
2016-03-22 22:33 ` Jason Merrill
2016-03-22 22:45   ` Patrick Palka
2016-03-23  1:33     ` Patrick Palka
2016-03-23 13:05       ` Jason Merrill
2016-03-23 13:25         ` Patrick Palka [this message]
2016-03-23 13:52           ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.11.1603230918240.1308@idea \
    --to=patrick@parcs.ath.cx \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).