C++ PATCH for c++/91644 - ICE with constinit in function template

Message ID 20190903210834.GT14737@redhat.com
State New
Headers show
Series
  • C++ PATCH for c++/91644 - ICE with constinit in function template
Related show

Commit Message

Marek Polacek Sept. 3, 2019, 9:08 p.m.
First constinit bug report (yay?).  The problem is that while I made sure
that constinit variable templates work as they should, I clearly neglected
ordinary static variables declared constinit in function templates.

This was an ICE in cp_finish_decl when setting TINFO_VAR_DECLARED_CONSTINIT
because 'v' here

template <typename T>
static void f ()
{
  static constinit int v = 42;
}

didn't have DECL_TEMPLATE_INFO allocated.  Fixed by using retrofit_lang_decl
so that push_template_decl then saves the DECL_TEMPLATE_INFO it creates:
 5821   info = build_template_info (tmpl, args);
 ...
 5829       if (DECL_LANG_SPECIFIC (decl))
 5830         DECL_TEMPLATE_INFO (decl) = info;

And then we need to make sure that tsubst_expr actually uses the T_V_D_C flag.

As an aside, I'm concerned that if we're required to handle
// Foo.h
struct Foo {
  constinit static int x;
};
// Foo.cpp
int Foo::x = 42;

we might need to resurrect DECL_DECLARED_CONSTINIT_P...

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-09-03  Marek Polacek  <polacek@redhat.com>

	PR c++/91644 - ICE with constinit in function template.
	* decl.c (start_decl): Call retrofit_lang_decl for constinit variables.
	* pt.c (tsubst_expr): Pass LOOKUP_CONSTINIT down to cp_finish_decl for
	constinit variables.

	* g++.dg/cpp2a/constinit13.C: New test.

Comments

Jason Merrill Sept. 4, 2019, 8 p.m. | #1
On 9/3/19 4:08 PM, Marek Polacek wrote:
> First constinit bug report (yay?).  The problem is that while I made sure

> that constinit variable templates work as they should, I clearly neglected

> ordinary static variables declared constinit in function templates.


Hmm, right, locals don't get DECL_TEMPLATE_INFO.

> As an aside, I'm concerned that if we're required to handle

> // Foo.h

> struct Foo {

>    constinit static int x;

> };

> // Foo.cpp

> int Foo::x = 42;

> 

> we might need to resurrect DECL_DECLARED_CONSTINIT_P...


Or use an (internal) attribute?

Jason
Marek Polacek Sept. 4, 2019, 8:06 p.m. | #2
On Wed, Sep 04, 2019 at 04:00:24PM -0400, Jason Merrill wrote:
> On 9/3/19 4:08 PM, Marek Polacek wrote:

> > First constinit bug report (yay?).  The problem is that while I made sure

> > that constinit variable templates work as they should, I clearly neglected

> > ordinary static variables declared constinit in function templates.

> 

> Hmm, right, locals don't get DECL_TEMPLATE_INFO.


And if I create a new, clear one, it crashes.  But I was able to work around
that by remembering if we saw constinit.

> > As an aside, I'm concerned that if we're required to handle

> > // Foo.h

> > struct Foo {

> >    constinit static int x;

> > };

> > // Foo.cpp

> > int Foo::x = 42;

> > 

> > we might need to resurrect DECL_DECLARED_CONSTINIT_P...

> 

> Or use an (internal) attribute?


Luckily it would appear that we won't be required to handle that case.

Any comments on the patch itself?

Marek
Jason Merrill Sept. 5, 2019, 3:13 p.m. | #3
On 9/4/19 3:06 PM, Marek Polacek wrote:
> On Wed, Sep 04, 2019 at 04:00:24PM -0400, Jason Merrill wrote:

>> On 9/3/19 4:08 PM, Marek Polacek wrote:

>>> First constinit bug report (yay?).  The problem is that while I made sure

>>> that constinit variable templates work as they should, I clearly neglected

>>> ordinary static variables declared constinit in function templates.

>>

>> Hmm, right, locals don't get DECL_TEMPLATE_INFO.

