Consolidate class property handling

There are currently two mechanisms for handling class properties. One that
is used when a class property is accessed through an object and other when
a class property is used freestanding in a class method.

Both are very similar, but there are some small differences. E.g. one
supports arrays, the other supports nested properties.

```
class B;
  int x;
endclass

class C;
  B b;
  B ba[2];
  task t;
    ba[0] = new; // Does work
    this.ba[0] = new; // Does not work
    b.x = 10; // Does not work
    this.b.x = 10; // Does work
  endtask
```

There is another problem where free standing properties take precedence
over local variables. E.g.

```
class C;
  int x = 1;
  task t();
    int x = 2;
    $display(x); // Should print 2, will print 1
  endtask
endclass
```

The class property elaboration also ignores the package scope of the
identifier resulting in access to a class property being elaborated if
there is a property of the same name as the scoped identifier. E.g.

```
package P;
  int x = 2;
endpackage

class C;
  int x = 1;
  task t;
    $display(P::x); // Should print 2, will print 1
  endtask
endclass
```

Consolidate the two implementation to use the same code path. This is
mainly done by letting the symbol search return a result for free standing
properties as if the property had been specified on the `this` object. I.e.
`prop` and `this.prop` will return the same result from the symbol search.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

PExpr.h

@@ -412,7 +412,6 @@ class PEIdent : public PExpr {
 				      bool is_force, bool is_cassign,
 				      NetNet *reg, ivl_type_t data_type,
 				      pform_name_t tail_path) const;
-      NetAssign_*elaborate_lval_method_class_member_(Design*, NetScope*) const;
       NetAssign_*elaborate_lval_net_word_(Design*, NetScope*, NetNet*,
 					  bool need_const_idx) const;
       bool elaborate_lval_net_bit_(Design*, NetScope*, NetAssign_*,
@@ -511,11 +510,6 @@ class PEIdent : public PExpr {
 					   NetScope*found,
 					   bool need_const) const;
 
-      NetExpr*elaborate_expr_class_member_(Design*des,
-					   NetScope*scope,
-					   unsigned expr_wid,
-					   unsigned flags) const;
-
       NetExpr *elaborate_expr_class_field_(Design*des, NetScope*scope,
 					   const symbol_search_results &sr,
 					   unsigned expr_wid,

elab_expr.cc

@@ -2601,7 +2601,37 @@ NetExpr* PEIdent::elaborate_expr_class_field_(Design*des, NetScope*scope,
 						    prop_name);
       }
 
-      NetEProperty*tmp = new NetEProperty(sr.net, pidx);
+      NetExpr *canon_index = nullptr;
+      ivl_type_t tmp_type = class_type->get_prop_type(pidx);
+      if (const netuarray_t *tmp_ua = dynamic_cast<const netuarray_t*>(tmp_type)) {
+	    const std::vector<netrange_t> &dims = tmp_ua->static_dimensions();
+
+	    if (debug_elaborate) {
+		  cerr << get_fileline() << ": PEIdent::elaborate_expr_class_member_: "
+		       << "Property " << class_type->get_prop_name(pidx)
+		       << " has " << dims.size() << " dimensions, "
+		       << " got " << comp.index.size() << " indices." << endl;
+	    }
+
+	    if (dims.size() != comp.index.size()) {
+		  cerr << get_fileline() << ": error: "
+		       << "Got " << comp.index.size() << " indices, "
+		       << "expecting " << dims.size()
+		       << " to index the property " << class_type->get_prop_name(pidx) << "." << endl;
+		  des->errors++;
+	    } else {
+		  canon_index = make_canonical_index(des, scope, this,
+						     comp.index, tmp_ua, false);
+	    }
+      }
+
+      if (debug_elaborate && canon_index) {
+	    cerr << get_fileline() << ": PEIdent::elaborate_expr_class_member_: "
+		 << "Property " << class_type->get_prop_name(pidx)
+		 << " canonical index: " << *canon_index << endl;
+      }
+
+      NetEProperty *tmp = new NetEProperty(sr.net, pidx, canon_index);
       tmp->set_line(*this);
       return tmp;
 }
