diff mbox series

[PR,c++/84962] ICE with anon-struct member

Message ID c6049b51-05df-a27a-843b-662499ea2a27@acm.org
State New
Headers show
Series [PR,c++/84962] ICE with anon-struct member | expand

Commit Message

Nathan Sidwell March 20, 2018, 1:55 p.m. UTC
This fixes the 84962 ICE, which is where we name a member function of an 
anon-struct in the width specifier of a bitfield.  The problem is that 
when permissive we allow some of this strangeness. Prior to my lookup 
changes, we only added TYPE_FIELDS and we still do that.  Trouble is 
that now contains member functions, and badness happens.  The fix is to 
add the member_vec members, if that exists, which takes care of overloaded.

Due to history, we don't check some of the anon-struct restrictions 
until the outer struct is complete.  That's partly historical and partly 
because we don't tell the struct parser the difference between

struct X {
    struct { ... };  // anon-struct
    typedef struct { ... } name; // not an anon-struct
};

It'd be nice to fix that, but not right now.

However, all this stuff is an extension:
1) we permit anon-struct, not just anon-union
2) we permit things other than non-static-data members

Maybe we could deprecate #2 at least?  I'm not sure what its use case 
might be.

nathan

Comments

Jason Merrill March 20, 2018, 4:30 p.m. UTC | #1
On Tue, Mar 20, 2018 at 9:55 AM, Nathan Sidwell <nathan@acm.org> wrote:
> This fixes the 84962 ICE, which is where we name a member function of an
> anon-struct in the width specifier of a bitfield.  The problem is that when
> permissive we allow some of this strangeness. Prior to my lookup changes, we
> only added TYPE_FIELDS and we still do that.  Trouble is that now contains
> member functions, and badness happens.  The fix is to add the member_vec
> members, if that exists, which takes care of overloaded.
>
> Due to history, we don't check some of the anon-struct restrictions until
> the outer struct is complete.  That's partly historical and partly because
> we don't tell the struct parser the difference between
>
> struct X {
>    struct { ... };  // anon-struct
>    typedef struct { ... } name; // not an anon-struct

Right, because we can't tell until after the struct:

     struct { ... } member; // also not an anon-struct

> However, all this stuff is an extension:
> 1) we permit anon-struct, not just anon-union
> 2) we permit things other than non-static-data members

> Maybe we could deprecate #2 at least?  I'm not sure what its use case might
> be.

Fine with me.  I know #1 was deliberate, but I haven't been aware of #2 before.

Jason
diff mbox series

Patch

2018-03-20  Nathan Sidwell  <nathan@acm.org>

	PR c++/84962
	* name-lookup.c (pushdecl_class_level): Push anon-struct's
	member_vec, if there is one.

	PR c++/84962
	* g++.dg/lookup/pr84962.C: New.

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 258676)
+++ cp/name-lookup.c	(working copy)
@@ -4490,16 +4490,30 @@  pushdecl_class_level (tree x)
       /* If X is an anonymous aggregate, all of its members are
 	 treated as if they were members of the class containing the
 	 aggregate, for naming purposes.  */
-      tree f;
-
-      for (f = TYPE_FIELDS (TREE_TYPE (x)); f; f = DECL_CHAIN (f))
-	{
-	  location_t save_location = input_location;
-	  input_location = DECL_SOURCE_LOCATION (f);
-	  if (!pushdecl_class_level (f))
-	    is_valid = false;
-	  input_location = save_location;
+      location_t save_location = input_location;
+      tree anon = TREE_TYPE (x);
+      if (vec<tree, va_gc> *member_vec = CLASSTYPE_MEMBER_VEC (anon))
+	for (unsigned ix = member_vec->length (); ix--;)
+	  {
+	    tree binding = (*member_vec)[ix];
+	    if (STAT_HACK_P (binding))
+	      {
+		if (!pushdecl_class_level (STAT_TYPE (binding)))
+		  is_valid = false;
+		binding = STAT_DECL (binding);
+	      }
+	    if (!pushdecl_class_level (binding))
+	      is_valid = false;
 	}
+      else
+	for (tree f = TYPE_FIELDS (anon); f; f = DECL_CHAIN (f))
+	  if (TREE_CODE (f) == FIELD_DECL)
+	    {
+	      input_location = DECL_SOURCE_LOCATION (f);
+	      if (!pushdecl_class_level (f))
+		is_valid = false;
+	    }
+      input_location = save_location;
     }
   timevar_cond_stop (TV_NAME_LOOKUP, subtime);
   return is_valid;
Index: testsuite/g++.dg/lookup/pr84962.C
===================================================================
--- testsuite/g++.dg/lookup/pr84962.C	(revision 0)
+++ testsuite/g++.dg/lookup/pr84962.C	(working copy)
@@ -0,0 +1,14 @@ 
+// PR c++/84952 ICE with anon-struct having member fns
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wno-pedantic }
+
+struct X {
+  struct 
+  {
+    template <typename> int a ();
+    // { dg-error "can only have" "" { target *-*-* } .-1 }
+  };
+
+  int  : a; // { dg-error "non-integral" }
+};
+