Merge pull request #859 from larsclausen/func-empty-arg

Improvements for calling functions with empty arguments

PExpr.cc

@@ -286,6 +286,7 @@ PECallFunction::~PECallFunction()
 void PECallFunction::declare_implicit_nets(LexicalScope*scope, NetNet::Type type)
 {
       for (unsigned idx = 0 ; idx < parms_.size() ; idx += 1) {
+	    if (parms_[idx])
 		  parms_[idx]->declare_implicit_nets(scope, type);
       }
 }
@@ -294,7 +295,8 @@ bool PECallFunction::has_aa_term(Design*des, NetScope*scope) const
 {
       bool flag = false;
       for (unsigned idx = 0 ; idx < parms_.size() ; idx += 1) {
-	    flag = parms_[idx]->has_aa_term(des, scope) || flag;
+	    if (parms_[idx])
+		  flag |= parms_[idx]->has_aa_term(des, scope);
       }
       return flag;
 }

elab_expr.cc

@@ -1816,17 +1816,7 @@ NetExpr* PECallFunction::elaborate_sfunc_(Design*des, NetScope*scope,
 	    return cast_to_width_(sub, expr_wid);
       }
 
-	/* How many parameters are there? The Verilog language allows
-	   empty parameters in certain contexts, so the parser will
-	   allow things like func(1,,3). It will also cause func() to
-	   be interpreted as a single empty parameter.
-
-	   Functions cannot really take empty parameters, but the
-	   case ``func()'' is the same as no parameters at all. So
-	   catch that special case here. */
       unsigned nparms = parms_.size();
-      if ((nparms == 1) && (parms_[0] == 0))
-	    nparms = 0;
 
       NetESFunc*fun = new NetESFunc(name, expr_type_, expr_width_, nparms, is_overridden_);
       fun->set_line(*this);