@@ -4166,14 +4196,33 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode)
 	    return expr_width_;
       }
 
-	// Look for a class property.
+      if (sr.net) {
-      if (gn_system_verilog() && sr.cls_val) {
+	      // Similarly, if this net is an object, the path tail may
-	    expr_type_   = sr.cls_val->base_type();
+	      // be a class property.
-	    expr_width_  = sr.cls_val->packed_width();
+	    const netclass_t *class_type = dynamic_cast<const netclass_t*>(sr.type);
+	    if (class_type && !sr.path_tail.empty()) {
+		  perm_string pname = peek_tail_name(sr.path_tail);
+		  ivl_type_t par_type;
+		  const NetExpr *par = class_type->get_parameter(des, pname, par_type);
+		  if (par)
+			return test_width_parameter_(par, mode);
+
+		  int pidx = class_type->property_idx_from_name(pname);
+		  if (pidx >= 0) {
+			const name_component_t member_comp = sr.path_tail.front();
+			ivl_type_t ptype = class_type->get_prop_type(pidx);
+			if (!member_comp.index.empty()) {
+			      const netuarray_t*tmp_ua = dynamic_cast<const netuarray_t*>(ptype);
+			      if (tmp_ua) ptype = tmp_ua->element_type();
+			}
+			expr_type_   = ptype->base_type();
+			expr_width_  = ptype->packed_width();
 			min_width_   = expr_width_;
-	    signed_flag_ = sr.cls_val->get_signed();
+			signed_flag_ = ptype->get_signed();
 			return expr_width_;
 		  }
+	    }
+      }
 
       if (use_width != UINT_MAX) {
 	      // We have a bit/part select. Account for any remaining dimensions
@@ -4221,27 +4270,6 @@ unsigned PEIdent::test_width(Design*des, NetScope*scope, width_mode_t&mode)
 		  }
 	    }
 
-	      // Similarly, if this net is an object, the path tail may
-	      // be a class property.
-	    const netclass_t *class_type = dynamic_cast<const netclass_t*>(sr.type);
-	    if (class_type && !sr.path_tail.empty()) {
-		  perm_string pname = peek_tail_name(sr.path_tail);
-		  ivl_type_t par_type;
-		  const NetExpr *par = class_type->get_parameter(des, pname, par_type);
-		  if (par)
-			return test_width_parameter_(par, mode);
-
-		  int pidx = class_type->property_idx_from_name(pname);
-		  if (pidx >= 0) {
-			ivl_type_t ptype = class_type->get_prop_type(pidx);
-			expr_type_   = ptype->base_type();
-			expr_width_  = ptype->packed_width();
-			min_width_   = expr_width_;
-			signed_flag_ = ptype->get_signed();
-			return expr_width_;
-		  }
-	    }
-
 	    size_t use_depth = name_tail.index.size();
 	      // Account for unpacked dimensions by assuming that the
 	      // unpacked dimensions are consumed first, so subtract
@@ -4321,10 +4349,6 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope,
 	    ivl_assert(*this, use_scope);
       }
 
-      if (NetExpr* tmp = elaborate_expr_class_member_(des, scope, 0, flags)) {
-	    return tmp;
-      }
-
       symbol_search_results sr;
       symbol_search(this, des, use_scope, path_, &sr);
 
@@ -4454,105 +4478,6 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope,
       return tmp;
 }
 
