On 09/20/2010 09:58 AM, Rodrigo Rivas wrote:
Hello all.

This patch tries to implement the C++0x featue "Forward declarations
for enums" aka "opaque enum declarations":
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2764.pdf

Please note that this is a WIP, and as such lacks formatting,
comments, testcases, etc.

Except for the things noted below, I think it works pretty well.

TODOs
1
A enum declaration should fail if adds a list of constants and it
already have one. I check it with "TYPE_VALUES (type)", but this is
incorrect because an empty list will not be seen.
So, the current patch will accept incorrect code like:
enum class Foo { };
enum class Foo { A, B, C};
I think that a new flag is needed for ENUMERAL_TYPE, but I don't know which one.

2
I am calling "finish_enum" several types, for each of the opaque
declarations until it gets a list of constants. It doesn't seem to
cause problems except with the debug information. With default flags
and everything, gdb sees only the first declaration:
enum class Foo;
enum class Foo {A, B, C}
Foo f;

(gdb) ptype f;
enum Foo {}

I don't see an easy way to solve it...

3
I don't like very much the added parameter to the "start_enum"
function, but I don't see how to do it without breaking existing code.

Comments are most welcomed.

Regards.
Rodrigo

I did a very cursory look at the patch.

I had to initialize the variable nested_being_defined to get it to compile (possible uninitialized warning). I initialized it to false.
I added some test cases mostly from the paper.

make check is still running. I know I don't have the deja-gnu error trapping right so there will be FAILs that shouldn't be there.

I'll look in greater detail at your other points tonight.

It looks like the first two are related. What does an enum look like in tree? Maybe the tree for enum could have a has_definition flag. Set it to -1 on the first declaration. Subsequent declarations would know not to finish_enum. Set it to 1 on seeing the definition. subsequent definitions would fail.

Ed





