[gcjx] Patch: FYI: fix enum switch handling

Tom Tromey tromey@redhat.com
Fri Dec 2 18:36:00 GMT 2005


I'm checking this in on the gcjx branch.

When a switch expression has enum type, the case expressions must be
identifiers which are looked up as enum constants in the enum type.
We weren't properly implementing this.  This patch is somewhat hacky
in that it violates an abstraction in the model; but it seemed to me
that some other fix (e.g., adding a new model element for the sole
identifier case) would be just as hacky in a different way.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	* model/memberref.cc (resolve_as_enum_constant): New methods.
	(resolve): Use it.
	* model/memberref.hh
	(model_memberref_forward::resolve_as_enum_constant): Declare.
	(model_memberref_base::resolve_as_enum_constant): Declare.
	* model/switch.hh (model_switch_block::resolve_as_enum_constant):
	Declare.
	* model/switch.cc (resolve_as_enum_constant): New method.

Index: model/memberref.cc
===================================================================
--- model/memberref.cc	(revision 107604)
+++ model/memberref.cc	(working copy)
@@ -22,6 +22,28 @@
 #include "typedefs.hh"
 
 void
+model_memberref_base::resolve_as_enum_constant (resolution_scope *scope,
+						model_class *klass,
+						const std::string &field_name)
+{
+  std::set<model_field *> result;
+  klass->find_members (field_name, result, scope->get_current_class (), NULL);
+  if (result.empty ())
+    throw error ("no %<enum%> constant named %1 in %2")
+      % field_name % klass;
+  else if (result.size () > 1)
+    throw error ("ambiguous reference to %<enum%> constant named %1 in %2")
+      % field_name % klass;
+
+  ref_field_ref rf = new model_field_ref (get_location ());
+  rf->set_field (field_name);
+  rf->set_field (*(result.begin ()));
+
+  real = rf;
+  set_type (klass);
+}
+
+void
 model_memberref_forward::resolve (resolution_scope *scope)
 {
   if (is_call)
@@ -56,6 +78,17 @@
 }
 
 void
+model_memberref_forward::resolve_as_enum_constant (resolution_scope *scope,
+						   model_enum *enum_class)
+{
+  if (is_call || ids.size () != 1)
+    throw error ("%<enum%> case expression must be an identifier");
+  assert (type_parameters.empty ());
+  model_memberref_base::resolve_as_enum_constant (scope, enum_class,
+						  ids.back ());
+}
+
+void
 model_memberref_enum::resolve (resolution_scope *scope)
 {
   base_type->resolve (scope);
@@ -63,21 +96,7 @@
     throw error ("type %1 not an %<enum%>") % base_type->type ();
 
   model_class *klass = assert_cast<model_class *> (base_type->type ());
-  std::set<model_field *> result;
-  klass->find_members (field_name, result, scope->get_current_class (), NULL);
-  if (result.empty ())
-    throw error ("no %<enum%> constant named %1 in %2")
-      % field_name % klass;
-  else if (result.size () > 1)
-    throw error ("ambiguous reference to %<enum%> constant named %1 in %2")
-      % field_name % klass;
-
-  ref_field_ref rf = new model_field_ref (get_location ());
-  rf->set_field (field_name);
-  rf->set_field (*(result.begin ()));
-
-  real = rf;
-  set_type (klass);
+  resolve_as_enum_constant (scope, klass, field_name);
 }
 
 void
Index: model/memberref.hh
===================================================================
--- model/memberref.hh	(revision 107604)
+++ model/memberref.hh	(working copy)
@@ -22,6 +22,8 @@
 #ifndef GCJX_MODEL_MEMBERREF_HH
 #define GCJX_MODEL_MEMBERREF_HH
 
+class model_enum;
+
 /// The base class for all kinds of deferred member references.
 class model_memberref_base : public model_expression
 {
@@ -35,6 +37,10 @@
   {
   }
 
+  /// This is a helper method used by the subclasses.
+  void resolve_as_enum_constant (resolution_scope *, model_class *,
+				 const std::string &);
+
 public:
 
   // This is sort of ugly, see unary.cc to see its use.
@@ -120,6 +126,11 @@
 
   void resolve (resolution_scope *);
 
+  /// This is like resolve, but is used in the special case of an
+  /// identifier that is in a 'case' of a switch whose expression has
+  /// enum type.
+  void resolve_as_enum_constant (resolution_scope *, model_enum *);
+
   void visit (visitor *v)
   {
     real->visit (v);
Index: model/switch.cc
===================================================================
--- model/switch.cc	(revision 107858)
+++ model/switch.cc	(working copy)
@@ -28,6 +28,24 @@
 }
 
 void
+model_switch_block::resolve_as_enum_constant (resolution_scope *scope,
+					      model_type *sw_type,
+					      model_expression *expr)
+{
+  model_enum *switch_type = assert_cast<model_enum *> (sw_type);
+
+  // An case which is an enum constant must be a single identifier.
+  // It is looked up in the type of the switch expression (though this
+  // is not explicitly documented in the JLS).  FIXME: here we have
+  // unwarranted knowledge of what the parser generates.
+  model_memberref_forward *fwd_expr
+    = dynamic_cast<model_memberref_forward *> (expr);
+  if (fwd_expr == NULL)
+    throw expr->error ("%<enum%> case expression must be identifier");
+  fwd_expr->resolve_as_enum_constant (scope, switch_type);
+}
+
+void
 model_switch_block::resolve (resolution_scope *scope,
 			     model_type *switch_type,
 			     std::map<jint, model_expression *> &seen,
@@ -40,7 +58,10 @@
        i != labels.end ();
        ++i)
     {
-      (*i)->resolve (scope);
+      if (switch_type->enum_p ())
+	resolve_as_enum_constant (scope, switch_type, (*i).get ());
+      else
+	(*i)->resolve (scope);
       if (assignment_conversion (switch_type, (*i).get ()) == NULL)
 	throw error ("case expression not assignable to switch type");
 
Index: model/switch.hh
===================================================================
--- model/switch.hh	(revision 107604)
+++ model/switch.hh	(working copy)
@@ -38,6 +38,9 @@
   // True if we can complete normally.
   watch<bool> normal_completion;
 
+  void resolve_as_enum_constant (resolution_scope *, model_type *,
+				 model_expression *);
+
 public:
 
   model_switch_block (const location &w)



More information about the Java-patches mailing list