-/*
- * Guess that the path_ is the name of a member of a containing class,
- * and see how that works. If it turns out that the current scope is
- * not a method, or the name is not in the parent class, then
- * fail. Otherwise, return a NetEProperty.
- */
-NetExpr* PEIdent::elaborate_expr_class_member_(Design*des, NetScope*scope,
-					       unsigned, unsigned) const
-{
-      if (!gn_system_verilog())
-	    return 0;
-      if (scope->parent() == 0)
-	    return 0;
-      if (path_.size() != 1)
-	    return 0;
-
-      const netclass_t*class_type = find_class_containing_scope(*this, scope);
-      if (class_type == 0)
-	    return 0;
-
-      const name_component_t&name_comp = path_.back();
-
-      perm_string member_name = name_comp.name;
-      int pidx = class_type->property_idx_from_name(member_name);
-      if (pidx < 0)
-	    return 0;
-
-      NetScope*scope_method = find_method_containing_scope(*this, scope);
-      ivl_assert(*this, scope_method);
-
-      NetNet*this_net = scope_method->find_signal(perm_string::literal(THIS_TOKEN));
-      if (this_net == 0) {
-	    cerr << get_fileline() << ": internal error: "
-		 << "Unable to find 'this' port of " << scope_path(scope_method)
-		 << "." << endl;
-	    return 0;
-      }
-
-      if (debug_elaborate) {
-	    cerr << get_fileline() << ": PEIdent::elaborate_expr_class_member: "
-		 << "Found member " << member_name
-		 << " is a member of class " << class_type->get_name()
-		 << ", context scope=" << scope_path(scope)
-		 << ", type=" << *class_type->get_prop_type(pidx)
-		 << ", so making a NetEProperty." << endl;
-      }
-
-      property_qualifier_t qual = class_type->get_prop_qual(pidx);
-      if (qual.test_local() && ! class_type->test_scope_is_method(scope)) {
-	    cerr << get_fileline() << ": error: "
-		 << "Local property " << class_type->get_prop_name(pidx)
-		 << " is not accessible in this context."
-		 << " (scope=" << scope_path(scope) << ")" << endl;
-	    des->errors += 1;
-      }
-
-      if (qual.test_static()) {
-	    return class_static_property_expression(this, class_type, member_name);
-      }
-
-      NetExpr*canon_index = 0;
-      ivl_type_t tmp_type = class_type->get_prop_type(pidx);
-      if (const netuarray_t*tmp_ua = dynamic_cast<const netuarray_t*>(tmp_type)) {
-
-	    const std::vector<netrange_t>&dims = tmp_ua->static_dimensions();
-
-	    if (debug_elaborate) {
-		  cerr << get_fileline() << ": PEIdent::elaborate_expr_class_member_: "
-		       << "Property " << class_type->get_prop_name(pidx)
-		       << " has " << dims.size() << " dimensions, "
-		       << " got " << name_comp.index.size() << " indices." << endl;
-	    }
-
-	    if (dims.size() != name_comp.index.size()) {
-		  cerr << get_fileline() << ": error: "
-		       << "Got " << name_comp.index.size() << " indices, "
-		       << "expecting " << dims.size()
-		       << " to index the property " << class_type->get_prop_name(pidx) << "." << endl;
-		  des->errors += 1;
-
-	    } else {
-
-		  canon_index = make_canonical_index(des, scope, this,
-						     name_comp.index, tmp_ua, false);
-	    }
-      }
-
-      if (debug_elaborate && canon_index) {
-	    cerr << get_fileline() << ": PEIdent::elaborate_expr_class_member_: "
-		 << "Property " << class_type->get_prop_name(pidx)
-		 << " canonical index: " << *canon_index << endl;
-      }
-
-      NetEProperty*tmp = new NetEProperty(this_net, pidx, canon_index);
-      tmp->set_line(*this);
-      return tmp;
-}
-
-
 /*
  * Elaborate an identifier in an expression. The identifier can be a
  * parameter name, a signal name or a memory name. It can also be a
@@ -4587,17 +4512,6 @@ NetExpr* PEIdent::elaborate_expr_(Design*des, NetScope*scope,
 		 << endl;
       }
 
-	// Special case: Detect the special situation that this name
-	// is the name of a variable in the class, and this is a class
-	// method. We sense that this might be the case by noting that
-	// the parent scope of where we are working is a
-	// NetScope::CLASS, the path_ is a single component, and the
-	// name is a property of the class. If that turns out to be
-	// the case, then handle this specially.
-      if (NetExpr*tmp = elaborate_expr_class_member_(des, scope, expr_wid, flags)) {
-	    return tmp;
-      }
-
       if (path_.size() > 1) {
             if (NEED_CONST & flags) {
                   cerr << get_fileline() << ": error: A hierarchical reference"

elab_lval.cc

@@ -165,11 +165,6 @@ NetAssign_* PEIdent::elaborate_lval(Design*des,
 		 << "Elaborate l-value ident expression: " << *this << endl;
       }
 
-	/* Try to detect the special case that we are in a method and
-	   the identifier is a member of the class. */
-      if (NetAssign_*tmp = elaborate_lval_method_class_member_(des, scope))
-	    return tmp;
-
 	/* Normally find the name in the passed scope. But if this is
 	   imported from a package, then located the variable from the
 	   package scope. */
