diff mbox series

c++: Completeness of typedef structs [PR 99294]

Message ID ad686964-d251-3e2c-82e8-9e2332c6f48f@acm.org
State New
Headers show
Series c++: Completeness of typedef structs [PR 99294] | expand

Commit Message

Nathan Sidwell March 1, 2021, 1:56 p.m. UTC
When we	read in	a class	definition, we use fixup_type_variants to 
propagate the now-completed fields of the class's TYPE to other 
variants.  Unfortunately that doesn't propagate all of them, and in this 
case we had a typedef to an (incomplete) instantiation.  That typedef 
ended up with a VOIDmode, which blew up gimple expansion as the type 
itself isn't VOID.  Without modules, that information is propagated in 
finalize_type_size when laying out the class.  But that doesn't happen 
with stream-in -- we already know the layout.  There is already some 
overlap between the two functions, now there's a bit more.  In 
fixup_type_variants, I pay attention to the TYPE_NAME	to decide whether 
to override a user's TYPE_ALIGN -- variants of the main-variant typedef 
just copy the main-variant.  Other variants recalculate.  Overaligning 
is still permitted.

I also added a TYPE_ALIGN_RAW accessor,	and fixed a bug	in the alignment 
streaming I noticed.	I did not refactor TYPE_ALIGN beyond using the new 
accessor.  (It could be written as ((1 << align_raw) >> 1), rather than 
use the conditional.)

         PR c++/99294
         gcc/
         * gcc/tree.h (TYPE_ALIGN_RAW): New accessor.
         (TYPE_ALIGN): Use it.
         gcc/cp/
         * class.c (fixup_type_variants): Propagate mode, precision,
         alignment & emptiness.
         * module.cc (trees_out::type_node): Use TYPE_ALIGN_RAW.
         (trees_in::tree_node): Rematerialize alignment here.
         gcc/testsuite/
         * g++.dg/modules/pr99294.h: New.
         * g++.dg/modules/pr99294_a.C: New.
         * g++.dg/modules/pr99294_b.C: New.
diff mbox series

Patch

diff --git c/gcc/cp/class.c w/gcc/cp/class.c
index 40f5fef7baa..ea007e88e6e 100644
--- c/gcc/cp/class.c
+++ w/gcc/cp/class.c
@@ -2005,35 +2005,45 @@  determine_primary_bases (tree t)
 /* Update the variant types of T.  */
 
 void
-fixup_type_variants (tree t)
+fixup_type_variants (tree type)
 {
-  tree variants;
-
-  if (!t)
+  if (!type)
     return;
 
-  for (variants = TYPE_NEXT_VARIANT (t);
-       variants;
-       variants = TYPE_NEXT_VARIANT (variants))
+  for (tree variant = TYPE_NEXT_VARIANT (type);
+       variant;
+       variant = TYPE_NEXT_VARIANT (variant))
     {
       /* These fields are in the _TYPE part of the node, not in
 	 the TYPE_LANG_SPECIFIC component, so they are not shared.  */
-      TYPE_HAS_USER_CONSTRUCTOR (variants) = TYPE_HAS_USER_CONSTRUCTOR (t);
-      TYPE_NEEDS_CONSTRUCTING (variants) = TYPE_NEEDS_CONSTRUCTING (t);
-      TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variants)
-	= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t);
+      TYPE_HAS_USER_CONSTRUCTOR (variant) = TYPE_HAS_USER_CONSTRUCTOR (type);
+      TYPE_NEEDS_CONSTRUCTING (variant) = TYPE_NEEDS_CONSTRUCTING (type);
+      TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variant)
+	= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type);
 
-      TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t);
-      CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t);
+      TYPE_POLYMORPHIC_P (variant) = TYPE_POLYMORPHIC_P (type);
+      CLASSTYPE_FINAL (variant) = CLASSTYPE_FINAL (type);
 
-      TYPE_BINFO (variants) = TYPE_BINFO (t);
+      TYPE_BINFO (variant) = TYPE_BINFO (type);
 
       /* Copy whatever these are holding today.  */
-      TYPE_VFIELD (variants) = TYPE_VFIELD (t);
-      TYPE_FIELDS (variants) = TYPE_FIELDS (t);
+      TYPE_VFIELD (variant) = TYPE_VFIELD (type);
+      TYPE_FIELDS (variant) = TYPE_FIELDS (type);
+
+      TYPE_SIZE (variant) = TYPE_SIZE (type);
+      TYPE_SIZE_UNIT (variant) = TYPE_SIZE_UNIT (type);
+
+      if (!TYPE_USER_ALIGN (variant)
+	  || TYPE_NAME (variant) == TYPE_NAME (type)
+	  || TYPE_ALIGN_RAW (variant) < TYPE_ALIGN_RAW (type))
+	{
+	  TYPE_ALIGN_RAW (variant) =  TYPE_ALIGN_RAW (type);
+	  TYPE_USER_ALIGN (variant) = TYPE_USER_ALIGN (type);
+	}
 