> 

> And if I create a new, clear one, it crashes.  But I was able to work around

> that by remembering if we saw constinit.

> 

>>> As an aside, I'm concerned that if we're required to handle

>>> // Foo.h

>>> struct Foo {

>>>     constinit static int x;

>>> };

>>> // Foo.cpp

>>> int Foo::x = 42;

>>>

>>> we might need to resurrect DECL_DECLARED_CONSTINIT_P...

>>

>> Or use an (internal) attribute?

> 

> Luckily it would appear that we won't be required to handle that case.

> 

> Any comments on the patch itself?


The patch is OK.

Jason

Patch

diff --git gcc/cp/decl.c gcc/cp/decl.c
index 6de95cdfe79..825e1e61b5e 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -5308,7 +5308,14 @@  start_decl (const cp_declarator *declarator,
     decl = maybe_push_decl (decl);
 
   if (processing_template_decl)
-    decl = push_template_decl (decl);
+    {
+      /* Make sure that for a `constinit' decl push_template_decl creates
+	 a DECL_TEMPLATE_INFO info for us, so that cp_finish_decl can then set
+	 TINFO_VAR_DECLARED_CONSTINIT.  */
+      if (decl_spec_seq_has_spec_p (declspecs, ds_constinit))
+	retrofit_lang_decl (decl);
+      decl = push_template_decl (decl);
+    }
   if (decl == error_mark_node)
     return error_mark_node;
 
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 187f9d857bc..3e6a2000780 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -17108,6 +17108,13 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	else
 	  {
 	    init = DECL_INITIAL (decl);
+	    /* The following tsubst call will clear the DECL_TEMPLATE_INFO
+	       for local variables, so save if DECL was declared constinit.  */
+	    const bool constinit_p
+	      = (VAR_P (decl)
+		 && DECL_LANG_SPECIFIC (decl)
+		 && DECL_TEMPLATE_INFO (decl)
+		 && TINFO_VAR_DECLARED_CONSTINIT (DECL_TEMPLATE_INFO (decl)));
 	    decl = tsubst (decl, args, complain, in_decl);
 	    if (decl != error_mark_node)
 	      {
@@ -17146,7 +17153,7 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		  }
 		else
 		  {
-		    int const_init = false;
+		    bool const_init = false;
 		    unsigned int cnt = 0;
 		    tree first = NULL_TREE, ndecl = error_mark_node;
 		    maybe_push_decl (decl);
@@ -17167,7 +17174,8 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 		    if (ndecl != error_mark_node)
 		      cp_maybe_mangle_decomp (ndecl, first, cnt);
 
-		    cp_finish_decl (decl, init, const_init, NULL_TREE, 0);
+		    cp_finish_decl (decl, init, const_init, NULL_TREE,
+				    constinit_p ? LOOKUP_CONSTINIT : 0);
 
 		    if (ndecl != error_mark_node)
 		      cp_finish_decomp (ndecl, first, cnt);
diff --git gcc/testsuite/g++.dg/cpp2a/constinit13.C gcc/testsuite/g++.dg/cpp2a/constinit13.C
new file mode 100644
index 00000000000..8ea64cc0e1e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/constinit13.C
@@ -0,0 +1,33 @@ 
+// PR c++/91644 - ICE with constinit in function template.
+// { dg-do compile { target c++11 } }
+
+template <typename T>
+static void fn1 ()
+{
+  static __constinit auto v1 = 0;
+  static __constinit int v2 = 0;
+}
+
+int nonconst;
+
+template <typename T>
+static void fn2 ()
+{
+  static __constinit auto v1 = nonconst; // { dg-error "does not have a constant initializer|not usable" }
+  static __constinit int v2 = nonconst; // { dg-error "does not have a constant initializer|not usable" }
+}
+
+template <typename T>
+static void fn3 ()
+{
+  static __constinit T v1 = 0;
+  static __constinit T v2 = nonconst; // { dg-error "does not have a constant initializer|not usable" }
+}
+
+void
+g ()
+{
+  fn1<int>();
+  fn2<int>();
+  fn3<int>();
+}