@@ -364,132 +359,6 @@ NetAssign_*PEIdent::elaborate_lval_var_(Design *des, NetScope *scope,
       return lv;
 }
 
-NetAssign_* PEIdent::elaborate_lval_method_class_member_(Design*des,
-							 NetScope*scope) const
-{
-      if (!gn_system_verilog())
-	    return 0;
-      if (scope->parent() == 0 || scope->type() == NetScope::CLASS)
-	    return 0;
-      if (path_.size() != 1)
-	    return 0;
-
-      const netclass_t*class_type = find_class_containing_scope(*this, scope);
-      if (class_type == 0)
-	    return 0;
-
-      const name_component_t&name_comp = path_.back();
-
-      perm_string member_name = name_comp.name;
-      int pidx = class_type->property_idx_from_name(member_name);
-      if (pidx < 0)
-	    return 0;
-
-      property_qualifier_t qual = class_type->get_prop_qual(pidx);
-
-      if (qual.test_static()) {
-	    NetNet *sig = class_type->find_static_property(member_name);
-	    return elaborate_lval_var_(des, scope, false, false, sig,
-				       class_type, {});
-      }
-
-      NetScope*scope_method = find_method_containing_scope(*this, scope);
-      ivl_assert(*this, scope_method);
-
-      NetNet*this_net = scope_method->find_signal(perm_string::literal(THIS_TOKEN));
-      if (this_net == 0) {
-	    cerr << get_fileline() << ": internal error: "
-		 << "Unable to find 'this' port of " << scope_path(scope_method)
-		 << "." << endl;
-	    return 0;
-      }
-
-      if (debug_elaborate) {
-	    cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
-		 << "Ident " << member_name
-		 << " is a property of class " << class_type->get_name() << endl;
-      }
-
-      NetExpr*canon_index = 0;
-      if (! name_comp.index.empty()) {
-	    ivl_type_t property_type = class_type->get_prop_type(pidx);
-
-	    if (const netsarray_t* stype = dynamic_cast<const netsarray_t*> (property_type)) {
-		  canon_index = make_canonical_index(des, scope, this,
-						     name_comp.index, stype, false);
-
-	    } else {
-		  cerr << get_fileline() << ": error: "
-		       << "Index expressions don't apply to this type of property." << endl;
-		  des->errors += 1;
-	    }
-      }
-
-	// Detect assignment to constant properties. Note that the
-	// initializer constructor MAY assign to constant properties,
-	// as this is how the property gets its value.
-      if (qual.test_const()) {
-	    if (class_type->get_prop_initialized(pidx)) {
-		  cerr << get_fileline() << ": error: "
-		       << "Property " << class_type->get_prop_name(pidx)
-		       << " is constant in this method."
-		       << " (scope=" << scope_path(scope) << ")" << endl;
-		  des->errors += 1;
-
-	    } else if (scope->basename()!="new" && scope->basename()!="new@") {
-		  cerr << get_fileline() << ": error: "
-		       << "Property " << class_type->get_prop_name(pidx)
-		       << " is constant in this method."
-		       << " (scope=" << scope_path(scope) << ")" << endl;
-		  des->errors += 1;
-
-	    } else {
-
-		    // Mark this property as initialized. This is used
-		    // to know that we have initialized the constant
-		    // object so the next assignment will be marked as
-		    // illegal.
-		  class_type->set_prop_initialized(pidx);
-
-		  if (debug_elaborate) {
-			cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
-			     << "Found initializers for property " << class_type->get_prop_name(pidx) << endl;
-		  }
-	    }
-      }
-
-      ivl_type_t tmp_type = class_type->get_prop_type(pidx);
-      if (const netuarray_t*tmp_ua = dynamic_cast<const netuarray_t*>(tmp_type)) {
-
-	    const std::vector<netrange_t>&dims = tmp_ua->static_dimensions();
-
-	    if (debug_elaborate) {
-		  cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
-		       << "Property " << class_type->get_prop_name(pidx)
-		       << " has " << dims.size() << " dimensions, "
-		       << " got " << name_comp.index.size() << " indices." << endl;
-		  if (canon_index) {
-			cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
-			     << "Canonical index is:" << *canon_index << endl;
-		  };
-	    }
-
-	    if (dims.size() != name_comp.index.size()) {
-		  cerr << get_fileline() << ": error: "
-		       << "Got " << name_comp.index.size() << " indices, "
-		       << "expecting " << dims.size()
-		       << " to index the property " << class_type->get_prop_name(pidx) << "." << endl;
-		  des->errors += 1;
-	    }
-      }
-
-      NetAssign_*this_lval = new NetAssign_(this_net);
-      this_lval->set_property(member_name, pidx);
-      if (canon_index) this_lval->set_word(canon_index);
-
-      return this_lval;
-}
-
 NetAssign_* PEIdent::elaborate_lval_net_word_(Design*des,
 					      NetScope*scope,
 					      NetNet*reg,
@@ -1151,10 +1020,30 @@ NetAssign_* PEIdent::elaborate_lval_net_class_member_(Design*des, NetScope*scope
 		  return lv;
 
 	    } else if (qual.test_const()) {
+		 if (class_type->get_prop_initialized(pidx)) {
 		       cerr << get_fileline() << ": error: "
 			    << "Property " << class_type->get_prop_name(pidx)
-		       << " is constant in this context." << endl;
+			    << " is constant in this method."
-		  des->errors += 1;
+			    << " (scope=" << scope_path(scope) << ")" << endl;
+		       des->errors++;
+		 } else if (scope->basename() != "new" && scope->basename() != "new@") {
+		       cerr << get_fileline() << ": error: "
+			    << "Property " << class_type->get_prop_name(pidx)
+			    << " is constant in this method."
+			    << " (scope=" << scope_path(scope) << ")" << endl;
+		       des->errors++;
+		 } else {
+			 // Mark this property as initialized. This is used
+			 // to know that we have initialized the constant
+			 // object so the next assignment will be marked as
+			 // illegal.
+		       class_type->set_prop_initialized(pidx);
+
+		       if (debug_elaborate) {
+			     cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
+				  << "Found initializers for property " << class_type->get_prop_name(pidx) << endl;
+		       }
+		 }
 	    }
 
 	    lv = lv? new NetAssign_(lv) : new NetAssign_(sig);
@@ -1171,6 +1060,44 @@ NetAssign_* PEIdent::elaborate_lval_net_class_member_(Design*des, NetScope*scope
 			des->errors += 1;
 		  }
 	    }