Index: gcc/testsuite/g++.dg/cpp0x/enum_decl1.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/enum_decl1.C     (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/enum_decl1.C     (revision 0)
@@ -0,0 +1,46 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++0x" }
+
+//  Test opaque enum declarations (pre-declared enumerations).
+//  Basic tests of declaration, definition and access.
+
+// Predeclare...
+
+enum E : short;
+
+enum class G : short;
+
+enum class H;
+
+// Redeclare...
+
+enum E : short;
+
+enum class G : short;
+
+enum class H;
+
+enum class H : int;
+
+// Define...
+
+enum E : short
+{
+  face = 0x0cab,
+  beef = 0x0bee
+};
+
+enum class H
+{
+  thing1,
+  thing2
+};
+
+// Use...
+
+void
+test1()
+{
+  short e = face;
+  H h = H::thing1;
+}
Index: gcc/testsuite/g++.dg/cpp0x/enum_decl2.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/enum_decl2.C     (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/enum_decl2.C     (revision 0)
@@ -0,0 +1,10 @@
+// { dg-do compile }
+
+//  Test opaque enum declarations (pre-declared enumerations).
+//  Predeclared enums should fail in C++98.
+
+enum E : short; // { dg-warning "scoped enums only available with -std=c++0x 
or -std=gnu++0x" } { dg-error "" }
+
+enum class G : short; // { dg-warning "scoped enums only available with 
-std=c++0x or -std=gnu++0x" } { dg-error "" }
+
+enum class H; // { dg-warning "scoped enums only available with -std=c++0x or 
-std=gnu++0x" } { dg-error "" }
Index: gcc/testsuite/g++.dg/cpp0x/enum_decl3.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/enum_decl3.C     (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/enum_decl3.C     (revision 0)
@@ -0,0 +1,18 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++0x" }
+
+//  Test opaque enum declarations (pre-declared enumerations).
+//  Test declaration of nested enumerations and scoped definition.
+
+struct S
+{
+  enum E : int;
+  E e;
+};
+
+enum S::E : int
+{
+  A,
+  B,
+  C
+};
Index: gcc/testsuite/g++.dg/cpp0x/enum_decl4.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/enum_decl4.C     (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/enum_decl4.C     (revision 0)
@@ -0,0 +1,40 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++0x" }
+
+//  Test opaque enum declarations (pre-declared enumerations).
+//  You still need the full definition bfore use.
+
+// Predeclare...
+
+enum E : short;
+
+enum class H;
+
+// Redeclare...
+
+enum E : short;
+
+enum class H;
+
+// Use...
+
+void
+test1()
+{
+  short e = face;  // { dg-warning "was not declared in this scope" }
+  H h = H::thing1; // { dg-warning "is not a member of" }
+}
+
+// Define...
+
+enum E : short
+{
+  face = 0x0cab,
+  beef = 0x0bee
+};
+
+enum class H
+{
+  thing1,
+  thing2
+};
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c       (revision 164248)
+++ gcc/cp/decl.c       (working copy)
@@ -11319,27 +11319,48 @@
    may be used to declare the individual values as they are read.  */
 
 tree
-start_enum (tree name, tree underlying_type, bool scoped_enum_p)
+start_enum (tree name, tree enumtype, tree underlying_type, bool scoped_enum_p)
 {
-  tree enumtype;
-
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
+  /* [C++0x dcl.enum]p5: 
+
+    If not explicitly specified, the underlying type of a scoped
+    enumeration type is int.  */
+  if (!underlying_type && scoped_enum_p)
+    underlying_type = integer_type_node;
+
   /* If this is the real definition for a previous forward reference,
      fill in the contents in the same object that used to be the
      forward reference.  */
-
-  enumtype = lookup_and_check_tag (enum_type, name,
-                                  /*tag_scope=*/ts_current,
-                                  /*template_header_p=*/false);
-
+  if (!enumtype)
+    enumtype = lookup_and_check_tag (enum_type, name,
+                                    /*tag_scope=*/ts_current,
+                                    /*template_header_p=*/false);
   if (enumtype != NULL_TREE && TREE_CODE (enumtype) == ENUMERAL_TYPE)
     {
-      error_at (input_location, "multiple definition of %q#T", enumtype);
-      error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
-               "previous definition here");
-      /* Clear out TYPE_VALUES, and start again.  */
-      TYPE_VALUES (enumtype) = NULL_TREE;
+      if (scoped_enum_p != SCOPED_ENUM_P (enumtype))
+       {
+         error_at (input_location, "scoped/unscoped mismatch in enum %q#T", 
enumtype);
+         error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+                   "previous definition here");
+       }
+      if (!underlying_type
+         || !same_type_p (underlying_type, ENUM_UNDERLYING_TYPE (enumtype)))
+       {
+         error_at (input_location, "different underlying type in enum %q#T", 
enumtype);
+         error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+                   "previous definition here");
+       }
+
+      if (cxx_dialect == cxx98)
+       {
+         error_at (input_location, "multiple definition of %q#T", enumtype);
+         error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+                   "previous definition here");
+         /* Clear out TYPE_VALUES, and start again.  */
+         TYPE_VALUES (enumtype) = NULL_TREE;
+       }
     }
   else
     {
@@ -11355,19 +11376,10 @@
   if (enumtype == error_mark_node)
     return enumtype;
 
+  SET_SCOPED_ENUM_P (enumtype, scoped_enum_p);
   if (scoped_enum_p)
-    {
-      SET_SCOPED_ENUM_P (enumtype, 1);
-      begin_scope (sk_scoped_enum, enumtype);
+    begin_scope (sk_scoped_enum, enumtype);
 
-      /* [C++0x dcl.enum]p5: 
-
-          If not explicitly specified, the underlying type of a scoped
-          enumeration type is int.  */
-      if (!underlying_type)
-        underlying_type = integer_type_node;
-    }
-
   if (underlying_type)
     {
       if (CP_INTEGRAL_TYPE_P (underlying_type))
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c (revision 164248)
+++ gcc/cp/pt.c (working copy)
@@ -6654,7 +6654,7 @@
          if (!is_dependent_type)
            {
              set_current_access_from_decl (TYPE_NAME (template_type));
-             t = start_enum (TYPE_IDENTIFIER (template_type),
+             t = start_enum (TYPE_IDENTIFIER (template_type), NULL_TREE,
                               tsubst (ENUM_UNDERLYING_TYPE (template_type),
                                       arglist, complain, in_decl),
                               SCOPED_ENUM_P (template_type));
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c     (revision 164248)
+++ gcc/cp/parser.c     (working copy)
@@ -13212,10 +13212,13 @@
 cp_parser_enum_specifier (cp_parser* parser)
 {
   tree identifier;
-  tree type;
+  tree type = NULL_TREE;
+  tree nested_name_specifier;
   tree attributes;
   bool scoped_enum_p = false;
   bool has_underlying_type = false;
+  bool nested_being_defined = false;
+  bool must_finish = true;
   tree underlying_type = NULL_TREE;
 
   /* Parse tentatively so that we can back up if we don't find a
@@ -13244,10 +13247,36 @@
 
   attributes = cp_parser_attributes_opt (parser);
 
-  if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
-    identifier = cp_parser_identifier (parser);
+  push_deferring_access_checks (dk_no_check);
+  nested_name_specifier
+    = cp_parser_nested_name_specifier_opt (parser,
+                                          /*typename_keyword_p=*/false,
+                                          /*check_dependency_p=*/false,
+                                          /*type_p=*/false,
+                                          /*is_declaration=*/false);
+
+  if (nested_name_specifier)
+    {
+      tree name;
+      identifier = cp_parser_identifier (parser);
+      name =  cp_parser_lookup_name (parser, identifier,
+                                    enum_type,
+                                    /*is_template=*/false,
+                                    /*is_namespace=*/false,
+                                    /*check_dependency=*/true,
+                                    /*ambiguous_decls=*/NULL,
+                                    input_location);
+      if (name)
+       type = TREE_TYPE (name);
+    }
   else
-    identifier = make_anon_name ();
+    {
+      if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
+       identifier = cp_parser_identifier (parser);
+      else
+       identifier = make_anon_name ();
+    }
+  pop_deferring_access_checks ();
 
   /* Check for the `:' that denotes a specified underlying type in C++0x.
      Note that a ':' could also indicate a bitfield width, however.  */
@@ -13285,14 +13314,31 @@
   /* Look for the `{' but don't consume it yet.  */
   if (!cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
     {
-      cp_parser_error (parser, "expected %<{%>");
-      if (has_underlying_type)
-       return NULL_TREE;
+      if (cxx_dialect == cxx98 || (!scoped_enum_p && !underlying_type))
+       {
+         cp_parser_error (parser, "expected %<{%>");
+         if (has_underlying_type)
+           return NULL_TREE;
+       }
     }
 
   if (!has_underlying_type && !cp_parser_parse_definitely (parser))
     return NULL_TREE;
 
+  if (nested_name_specifier && !scoped_enum_p)
+    {
+      if (CLASS_TYPE_P (nested_name_specifier))
+       {
+         nested_being_defined = TYPE_BEING_DEFINED (nested_name_specifier);
+         TYPE_BEING_DEFINED (nested_name_specifier) = 1;
+         push_scope (nested_name_specifier);
+       }
+      else if ( TREE_CODE(nested_name_specifier) == NAMESPACE_DECL)
+       {
+         push_nested_namespace (nested_name_specifier);
+       }
+    }
+
   /* Issue an error message if type-definitions are forbidden here.  */
   if (!cp_parser_check_type_definition (parser))
     type = error_mark_node;
@@ -13300,24 +13346,42 @@
     /* Create the new type.  We do this before consuming the opening
        brace so the enum will be recorded as being on the line of its
        tag (or the 'enum' keyword, if there is no tag).  */
-    type = start_enum (identifier, underlying_type, scoped_enum_p);
-  
-  /* Consume the opening brace.  */
-  cp_lexer_consume_token (parser->lexer);
+    type = start_enum (identifier, type, underlying_type, scoped_enum_p);
 
-  if (type == error_mark_node)
+  /* If the next token is not '{' it is an opaque enum.  */
+  if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
     {
-      cp_parser_skip_to_end_of_block_or_statement (parser);
-      return error_mark_node;
-    }
+      /* Consume the opening brace.  */
+      cp_lexer_consume_token (parser->lexer);
 
-  /* If the next token is not '}', then there are some enumerators.  */
-  if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_BRACE))
-    cp_parser_enumerator_list (parser, type);
+      if (TYPE_VALUES (type)) /* TODO: should be ENUM_HAS_VALUE_LIST_P (type) 
*/
+       {
+         error_at (input_location, "multiple definition of %q#T", type);
+         error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)),
+                   "previous definition here");
+         type = error_mark_node;
+       }
 
-  /* Consume the final '}'.  */
-  cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE);
+      if (type == error_mark_node)
+       {
+         cp_parser_skip_to_end_of_block_or_statement (parser);
+         return error_mark_node;
+       }
 
