Re: [PATCH] implement support for "enum class"

[Tom Tromey <tromey at redhat dot com>]

Tom> I was anticipating that GCC would emit an unnamed type for the
Tom> underlying type in the case where the underlying type is not specified
Tom> in the source.  However I can see why it would not necessarily do this,
Tom> and certainly there's no requirement in DWARF along these lines.

Mark> Funny. You pointed out my original patch accidentally did this
Mark> sometimes. Causing lots of extra base type DIEs. I indeed fixed this to
Mark> emit the already known base type in the CU (which often is the named int
Mark> base type) to reduce duplication. I think this is the correct thing to
Mark> do.

Yeah, I didn't realize that the fix would cause this.
Actually at the time the duplicated DIEs were all nameless, so adding a
name is a further wrinkle; but not invalid.

Mark> I am also not sure what the right thing to do is here. But I think it
Mark> makes sense to make printing it depend on the language used and always
Mark> print it when the language supports "derived enums" whether the user
Mark> explicitly defined it or it was guessed by the compiler. It is an valid
Mark> expression in any case and the GDB user might appreciate the extra info
Mark> (in the case of C++).

I went this route.

Here's a new patch, this one regtested on x86-64 Fedora 20 against both
the system compiler and a GCC with your enum patches applied.  I had to
update gdb.cp/classes.exp and I added a new test as well.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e4e5db8..00b1231 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2014-03-31  Tom Tromey  <tromey@redhat.com>
+
+	PR c++/15246:
+	* c-exp.y (type_aggregate_p): New function.
+	(qualified_name, classify_inner_name): Use it.
+	* c-typeprint.c (c_type_print_base): Handle TYPE_DECLARED_CLASS
+	and TYPE_TARGET_TYPE of an enum type.
+	* dwarf2read.c (read_enumeration_type): Set TYPE_DECLARED_CLASS on
+	an enum type.
+	(determine_prefix) <case DW_TAG_enumeration_type>: New case;
+	handle TYPE_DECLARED_CLASS.
+	* gdbtypes.c (rank_one_type): Handle TYPE_DECLARED_CLASS on enum
+	types.
+	* gdbtypes.h (TYPE_DECLARED_CLASS): Update comment.
+	* valops.c (enum_constant_from_type): New function.
+	(value_aggregate_elt): Use it.
+	* cp-namespace.c (cp_lookup_nested_symbol): Handle
+	TYPE_CODE_ENUM.
+
 2014-03-27  Tom Tromey  <tromey@redhat.com>
 
 	* valops.c (value_aggregate_elt, value_struct_elt_for_reference)
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index fc79807..01af9dd 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -129,6 +129,8 @@ static int yylex (void);
 
 void yyerror (char *);
 
+static int type_aggregate_p (struct type *);
+
 %}
 
 /* Although the yacc "value" of an expression is not used,
@@ -978,9 +980,7 @@ qualified_name:	TYPENAME COLONCOLON name
 			{
 			  struct type *type = $1.type;
 			  CHECK_TYPEDEF (type);
-			  if (TYPE_CODE (type) != TYPE_CODE_STRUCT
-			      && TYPE_CODE (type) != TYPE_CODE_UNION
-			      && TYPE_CODE (type) != TYPE_CODE_NAMESPACE)
+			  if (!type_aggregate_p (type))
 			    error (_("`%s' is not defined as an aggregate type."),
 				   TYPE_SAFE_NAME (type));
 
@@ -996,9 +996,7 @@ qualified_name:	TYPENAME COLONCOLON name
 			  char *buf;
 
 			  CHECK_TYPEDEF (type);
-			  if (TYPE_CODE (type) != TYPE_CODE_STRUCT
-			      && TYPE_CODE (type) != TYPE_CODE_UNION
-			      && TYPE_CODE (type) != TYPE_CODE_NAMESPACE)
+			  if (!type_aggregate_p (type))
 			    error (_("`%s' is not defined as an aggregate type."),
 				   TYPE_SAFE_NAME (type));
 			  buf = alloca ($4.length + 2);
@@ -1693,6 +1691,18 @@ operator_stoken (const char *op)
   return st;
 };
 
+/* Return true if the type is aggregate-like.  */
+
+static int
+type_aggregate_p (struct type *type)
+{
+  return (TYPE_CODE (type) == TYPE_CODE_STRUCT
+	  || TYPE_CODE (type) == TYPE_CODE_UNION
+	  || TYPE_CODE (type) == TYPE_CODE_NAMESPACE
+	  || (TYPE_CODE (type) == TYPE_CODE_ENUM
+	      && TYPE_DECLARED_CLASS (type)));
+}
+
 /* Validate a parameter typelist.  */
 
 static void
