Merge pull request #984 from larsclausen/class-constructor-chain

Fix class constructor chaining corner cases

elab_expr.cc

@@ -6634,29 +6634,7 @@ NetExpr* PENewClass::elaborate_expr_constructor_(Design*des, NetScope*scope,
 {
       ivl_assert(*this, ctype);
 
-	// If there is an initializer function, then pass the object
-	// through that function first. Note that the initializer
-	// function has no arguments other than the object itself.
-      if (NetScope*new1_scope = ctype->method_from_name(perm_string::literal("new@"))) {
-	    NetFuncDef*def1 = new1_scope->func_def();
-	    ivl_assert(*this, def1);
-	    ivl_assert(*this, def1->port_count()==1);
-	    vector<NetExpr*> parms1 (1);
-	    parms1[0] = obj;
-
-	      // The return value of the initializer is the "this"
-	      // variable, instead of the "new&" scope name.
-	    NetNet*res1 = new1_scope->find_signal(perm_string::literal(THIS_TOKEN));
-	    ivl_assert(*this, res1);
-
-	    NetESignal*eres = new NetESignal(res1);
-	    NetEUFunc*tmp = new NetEUFunc(scope, new1_scope, eres, parms1, true);
-	    tmp->set_line(*this);
-	    obj = tmp;
-      }
-
-
-      NetScope*new_scope = ctype->method_from_name(perm_string::literal("new"));
+      NetScope *new_scope = ctype->get_constructor();
       if (new_scope == 0) {
 	      // No constructor.
 	    if (parms_.size() > 0) {

elaborate.cc

@@ -3289,25 +3289,9 @@ NetProc* PChainConstructor::elaborate(Design*des, NetScope*scope) const
 	// going to pass this to the chained constructor.
       NetNet*var_this = scope->find_signal(perm_string::literal(THIS_TOKEN));
 
-	// If super.new is an implicit constructor, then there are no
-	// arguments (other than "this" to worry about, so make a
-	// NetEUFunc and there we go.
-      if (NetScope*new_scope = class_super->method_from_name(perm_string::literal("new@"))) {
-	    NetESignal*eres = new NetESignal(var_this);
-	    vector<NetExpr*> parms(1);
-	    parms[0] = eres;
-	    NetEUFunc*tmp = new NetEUFunc(scope, new_scope, eres, parms, true);
-	    tmp->set_line(*this);
-
-	    NetAssign_*lval_this = new NetAssign_(var_this);
-	    NetAssign*stmt = new NetAssign(lval_this, tmp);
-	    stmt->set_line(*this);
-	    return stmt;
-      }
-
 	// If super.new(...) is a user defined constructor, then call
 	// it. This is a bit more complicated because there may be arguments.
-      if (NetScope*new_scope = class_super->method_from_name(perm_string::literal("new"))) {
+      if (NetScope*new_scope = class_super->get_constructor()) {
 
 	    int missing_parms = 0;
 	    NetFuncDef*def = new_scope->func_def();

ivtest/ivltests/sv_chained_constructor1.v

@@ -0,0 +1,40 @@
+// Check that the constructor of a base class gets called exactly once when
+// mixing implicit and explicit constructors.
+
+module test;
+
+  bit failed = 1'b0;
+
+  `define check(val, exp) do begin \
+    if ((val) !== (exp)) begin \
+      $display("FAILED(%0d): `%s`, expected `%0d`, got `%0d`.", `__LINE__, \
+               `"val`", (exp), (val)); \
+      failed = 1'b1; \
+    end \
+  end while (0)
+
+  int c_new_calls = 0;
+
+  class C;
+    function new;
+      c_new_calls++;
+    endfunction
+  endclass
+
+  class D extends C;
+    int x = 10;
+  endclass
+
+  initial begin
+    D d;
+    d = new;
+
+    `check(c_new_calls, 1);
+    `check(d.x, 10);
+
+    if (!failed) begin
+      $display("PASSED");
+    end
+  end
+
+endmodule

ivtest/ivltests/sv_chained_constructor2.v

@@ -0,0 +1,48 @@
+// Check that the constructor of a base class gets called exactly once when
+// mixing implicit and explicit constructors.
+
+module test;
+
+  bit failed = 1'b0;
+
+  `define check(val, exp) do begin \
+    if ((val) !== (exp)) begin \
+      $display("FAILED(%0d): `%s`, expected `%0d`, got `%0d`.", `__LINE__, \
+               `"val`", (exp), (val)); \
+      failed = 1'b1; \
+    end \
+  end while (0)
+
+  int d_new_calls = 0;
+
+  class C;
+    int x = 10;
+  endclass
+
+  class D extends C;
+    function new;
+      d_new_calls++;
+    endfunction
+  endclass
+
+  class E extends D;
+    int y;
+    function new;
+      y = 20;
+    endfunction
+  endclass
+
+  initial begin
+    E e;
+    e = new;
+
+    `check(d_new_calls, 1);
+    `check(e.x, 10);
+    `check(e.y, 20);
+
+    if (!failed) begin
+      $display("PASSED");
+    end
+  end
+
+endmodule

ivtest/ivltests/sv_chained_constructor3.v

@@ -0,0 +1,22 @@
+// Check that forwarding base class constructor arguments through the `extends`
+// works when the derived class has no constructor.
+
+module test;
+
+  class C;
+    function new(int a);
+      if (a == 10) begin
+        $display("PASSED");
+      end else begin
+        $display("FAILED");
+      end
+    endfunction
+  endclass
+
+  // D has no constructor
+  class D extends C(10);
+  endclass
+
+  D d = new;
+
+endmodule

ivtest/ivltests/sv_chained_constructor4.v

@@ -0,0 +1,23 @@
+// Check that forwarding base class constructor arguments through the `extends`
+// works when the derived class has an implicit constructor.
+
+module test;
+
+  class C;
+    function new(int a);
+      if (a == 10) begin
+        $display("PASSED");
+      end else begin
+        $display("FAILED");
+      end
+    endfunction
+  endclass
+
+  // D has an implicit constructor
+  class D extends C(10);
+    int y;
+  endclass
+
+  D d = new;
+
+endmodule

ivtest/ivltests/sv_chained_constructor5.v

@@ -0,0 +1,26 @@
+// Check that forwarding base class constructor arguments through the `extends`
+// works when the derived class has an explicit constructor.
+
+module test;
+
+  class C;
+    function new(int a);
+      if (a == 10) begin
+        $display("PASSED");
+      end else begin
+        $display("FAILED");
+      end
+    endfunction
+  endclass
+
+  // D has an explicit constructor
+  class D extends C(10);
+    int x;
+    function new;
+      x = 10;
+    endfunction
+  endclass
+
+  D d = new;
+
+endmodule

ivtest/regress-vvp.list

@@ -61,6 +61,11 @@ sv_array_assign_fail2	vvp_tests/sv_array_assign_fail2.json
 sv_array_cassign6		vvp_tests/sv_array_cassign6.json
 sv_array_cassign7		vvp_tests/sv_array_cassign7.json
 sv_automatic_2state		vvp_tests/sv_automatic_2state.json
+sv_chained_constructor1		vvp_tests/sv_chained_constructor1.json
+sv_chained_constructor2		vvp_tests/sv_chained_constructor2.json
+sv_chained_constructor3		vvp_tests/sv_chained_constructor3.json
+sv_chained_constructor4		vvp_tests/sv_chained_constructor4.json
+sv_chained_constructor5		vvp_tests/sv_chained_constructor5.json
 sv_const1			vvp_tests/sv_const1.json
 sv_const2			vvp_tests/sv_const2.json
 sv_const3			vvp_tests/sv_const3.json

ivtest/vvp_tests/sv_chained_constructor1.json

@@ -0,0 +1,5 @@
+{
+    "type"          : "normal",
+    "source"        : "sv_chained_constructor1.v",
+    "iverilog-args" : [ "-g2005-sv" ]
+}

ivtest/vvp_tests/sv_chained_constructor2.json

@@ -0,0 +1,5 @@
+{
+    "type"          : "normal",
+    "source"        : "sv_chained_constructor2.v",
+    "iverilog-args" : [ "-g2005-sv" ]
+}

ivtest/vvp_tests/sv_chained_constructor3.json

@@ -0,0 +1,5 @@
+{
+    "type"          : "normal",
+    "source"        : "sv_chained_constructor3.v",
+    "iverilog-args" : [ "-g2005-sv" ]
+}

ivtest/vvp_tests/sv_chained_constructor4.json

@@ -0,0 +1,5 @@
+{
+    "type"          : "normal",
+    "source"        : "sv_chained_constructor4.v",
+    "iverilog-args" : [ "-g2005-sv" ]
+}

ivtest/vvp_tests/sv_chained_constructor5.json

@@ -0,0 +1,5 @@
+{
+    "type"          : "normal",
+    "source"        : "sv_chained_constructor5.v",
+    "iverilog-args" : [ "-g2005-sv" ]
+}

netclass.cc

@@ -172,6 +172,22 @@ NetScope*netclass_t::method_from_name(perm_string name) const
 
 }
 
+NetScope* netclass_t::get_constructor() const
+{
+      auto task = class_scope_->child(hname_t(perm_string::literal("new")));
+      if (task)
+	    return task;
+
+      task = class_scope_->child(hname_t(perm_string::literal("new@")));
+      if (task)
+	    return task;
+
+      if (super_)
+	    return super_->get_constructor();
+
+      return nullptr;
+}
+
 NetNet* netclass_t::find_static_property(perm_string name) const
 {
       NetNet *net = class_scope_->find_signal(name);

netclass.h

@@ -94,6 +94,10 @@ class netclass_t : public ivl_type_s {
 	// The task method scopes from the method name.
       NetScope*method_from_name(perm_string mname) const;
 
+	// Returns the constructor task method of the class. Might be nullptr if
+	// there is nothing to do in the constructor.
+      NetScope* get_constructor() const;
+
 	// Find the elaborated signal (NetNet) for a static
 	// property. Search by name. The signal is created by the
 	// elaborate_sig pass.

pform_pclass.cc

@@ -147,8 +147,11 @@ void pform_end_class_declaration(const struct vlltype&loc)
       ivl_assert(loc, pform_cur_class);
 
 	// If there were initializer statements, then collect them
-	// into an implicit constructor function.
-      if (! pform_cur_class->type->initialize.empty()) {
+	// into an implicit constructor function. Also make sure that an
+	// explicit constructor exists if base class constructor arguments are
+	// specified, so that the base class constructor will be called.
+      if (!pform_cur_class->type->initialize.empty() ||
+          !pform_cur_class->type->base_args.empty()) {
 	    PFunction*func = pform_push_function_scope(loc, "new@", LexicalScope::AUTOMATIC);
 	    func->set_ports(0);
 	    pform_set_constructor_return(func);