+	    NetExpr *canon_index = nullptr;
+	    if (!member_cur.index.empty()) {
+		  if (const netsarray_t *stype = dynamic_cast<const netsarray_t*>(ptype)) {
+			canon_index = make_canonical_index(des, scope, this,
+							   member_cur.index, stype, false);
+
+		  } else {
+			cerr << get_fileline() << ": error: "
+			     << "Index expressions don't apply to this type of property." << endl;
+			des->errors++;
+		  }
+	    }
+
+	    if (const netuarray_t *tmp_ua = dynamic_cast<const netuarray_t*>(ptype)) {
+		  const std::vector<netrange_t> &dims = tmp_ua->static_dimensions();
+
+		  if (debug_elaborate) {
+			cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
+			     << "Property " << class_type->get_prop_name(pidx)
+			     << " has " << dims.size() << " dimensions, "
+			     << " got " << member_cur.index.size() << " indices." << endl;
+			if (canon_index) {
+			      cerr << get_fileline() << ": PEIdent::elaborate_lval_method_class_member_: "
+				   << "Canonical index is:" << *canon_index << endl;
+			}
+		  }
+
+		  if (dims.size() != member_cur.index.size()) {
+			cerr << get_fileline() << ": error: "
+			     << "Got " << member_cur.index.size() << " indices, "
+			     << "expecting " << dims.size()
+			     << " to index the property " << class_type->get_prop_name(pidx) << "." << endl;
+			des->errors++;
+		  }
+	    }
+
+	    if (canon_index)
+		  lv->set_word(canon_index);
 
 	      // If the current member is a class object, then get the
 	      // type. We may wind up iterating, and need the proper