@@ -2992,9 +3002,7 @@ classify_inner_name (struct parser_state *par_state,
     return classify_name (par_state, block, 0);
 
   type = check_typedef (context);
-  if (TYPE_CODE (type) != TYPE_CODE_STRUCT
-      && TYPE_CODE (type) != TYPE_CODE_UNION
-      && TYPE_CODE (type) != TYPE_CODE_NAMESPACE)
+  if (!type_aggregate_p (type))
     return ERROR;
 
   copy = copy_name (yylval.ssym.stoken);
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 4edc9ec..3d647da 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1324,6 +1324,8 @@ c_type_print_base (struct type *type, struct ui_file *stream,
     case TYPE_CODE_ENUM:
       c_type_print_modifier (type, stream, 0, 1);
       fprintf_filtered (stream, "enum ");
+      if (TYPE_DECLARED_CLASS (type))
+	fprintf_filtered (stream, "class ");
       /* Print the tag name if it exists.
          The aCC compiler emits a spurious 
          "{unnamed struct}"/"{unnamed union}"/"{unnamed enum}"
@@ -1349,6 +1351,23 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 	{
 	  LONGEST lastval = 0;
 
+	  /* We can't handle this case perfectly, as DWARF does not
+	     tell us whether or not the underlying type was specified
+	     in the source (and other debug formats don't provide this
+	     at all).  We choose to print the underlying type, if it
+	     has a name, when in C++ on the theory that it's better to
+	     print too much than too little; but conversely not to
+	     print something egregiously outside the current
+	     language's syntax.  */
+	  if (current_language->la_language == language_cplus
+	      && TYPE_TARGET_TYPE (type) != NULL)
+	    {
+	      struct type *underlying = check_typedef (TYPE_TARGET_TYPE (type));
+
+	      if (TYPE_NAME (underlying) != NULL)
+		fprintf_filtered (stream, ": %s ", TYPE_NAME (underlying));
+	    }
+
 	  fprintf_filtered (stream, "{");
 	  len = TYPE_NFIELDS (type);
 	  for (i = 0; i < len; i++)
diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 74ccd45..03eb6a9 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -812,6 +812,7 @@ cp_lookup_nested_symbol (struct type *parent_type,
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_NAMESPACE:
     case TYPE_CODE_UNION:
+    case TYPE_CODE_ENUM:
     /* NOTE: Handle modules here as well, because Fortran is re-using the C++
        specific code to lookup nested symbols in modules, by calling the
        function pointer la_lookup_symbol_nonlocal, which ends up here.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 964c956..ccbd55f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13269,6 +13269,8 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu)
 	TYPE_LENGTH (type) = TYPE_LENGTH (TYPE_TARGET_TYPE (type));
     }
 
+  TYPE_DECLARED_CLASS (type) = dwarf2_flag_true_p (die, DW_AT_enum_class, cu);
+
   return set_die_type (die, type, cu);
 }
 
@@ -18611,6 +18613,15 @@ determine_prefix (struct die_info *die, struct dwarf2_cu *cu)
 	      return name;
 	  }
 	return "";
+      case DW_TAG_enumeration_type:
+	parent_type = read_type_die (parent, cu);
+	if (TYPE_DECLARED_CLASS (parent_type))
+	  {
+	    if (TYPE_TAG_NAME (parent_type) != NULL)
+	      return TYPE_TAG_NAME (parent_type);
+	    return "";
+	  }
+	/* Fall through.  */
       default:
 	return determine_prefix (parent, cu);
       }
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 98cb873..8feb745 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2947,6 +2947,8 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 	case TYPE_CODE_CHAR:
 	case TYPE_CODE_RANGE:
 	case TYPE_CODE_BOOL:
+	  if (TYPE_DECLARED_CLASS (arg))
+	    return INCOMPATIBLE_TYPE_BADNESS;
 	  return INTEGER_PROMOTION_BADNESS;
 	case TYPE_CODE_FLT:
 	  return INT_FLOAT_CONVERSION_BADNESS;
@@ -2964,6 +2966,8 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 	case TYPE_CODE_RANGE:
 	case TYPE_CODE_BOOL:
 	case TYPE_CODE_ENUM:
+	  if (TYPE_DECLARED_CLASS (parm) || TYPE_DECLARED_CLASS (arg))
+	    return INCOMPATIBLE_TYPE_BADNESS;
 	  return INTEGER_CONVERSION_BADNESS;
 	case TYPE_CODE_FLT:
 	  return INT_FLOAT_CONVERSION_BADNESS;
@@ -2977,6 +2981,8 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 	case TYPE_CODE_RANGE:
 	case TYPE_CODE_BOOL:
 	case TYPE_CODE_ENUM:
+	  if (TYPE_DECLARED_CLASS (arg))
+	    return INCOMPATIBLE_TYPE_BADNESS;
 	  return INTEGER_CONVERSION_BADNESS;
 	case TYPE_CODE_FLT:
 	  return INT_FLOAT_CONVERSION_BADNESS;
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c6943ef..9599e1c 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -330,8 +330,10 @@ enum type_instance_flag_value
 #define TYPE_OBJFILE(t) (TYPE_OBJFILE_OWNED(t)? TYPE_OWNER(t).objfile : NULL)
 
 /* * True if this type was declared using the "class" keyword.  This is
-   only valid for C++ structure types, and only used for displaying
-   the type.  If false, the structure was declared as a "struct".  */
+   only valid for C++ structure and enum types.  If false, a structure
+   was declared as a "struct"; if true it was declared "class".  For
+   enum types, this is true when "enum class" or "enum struct" was
+   used to declare the type..  */
 
 #define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class)
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0711f14..b8de0e3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,13 @@
 2014-03-31  Tom Tromey  <tromey@redhat.com>
 
+	* gdb.cp/classes.exp (test_enums): Handle underlying type.
+	* gdb.dwarf2/enum-type.exp: Add test for enum with underlying
+	type.
+	* gdb.cp/enum-class.exp: New file.
+	* gdb.cp/enum-class.cc: New file.
+
+2014-03-31  Tom Tromey  <tromey@redhat.com>
+
 	* gdb.dwarf2/enum-type.exp: New file.
 
 2014-03-31  Yao Qi  <yao@codesourcery.com>
diff --git a/gdb/testsuite/gdb.cp/classes.exp b/gdb/testsuite/gdb.cp/classes.exp
index 7c1a5d1..0141e3a 100644
--- a/gdb/testsuite/gdb.cp/classes.exp
+++ b/gdb/testsuite/gdb.cp/classes.exp
@@ -410,7 +410,7 @@ proc test_enums {} {
     # ptype on the enum member
 
     gdb_test_multiple "ptype obj_with_enum.priv_enum" "ptype obj_with_enum.priv_enum" {
-        -re "type = enum ClassWithEnum::PrivEnum \{ ?(ClassWithEnum::)?red, (ClassWithEnum::)?green, (ClassWithEnum::)?blue, (ClassWithEnum::)?yellow = 42 ?\}$nl$gdb_prompt $" {
+        -re "type = enum ClassWithEnum::PrivEnum (: unsigned int )?\{ ?(ClassWithEnum::)?red, (ClassWithEnum::)?green, (ClassWithEnum::)?blue, (ClassWithEnum::)?yellow = 42 ?\}$nl$gdb_prompt $" {
 	    pass "ptype obj_with_enum.priv_enum"
         }
 	-re "type = enum PrivEnum \{ ?(ClassWithEnum::)?red, (ClassWithEnum::)?green, (ClassWithEnum::)?blue, (ClassWithEnum::)?yellow = 42 ?\}$nl$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.cp/enum-class.cc b/gdb/testsuite/gdb.cp/enum-class.cc
new file mode 100644
index 0000000..bddfbbb
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum-class.cc
@@ -0,0 +1,46 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+enum class E1 {
+  HI = 7, THERE
+};
+
+enum class E2 {
+  HI = 23, THERE
+};
+
+// overload1(E1::HI) is ok.
+// overload1(77) is ok.
+int overload1 (int v) { return 0; }
+int overload1 (E1 v) { return static_cast<int> (v); }
+int overload1 (E2 v) { return - static_cast<int> (v); }
+
+// overload2(E1::HI) is ok.
+// overload1(77) fails.
+int overload2 (E1 v) { return static_cast<int> (v); }
+int overload2 (E2 v) { return - static_cast<int> (v); }
+
+// overload3(E1::HI) fails.
+// overload1(77) is ok.
+int overload3 (int v) { return 0; }
+int overload3 (E2 v) { return static_cast<int> (v); }
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/enum-class.exp b/gdb/testsuite/gdb.cp/enum-class.exp
new file mode 100644
index 0000000..2246f50
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/enum-class.exp
@@ -0,0 +1,48 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+if {[skip_cplus_tests]} { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile \
+	 {debug c++ additional_flags=-std=c++11}]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_test "ptype E1" \
+    "type = enum class E1 (: int )?{E1::HI = 7, E1::THERE}"
+
+gdb_test "print E1::HI" " = E1::HI"
+gdb_test "print (int) E1::HI" " = 7"
+gdb_test "print (int) E2::HI" " = 23"
+gdb_test "print HI" "No symbol .HI.*"
+
+gdb_test "print overload1(E1::THERE)" " = 8"
+gdb_test "print overload1(77)" " = 0"
+
+gdb_test "print overload2(E1::THERE)" " = 8"
+gdb_test "print overload2(77)" \
+    "Cannot resolve function overload2 to any overloaded instance"
+
+gdb_test "print overload3(E1::THERE)" \
+    "Cannot resolve function overload3 to any overloaded instance"
+gdb_test "print overload3(77)" " = 0"
diff --git a/gdb/testsuite/gdb.dwarf2/enum-type.exp b/gdb/testsuite/gdb.dwarf2/enum-type.exp
index 60457a6..a4825ec 100644
--- a/gdb/testsuite/gdb.dwarf2/enum-type.exp
+++ b/gdb/testsuite/gdb.dwarf2/enum-type.exp
@@ -30,8 +30,7 @@ Dwarf::assemble $asm_file {
                 {DW_AT_name     enum-type-dw.c}
                 {DW_AT_comp_dir /tmp}
         } {
-	    declare_labels integer_label array_elt_label array_label \
-                big_array_label
+	    declare_labels integer_label uinteger_label
 
             integer_label: DW_TAG_base_type {
                 {DW_AT_byte_size 4 DW_FORM_sdata}
@@ -39,6 +38,12 @@ Dwarf::assemble $asm_file {
                 {DW_AT_name      integer}
             }
 
+            uinteger_label: DW_TAG_base_type {
+                {DW_AT_byte_size 4 DW_FORM_sdata}
+                {DW_AT_encoding  @DW_ATE_unsigned}
+                {DW_AT_name      {unsigned integer}}
+            }
+
 	    DW_TAG_enumeration_type {
 		{DW_AT_name E}
 		{DW_AT_type :$integer_label}
@@ -48,6 +53,16 @@ Dwarf::assemble $asm_file {
 		    {DW_AT_const_value 1}
 		}
 	    }
+
+	    DW_TAG_enumeration_type {
+		{DW_AT_name EU}
+		{DW_AT_type :$uinteger_label}
+	    } {
+		DW_TAG_enumerator {
+		    {DW_AT_name TWO}
+		    {DW_AT_const_value 2 DW_FORM_sdata}
+		}
+	    }
 	}
     }
 }
@@ -58,3 +73,9 @@ if { [prepare_for_testing ${testfile}.exp ${testfile} \
 }
 
 gdb_test "print sizeof(enum E)" " = 4"
+
+gdb_test "ptype enum EU" "type = enum EU {TWO = 2}" \
+    "ptype EU in enum C"
+gdb_test_no_output "set lang c++"
+gdb_test "ptype enum EU" "type = enum EU : unsigned integer {TWO = 2}" \
+    "ptype EU in C++"
diff --git a/gdb/valops.c b/gdb/valops.c
index a10dbd6..e59e075 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3007,6 +3007,42 @@ destructor_name_p (const char *name, struct type *type)
   return 0;
 }
 
+/* Find an enum constant named NAME in TYPE.  TYPE must be an "enum
+   class".  If the name is found, return a value representing it;
+   otherwise throw an exception.  */
+
+static struct value *
+enum_constant_from_type (struct type *type, const char *name)
+{
+  int i;
+  int name_len = strlen (name);
+
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_ENUM
+	      && TYPE_DECLARED_CLASS (type));
+
+  for (i = TYPE_N_BASECLASSES (type); i < TYPE_NFIELDS (type); ++i)
+    {
+      const char *fname = TYPE_FIELD_NAME (type, i);
+      int len;
+
+      if (TYPE_FIELD_LOC_KIND (type, i) != FIELD_LOC_KIND_ENUMVAL
+	  || fname == NULL)
+	continue;
+
+      /* Look for the trailing "::NAME", since enum class constant
+	 names are qualified here.  */
+      len = strlen (fname);
+      if (len + 2 >= name_len
+	  && fname[len - name_len - 2] == ':'
+	  && fname[len - name_len - 1] == ':'
+	  && strcmp (&fname[len - name_len], name) == 0)
+	return value_from_longest (type, TYPE_FIELD_ENUMVAL (type, i));
+    }
+
+  error (_("no constant named \"%s\" in enum \"%s\""),
+	 name, TYPE_TAG_NAME (type));
+}
+
 /* C++: Given an aggregate type CURTYPE, and a member name NAME,
    return the appropriate member (or the address of the member, if
    WANT_ADDRESS).  This function is used to resolve user expressions
@@ -3028,6 +3064,10 @@ value_aggregate_elt (struct type *curtype, const char *name,
     case TYPE_CODE_NAMESPACE:
       return value_namespace_elt (curtype, name, 
 				  want_address, noside);
+
+    case TYPE_CODE_ENUM:
+      return enum_constant_from_type (curtype, name);
+
     default:
       internal_error (__FILE__, __LINE__,
 		      _("non-aggregate type in value_aggregate_elt"));