@@ -2937,11 +2927,7 @@ unsigned PECallFunction::elaborate_arguments_(Design*des, NetScope*scope,
       const unsigned parm_count = parms.size() - parm_off;
       const unsigned actual_count = parms_.size();
 
-	/* The parser can't distinguish between a function call with
+      if (parm_count == 0 && actual_count == 0)
-	   no arguments and a function call with one empty argument,
-	   and always supplies one empty argument. Handle the no
-	   argument case here. */
-      if ((parm_count == 0) && (actual_count == 1) && (parms_[0] == 0))
 	    return 0;
 
       if (actual_count > parm_count) {
@@ -2984,7 +2970,7 @@ unsigned PECallFunction::elaborate_arguments_(Design*des, NetScope*scope,
 			     << "requires SystemVerilog." << endl;
 			des->errors += 1;
 		  }
-		  parms[pidx] = def->port_defe(pidx);
+		  parms[pidx] = def->port_defe(pidx)->dup_expr();
 
 	    } else {
 		  missing_parms += 1;
@@ -6494,7 +6480,7 @@ NetExpr* PENewClass::elaborate_expr_constructor_(Design*des, NetScope*scope,
 	      // Ran out of explicit arguments. Is there a default
 	      // argument we can use?
 	    if (NetExpr*tmp = def->port_defe(idx)) {
-		  parms[idx] = tmp;
+		  parms[idx] = tmp->dup_expr();
 		  continue;
 	    }
 

elaborate.cc

@@ -3344,7 +3344,7 @@ NetProc* PChainConstructor::elaborate(Design*des, NetScope*scope) const
 		  }
 
 		  if (NetExpr*tmp = def->port_defe(idx)) {
-			parms[idx] = tmp;
+			parms[idx] = tmp->dup_expr();
 			continue;
 		  }
 
@@ -3508,13 +3508,6 @@ NetProc* PCallTask::elaborate_sys(Design*des, NetScope*scope) const
       }
 
       unsigned parm_count = parms_.size();
-
-	/* Catch the special case that the system task has no
-	   parameters. The "()" string will be parsed as a single
-	   empty parameter, when we really mean no parameters at all. */
-      if ((parm_count== 1) && (parms_[0] == 0))
-	    parm_count = 0;
-
       vector<NetExpr*>eparms (parm_count);
 
       perm_string name = peek_tail_name(path_);
@@ -3660,10 +3653,7 @@ NetProc* PCallTask::elaborate_sys_task_method_(Design*des, NetScope*scope,
       NetESignal*sig = new NetESignal(net);
       sig->set_line(*this);
 
-	/* If there is a single NULL argument then ignore it since it is
-	 * left over from the parser and is not needed by the method. */
       unsigned nparms = parms_.size();
-      if ((nparms == 1) && (parms_[0] == 0)) nparms = 0;
 
       vector<NetExpr*>argv (1 + nparms);
       argv[0] = sig;
@@ -4172,9 +4162,9 @@ NetProc* PCallTask::elaborate_build_call_(Design*des, NetScope*scope,
 			        "requires SystemVerilog." << endl;
 			des->errors += 1;
 		  }
-		  rv = def->port_defe(idx);
+		  rv = def->port_defe(idx)->dup_expr();
 		  if (lv_type==IVL_VT_BOOL||lv_type==IVL_VT_LOGIC)
-			rv = pad_to_width(rv->dup_expr(), wid, *this);
+			rv = pad_to_width(rv, wid, *this);
 
 	    } else {
 		  cerr << get_fileline() << ": error: "
@@ -4355,12 +4345,6 @@ bool PCallTask::elaborate_elab(Design*des, NetScope*scope) const
 
       unsigned parm_count = parms_.size();
 
-        /* Catch the special case that the elaboration task has no
-           parameters. The "()" string will be parsed as a single
-           empty parameter, when we really mean no parameters at all. */
-      if ((parm_count== 1) && (parms_[0] == 0))
-            parm_count = 0;
-
       perm_string name = peek_tail_name(path_);
 
       if (!gn_system_verilog()) {

ivtest/ivltests/func_empty_arg1.v

@@ -0,0 +1,33 @@
+// Check that it is possible to call functins with empty arguments if they have
+// default values.
+
+module test;
+
+  bit failed = 1'b0;
+
+  `define check(expr, val) \
+    if (expr !== val) begin \
+      $display("FAILED. %s, expected %d, got %d", `"expr`", val, expr); \
+      failed = 1'b1; \
+    end
+
+  function integer f(integer a = 1, integer b = 2, integer c = 3);
+    return a * 100 + b * 10 + c;
+  endfunction
+
+  initial begin
+    `check(f(4, 5, 6), 456);
+    `check(f(4, 5,  ), 453);
+    `check(f(4,  , 6), 426);
+    `check(f( , 5, 6), 156);
+    `check(f(4,  ,  ), 423);
+    `check(f( , 5,  ), 153);
+    `check(f( ,  , 6), 126);
+    `check(f( ,  ,  ), 123);
+
+    if (!failed) begin
+      $display("PASSED");
+    end
+  end
+
+endmodule

ivtest/ivltests/func_empty_arg2.v

@@ -0,0 +1,29 @@
+// Check that it is possible to call functins with empty arguments if they have
+// default values. Check that this works if the function call is in a module
+// port binding expression.
+
+module M (input [31:0] x, input [31:0] y);
+
+  initial begin
+    #1
+    if (x === 623 && y == 624) begin
+      $display("PASSED");
+    end else begin
+      $display("FAILED");
+    end
+  end
+
+endmodule
+
+module test;
+
+  function [31:0] f(reg [31:0] a, reg [31:0] b = 2, reg [31:0] c = 3);
+    return a * 100 + b * 10 + c;
+  endfunction
+
+  M i_m (
+    .x(f(6, )),
+    .y(f(6, , 4))
+  );
+
+endmodule

ivtest/ivltests/func_empty_arg3.v

@@ -0,0 +1,41 @@
+// Check that it is possible to call functins with empty arguments if they have
+// default values. Check that this works if the function call is part of a force
+// statement in an automatic context.
+
+module test;
+
+  bit failed = 1'b0;
+
+  `define check(expr, val) \
+    if (expr !== val) begin \
+      $display("FAILED. %s, expected %d, got %d", `"expr`", val, expr); \
+      failed = 1'b1; \
+    end
+
+  integer x, y, z, w;
+
+  function automatic integer f(integer a = 1, integer b = 2, integer c = 3);
+    return a * 100 + b * 10 + c;
+  endfunction
+
+  task automatic t;
+    force x = f(4,  , 6);
+    force y = f(4, 5,  );
+    force z = f(4,  ,  );
+    force w = f( ,  , 6);
+  endtask
+
+  initial begin
+    t;
+
+    `check(x, 426);
+    `check(y, 453);
+    `check(z, 423);
+    `check(w, 126);
+
+    if (!failed) begin
+      $display("PASSED");
+    end
+  end
+
+endmodule

ivtest/ivltests/func_empty_arg_fail1.v

@@ -0,0 +1,16 @@
+// Check that passing too many empty arguments to a function results in an error.
+
+module test;
+
+  function f(integer a = 1, integer b = 2, integer c = 3);
+    return a + 10 * b + 100 * c;
+  endfunction
+
+  initial begin
+    integer x;
+    x = f( ,  ,  ,); // This should fail. 4 empty args, even though the function
+                     // only takes 3 args
+    $display("FAILED");
+  end
+
+endmodule

ivtest/ivltests/func_empty_arg_fail2.v

@@ -0,0 +1,16 @@
+// Check that passing additional empty arguments to a function results in an error.
+
+module test;
+
+  function f(integer a = 1, integer b = 2, integer c = 3);
+    return a + 10 * b + 100 * c;
+  endfunction
+
+  initial begin
+    integer x;
+    x = f(4, 5, 6, ); // This should fail. 4 empty args, even though the function
+                      // only takes 3 args
+    $display("FAILED");
+  end
+
+endmodule

ivtest/ivltests/func_empty_arg_fail3.v

@@ -0,0 +1,16 @@
+// Check that passing empty arguments to a function without any ports is an
+// error.
+
+module test;
+
+  function f();
+    return 42;
+  endfunction
+
+  initial begin
+    integer x;
+    x = f( , ); // This should fail. The function takes no arguments. 
+    $display("FAILED");
+  end
+
+endmodule

ivtest/ivltests/func_empty_arg_fail4.v

@@ -0,0 +1,16 @@
+// Check that an error is reported when passing an empty function argument for a
+// port without a default value.
+
+module test;
+
+  function f(integer a, integer b = 2);
+    return a + 10 * b + 100 * c;
+  endfunction
+
+  initial begin
+    integer x;
+    x = f( , 3); // This should fail. No default value specified.
+    $display("FAILED");
+  end
+
+endmodule

ivtest/regress-sv.list

@@ -313,6 +313,13 @@ fork_join_any		normal,-g2009		ivltests
 fork_join_dis		normal,-g2009		ivltests
 fork_join_none		normal,-g2009		ivltests
 fr49			normal,-g2009		ivltests
+func_empty_arg1		normal,-g2005-sv	ivltests
+func_empty_arg2		normal,-g2005-sv	ivltests
+func_empty_arg3		normal,-g2005-sv	ivltests
+func_empty_arg_fail1	CE,-g2005-sv		ivltests
+func_empty_arg_fail2	CE,-g2005-sv		ivltests
+func_empty_arg_fail3	CE,-g2005-sv		ivltests
+func_empty_arg_fail4	CE,-g2005-sv		ivltests
 func_init_var1		normal,-g2009		ivltests
 func_init_var2		normal,-g2009		ivltests
 func_init_var3		normal,-g2009		ivltests

ivtest/regress-vlog95.list

@@ -82,6 +82,7 @@ automatic_task3		CE			ivltests
 br942			CE			ivltests
 br_gh531		CE			ivltests
 def_nettype		CE			ivltests
+func_empty_arg3		CE,-g2005-sv		ivltests
 func_init_var1		CE,-pallowsigned=1	ivltests
 func_init_var2		CE,-pallowsigned=1	ivltests
 func_init_var3		CE,-pallowsigned=1	ivltests

net_func.cc

@@ -82,15 +82,6 @@ bool PECallFunction::check_call_matches_definition_(Design*des, NetScope*dscope)
 {
       assert(dscope);
 
-	/* How many parameters have I got? Normally the size of the
-	   list is correct, but there is the special case of a list of
-	   1 nil pointer. This is how the parser tells me of no
-	   parameter. In other words, ``func()'' is 1 nil parameter. */
-
-      unsigned parms_count = parms_.size();
-      if ((parms_count == 1) && (parms_[0] == 0))
-	    parms_count = 0;
-
       if (dscope->type() != NetScope::FUNC) {
 	    cerr << get_fileline() << ": error: Attempt to call scope "
 		 << scope_path(dscope) << " as a function." << endl;

parse.y

@@ -174,15 +174,13 @@ template <class T> void append(vector<T>&out, const std::vector<T>&in)
 }
 
 /*
- * Look at the list and pull null pointers off the end.
+ * The parser parses an empty argument list as an argument list with an single
+ * empty argument. Fix this up here and replace it with an empty list.
  */
-static void strip_tail_items(list<PExpr*>*lst)
+static void argument_list_fixup(list<PExpr*>*lst)
 {
-      while (! lst->empty()) {
+      if (lst->size() == 1 && !lst->front())
-	    if (lst->back() != 0)
+	    lst->clear();
-		  return;
-	    lst->pop_back();
-      }
 }
 
 /*
@@ -641,7 +639,7 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector<Statem
 %type <expr>  delay_value delay_value_simple
 %type <exprs> delay1 delay3 delay3_opt delay_value_list
 %type <exprs> expression_list_with_nuls expression_list_proper
-%type <exprs> argument_list_parens_opt
+%type <exprs> argument_list_parens argument_list_parens_opt
 %type <exprs> cont_assign cont_assign_list
 
 %type <decl_assignment> variable_decl_assignment
@@ -984,9 +982,7 @@ class_scope
 
 class_new /* IEEE1800-2005 A.2.4 */
   : K_new argument_list_parens_opt
-      { std::list<PExpr*>*expr_list = $2;
+      { PENewClass*tmp = new PENewClass(*$2);
-	strip_tail_items(expr_list);
-	PENewClass*tmp = new PENewClass(*expr_list);
 	FILE_NAME(tmp, @1);
 	delete $2;
 	$$ = tmp;
@@ -994,9 +990,7 @@ class_new /* IEEE1800-2005 A.2.4 */
     // This can't be a class_scope_opt because it will lead to shift/reduce
     // conflicts with array_new
   | class_scope K_new argument_list_parens_opt
-      { std::list<PExpr*>*expr_list = $3;
+      { PENewClass *new_expr = new PENewClass(*$3, $1);
-	strip_tail_items(expr_list);
-	PENewClass *new_expr = new PENewClass(*expr_list, $1);
 	FILE_NAME(new_expr, @2);
 	delete $3;
 	$$ = new_expr;
@@ -3620,12 +3614,22 @@ expression_list_with_nuls
       }
   ;
 
+  /* An argument list enclosed in parenthesis. The parser will parse '()' as a
+   * argument list with an single empty item. We fix this up once the list
+   * parsing is done by replacing it with the empty list.
+   */
+argument_list_parens
+  : '(' expression_list_with_nuls ')'
+      { argument_list_fixup($2);
+	$$ = $2; }
+  ;
+
   /* A task or function can be invoked with the task/function name followed by
    * an argument list in parenthesis or with just the task/function name by
    * itself. When an argument list is used it might be empty. */
 argument_list_parens_opt
-  : '(' expression_list_with_nuls ')'
+  : argument_list_parens
-      { $$ = $2; }
+      { $$ = $1; }
   |
       { $$ = new std::list<PExpr*>; }
 
@@ -3757,21 +3761,17 @@ expr_primary
      function call. If a system identifier, then a system function
      call. It can also be a call to a class method (function). */
 
-  | hierarchy_identifier attribute_list_opt '(' expression_list_with_nuls ')'
+  | hierarchy_identifier attribute_list_opt argument_list_parens
-      { std::list<PExpr*>*expr_list = $4;
+      { PECallFunction*tmp = pform_make_call_function(@1, *$1, *$3);
-	strip_tail_items(expr_list);
-	PECallFunction*tmp = pform_make_call_function(@1, *$1, *expr_list);
 	delete $1;
 	delete $2;
-	delete expr_list;
+	delete $3;
 	$$ = tmp;
       }
-  | class_hierarchy_identifier '(' expression_list_with_nuls ')'
+  | class_hierarchy_identifier argument_list_parens
-      { list<PExpr*>*expr_list = $3;
+      { PECallFunction*tmp = pform_make_call_function(@1, *$1, *$2);
-	strip_tail_items(expr_list);
-	PECallFunction*tmp = pform_make_call_function(@1, *$1, *expr_list);
 	delete $1;
-	delete expr_list;
+	delete $2;
 	$$ = tmp;
       }
   | SYSTEM_IDENTIFIER '(' expression_list_proper ')'
@@ -3782,11 +3782,11 @@ expr_primary
 	delete $3;
 	$$ = tmp;
       }
-  | package_scope hierarchy_identifier { lex_in_package_scope(0); } '(' expression_list_with_nuls ')'
+  | package_scope hierarchy_identifier { lex_in_package_scope(0); } argument_list_parens
-      { PECallFunction*tmp = new PECallFunction($1, *$2, *$5);
+      { PECallFunction*tmp = new PECallFunction($1, *$2, *$4);
 	FILE_NAME(tmp, @2);
 	delete $2;
-	delete $5;
+	delete $4;
 	$$ = tmp;
       }
   | SYSTEM_IDENTIFIER '('  ')'