netmisc.h

@@ -46,7 +46,6 @@ struct symbol_search_results {
       inline symbol_search_results() {
 	    scope = 0;
 	    net = 0;
-	    cls_val = 0;
 	    par_val = 0;
 	    type = 0;
 	    eve = 0;
@@ -55,7 +54,6 @@ struct symbol_search_results {
       inline bool is_scope() const {
 	    if (net) return false;
 	    if (eve) return false;
-	    if (cls_val) return false;
 	    if (par_val) return false;
 	    if (scope) return true;
 	    return false;
@@ -64,7 +62,6 @@ struct symbol_search_results {
       inline bool is_found() const {
 	    if (net) return true;
 	    if (eve) return true;
-	    if (cls_val) return true;
 	    if (par_val) return true;
 	    if (scope) return true;
 	    return false;
@@ -75,8 +72,6 @@ struct symbol_search_results {
       NetScope*scope;
 	// If this was a net, the signal itself.
       NetNet*net;
-	// For a class property we only have type information.
-      ivl_type_t cls_val;
 	// If this was a parameter, the value expression and the
 	// optional value dimensions.
       const NetExpr*par_val;

symbol_search.cc

@@ -187,16 +187,21 @@ bool symbol_search(const LineInfo*li, Design*des, NetScope*scope,
 
 		    // Static items are just normal signals and are found above.
 		  if (scope->type() == NetScope::CLASS) {
-			netclass_t*clsnet = scope->find_class(des, scope->basename());
+			const netclass_t *clsnet = scope->class_def();
 			int pidx = clsnet->property_idx_from_name(path_tail.name);
 			if (pidx >= 0) {
-			      ivl_type_t prop_type = clsnet->get_prop_type(pidx);
+			      // This is a class property being accessed in a
-			      const netuarray_t*tmp_ua = dynamic_cast<const netuarray_t*>(prop_type);
+			      // class method. Return `this` for the net and the
-			      if (tmp_ua) prop_type = tmp_ua->element_type();
+			      // property name for the path tail.
-			      path.push_back(path_tail);
+			      NetScope *scope_method = find_method_containing_scope(*li, start_scope);
+			      ivl_assert(*li, scope_method);
+			      res->net = scope_method->find_signal(perm_string::literal(THIS_TOKEN));
+			      ivl_assert(*li, res->net);
 			      res->scope = scope;
-			      res->cls_val = prop_type;
+			      ivl_assert(*li, path.empty());
-			      res->path_head = path;
+			      res->path_head.push_back(name_component_t(perm_string::literal(THIS_TOKEN)));
+			      res->path_tail.push_front(path_tail);
+			      res->type = clsnet;
 			      return true;
 			}
 		  }