-      TYPE_SIZE (variants) = TYPE_SIZE (t);
-      TYPE_SIZE_UNIT (variants) = TYPE_SIZE_UNIT (t);
+      TYPE_PRECISION (variant) = TYPE_PRECISION (type);
+      TYPE_MODE_RAW (variant) = TYPE_MODE_RAW (type);
+      TYPE_EMPTY_P (variant) = TYPE_EMPTY_P (type);
     }
 }
 
diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc
index 0cb5bd9b644..369a02604fe 100644
--- c/gcc/cp/module.cc
+++ w/gcc/cp/module.cc
@@ -8714,7 +8714,7 @@  trees_out::type_node (tree type)
       else
 	{
 	  if (TYPE_USER_ALIGN (type))
-	    flags = exact_log2 (TYPE_ALIGN (type));
+	    flags = TYPE_ALIGN_RAW (type);
 	}
 
       if (streaming_p ())
@@ -9510,7 +9510,7 @@  trees_in::tree_node (bool is_use)
 	  }
 	else
 	  {
-	    res = build_aligned_type (res, 1u << flags);
+	    res = build_aligned_type (res, (1u << flags) >> 1);
 	    TYPE_USER_ALIGN (res) = true;
 	  }
 
diff --git c/gcc/testsuite/g++.dg/modules/pr99294.h w/gcc/testsuite/g++.dg/modules/pr99294.h
new file mode 100644
index 00000000000..757113c0283
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99294.h
@@ -0,0 +1,14 @@ 
+
+template <typename T>
+class basic_string;
+
+typedef basic_string<char> string;
+
+template <typename T>
+class basic_string
+{
+ public:
+  string Frob ();
+
+  basic_string (int);
+};
diff --git c/gcc/testsuite/g++.dg/modules/pr99294_a.C w/gcc/testsuite/g++.dg/modules/pr99294_a.C
new file mode 100644
index 00000000000..ac8b9a7de5b
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99294_a.C
@@ -0,0 +1,18 @@ 
+// PR 99294, ICE with -fno-module-lazy on class completeness
+// { dg-additional-options -fmodules-ts }
+
+// The instantiation of the *definition* of basic_string is used in
+// importers, *after* they have instantiated a declaration of it *and*
+// created type variants.
+
+module;
+
+#include "pr99294.h"
+
+export module foo;
+// { dg-module-cmi foo }
+
+export inline int greeter (string const &bob)
+{
+  return sizeof (bob); // instantiates string
+}
diff --git c/gcc/testsuite/g++.dg/modules/pr99294_b.C w/gcc/testsuite/g++.dg/modules/pr99294_b.C
new file mode 100644
index 00000000000..30d14d10187
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99294_b.C
@@ -0,0 +1,12 @@ 
+// PR 99294, ICE with -fno-module-lazy on class completeness
+// { dg-additional-options {-fmodules-ts -fno-module-lazy} }
+
+#include "pr99294.h"
+import foo;
+
+string Quux ()
+{
+  return 1;
+}
+
+// ICED complaining about Quux RETURN_DECL during gimple expand
diff --git c/gcc/tree.h w/gcc/tree.h
index 4f33868e8e1..f00ea2ef0af 100644
--- c/gcc/tree.h
+++ w/gcc/tree.h
@@ -2078,12 +2078,16 @@  extern tree vector_element_bits_tree (const_tree);
    to this type.  */
 #define TYPE_ATTRIBUTES(NODE) (TYPE_CHECK (NODE)->type_common.attributes)
 
+/* Raw access to the alignment field.  */
+#define TYPE_ALIGN_RAW(NODE)			\
+  (TYPE_CHECK (NODE)->type_common.align)
+
 /* The alignment necessary for objects of this type.
    The value is an int, measured in bits and must be a power of two.
    We support also an "alignment" of zero.  */
-#define TYPE_ALIGN(NODE) \
-    (TYPE_CHECK (NODE)->type_common.align \
-     ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0)
+#define TYPE_ALIGN(NODE)					\
+  (TYPE_ALIGN_RAW (NODE)					\
+   ? ((unsigned)1) << (TYPE_ALIGN_RAW(NODE) - 1) : 0)
 
 /* Specify that TYPE_ALIGN(NODE) is X.  */
 #define SET_TYPE_ALIGN(NODE, X) \