+      /* If the next token is not '}', then there are some enumerators.  */
+      if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_BRACE))
+       cp_parser_enumerator_list (parser, type);
+
+      /* Consume the final '}'.  */
+      cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE);
+    }
+  else
+    {
+      /* If this enum has value list from a previous declaration, then it has 
already
+         been finished. Do not do it again.  */
+      must_finish = !TYPE_VALUES (type); /* TODO: should be 
ENUM_HAS_VALUE_LIST_P (type) */
+    }
+
   /* Look for trailing attributes to apply to this enumeration, and
      apply them if appropriate.  */
   if (cp_parser_allow_gnu_extensions_p (parser))
@@ -13330,8 +13394,21 @@
     }
 
   /* Finish up the enumeration.  */
-  finish_enum (type);
+  if (must_finish)
+    finish_enum (type);
 
+  if (nested_name_specifier && !scoped_enum_p)
+    {
+      if (CLASS_TYPE_P (nested_name_specifier))
+       {
+         TYPE_BEING_DEFINED (nested_name_specifier) = nested_being_defined;
+         pop_scope (nested_name_specifier);
+       }
+      else if ( TREE_CODE(nested_name_specifier) == NAMESPACE_DECL)
+       {
+         pop_nested_namespace (nested_name_specifier);
+       }
+    }
   return type;
 }
 
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h    (revision 164248)
+++ gcc/cp/cp-tree.h    (working copy)
@@ -4778,7 +4778,7 @@
 extern tree xref_tag                           (enum tag_types, tree, 
tag_scope, bool);
 extern tree xref_tag_from_type                 (tree, tree, tag_scope);
 extern bool xref_basetypes                     (tree, tree);
-extern tree start_enum                         (tree, tree, bool);
+extern tree start_enum                         (tree, tree, tree, bool);
 extern void finish_enum                                (tree);
 extern void build_enumerator                   (tree, tree, tree, location_t);
 extern tree lookup_enumerator                  (tree, tree);

Reply via email to