Fixup empty function/task argument lists in a consistent way

As a quirk of the (System)Verilog grammar a function or task argument list
with no arguments can not be distinguished from a argument list with a
single empty argument. The iverilog parses it as the latter. There are
currently many places in the code base where this is fixed up in slightly
different ways.

Fix this up in the parser in a central way before passing the arguments to
the elaboration stage.

The existing implementation in some cases removes all empty trailing
arguments. While this works to handle the case for zero arguments it also
hides some errors that should be detected. E.g. in the following 3
arguments are passed to a function which only takes two arguments. But no
error is reported since the explicitly specified empty arguments are
removed.

```
function f(integer a, integer b = 2); ... endfunction
f(1,,);
```

In the new implementation the empty argument will only be removed if there
is exactly one empty argument in the argument list.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
This commit is contained in:
Lars-Peter Clausen 2023-01-02 18:59:46 -08:00
parent aea202b2e9
commit 6cf19ec964
4 changed files with 30 additions and 69 deletions

View File

@ -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);
@ -2907,11 +2897,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
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))
if (parm_count == 0 && actual_count == 0)
return 0;
if (actual_count > parm_count) {

View File

@ -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;
@ -4356,12 +4346,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()) {

View File

@ -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;

58
parse.y
View File

@ -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->back() != 0)
return;
lst->pop_back();
}
if (lst->size() == 1 && !lst->front())
lst->clear();
}
/*
@ -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;
strip_tail_items(expr_list);
PENewClass*tmp = new PENewClass(*expr_list);
{ PENewClass*tmp = new PENewClass(*$2);
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;
strip_tail_items(expr_list);
PENewClass *new_expr = new PENewClass(*expr_list, $1);
{ PENewClass *new_expr = new PENewClass(*$3, $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 ')'
{ $$ = $2; }
: argument_list_parens
{ $$ = $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 ')'
{ std::list<PExpr*>*expr_list = $4;
strip_tail_items(expr_list);
PECallFunction*tmp = pform_make_call_function(@1, *$1, *expr_list);
| hierarchy_identifier attribute_list_opt argument_list_parens
{ PECallFunction*tmp = pform_make_call_function(@1, *$1, *$3);
delete $1;
delete $2;
delete expr_list;
delete $3;
$$ = tmp;
}
| class_hierarchy_identifier '(' expression_list_with_nuls ')'
{ list<PExpr*>*expr_list = $3;
strip_tail_items(expr_list);
PECallFunction*tmp = pform_make_call_function(@1, *$1, *expr_list);
| class_hierarchy_identifier argument_list_parens
{ PECallFunction*tmp = pform_make_call_function(@1, *$1, *$2);
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 ')'
{ PECallFunction*tmp = new PECallFunction($1, *$2, *$5);
| package_scope hierarchy_identifier { lex_in_package_scope(0); } argument_list_parens
{ PECallFunction*tmp = new PECallFunction($1, *$2, *$4);
FILE_NAME(tmp, @2);
delete $2;
delete $5;
delete $4;
$$ = tmp;
}
| SYSTEM_IDENTIFIER '(' ')'