From 0ab1ed916fd3c6a3b55763fcaabcf00f18ceadf4 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 1 Feb 2022 10:18:26 +0100 Subject: [PATCH] Report error when trying to override non-existing parameter Overriding a parameter that does not exist will only generate a warning at the moment. This can hide programming mistakes such as an typo in a parameter override. There is nothing in the LRMs to support that this should only be warning, so elevate this to an error. This is consistent with how an error is generated when trying to reference a non-existing port or variable. The generated error message differentiates between whether the parameter does not exist at all, or whether it is a localparam. There are two regression tests that rely on that only a warning is generated, these have been updated to expect an error. Signed-off-by: Lars-Peter Clausen --- elab_scope.cc | 12 +++-------- ivtest/gold/br_gh157.gold | 4 ++-- ivtest/gold/pr498b.gold | 2 ++ ivtest/ivltests/{pr498.v => pr498a.v} | 1 - ivtest/ivltests/pr498b.v | 30 +++++++++++++++++++++++++++ ivtest/regress-ivl1.list | 2 +- ivtest/regress-vlg.list | 3 ++- net_design.cc | 15 ++------------ net_scope.cc | 22 ++++++++++++++------ netlist.h | 2 +- 10 files changed, 59 insertions(+), 34 deletions(-) create mode 100644 ivtest/gold/pr498b.gold rename ivtest/ivltests/{pr498.v => pr498a.v} (95%) create mode 100644 ivtest/ivltests/pr498b.v diff --git a/elab_scope.cc b/elab_scope.cc index c7d205d1f..c1b8546dd 100644 --- a/elab_scope.cc +++ b/elab_scope.cc @@ -623,7 +623,7 @@ static void elaborate_scope_classes(Design*des, NetScope*scope, } } -static void replace_scope_parameters(NetScope*scope, const LineInfo&loc, +static void replace_scope_parameters(Design *des, NetScope*scope, const LineInfo&loc, const Module::replace_t&replacements) { if (debug_scopes) { @@ -649,13 +649,7 @@ static void replace_scope_parameters(NetScope*scope, const LineInfo&loc, cerr << loc.get_fileline() << ": : " << "Type=" << val->expr_type() << endl; } - bool flag = scope->replace_parameter((*cur).first, val, - scope->parent()); - if (! flag) { - cerr << val->get_fileline() << ": warning: parameter " - << (*cur).first << " not found in " - << scope_path(scope) << "." << endl; - } + scope->replace_parameter(des, (*cur).first, val, scope->parent()); } } @@ -818,7 +812,7 @@ bool Module::elaborate_scope(Design*des, NetScope*scope, // Run parameter replacements that were collected from the // containing scope and meant for me. - replace_scope_parameters(scope, *this, replacements); + replace_scope_parameters(des, scope, *this, replacements); elaborate_scope_enumerations(des, scope, enum_sets); diff --git a/ivtest/gold/br_gh157.gold b/ivtest/gold/br_gh157.gold index 46ba98b2f..14dc1f437 100644 --- a/ivtest/gold/br_gh157.gold +++ b/ivtest/gold/br_gh157.gold @@ -1,2 +1,2 @@ -./ivltests/br_gh157.v:13: warning: parameter x not found in test.dut. -2 +./ivltests/br_gh157.v:13: error: Cannot override localparam `x` in `test.dut`. +1 error(s) during elaboration. diff --git a/ivtest/gold/pr498b.gold b/ivtest/gold/pr498b.gold new file mode 100644 index 000000000..815b71411 --- /dev/null +++ b/ivtest/gold/pr498b.gold @@ -0,0 +1,2 @@ +./ivltests/pr498b.v:23: error: parameter `foo` not found in `main`. +1 error(s) during elaboration. diff --git a/ivtest/ivltests/pr498.v b/ivtest/ivltests/pr498a.v similarity index 95% rename from ivtest/ivltests/pr498.v rename to ivtest/ivltests/pr498a.v index 6a943a84b..44951133d 100644 --- a/ivtest/ivltests/pr498.v +++ b/ivtest/ivltests/pr498a.v @@ -20,7 +20,6 @@ module main; test tt(); - defparam foo = 3; /* This should generate a warning. */ defparam tt.foo = 4; endmodule // main diff --git a/ivtest/ivltests/pr498b.v b/ivtest/ivltests/pr498b.v new file mode 100644 index 000000000..449dba64e --- /dev/null +++ b/ivtest/ivltests/pr498b.v @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2002 Stephen Williams (steve@icarus.com) + * + * This source code is free software; you can redistribute it + * and/or modify it in source code form under the terms of the GNU + * General Public License as published by the Free Software + * Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + */ + + +module main; + test tt(); + defparam foo = 3; /* This should generate an error. */ +endmodule // main + +module test; + parameter foo = 10; + reg [foo-1:0] bar; + +endmodule // test diff --git a/ivtest/regress-ivl1.list b/ivtest/regress-ivl1.list index 97ead6058..bf2394966 100644 --- a/ivtest/regress-ivl1.list +++ b/ivtest/regress-ivl1.list @@ -121,7 +121,7 @@ clog2 normal ivltests clog2-signal normal ivltests param_vec normal ivltests param_vec2 normal ivltests -pr498 normal ivltests +pr498a normal ivltests pr721 normal ivltests pr809 normal ivltests pr809b normal ivltests diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index 1aafa42eb..89075234c 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -308,7 +308,7 @@ br_gh127e normal ivltests gold=br_gh127e.gold br_gh127f normal ivltests gold=br_gh127f.gold br_gh142 CE ivltests br_gh152 CE ivltests -br_gh157 normal ivltests gold=br_gh157.gold +br_gh157 CE ivltests gold=br_gh157.gold br_gh162 normal ivltests br_gh163 CE ivltests br_gh198 normal ivltests gold=br_gh198.gold @@ -736,6 +736,7 @@ pr445 normal ivltests pr478 normal ivltests pr487 normal ivltests gold=pr487.gold pr492 normal ivltests gold=pr492.gold +pr498b CE ivltests gold=pr498b.gold pr508 normal ivltests pr509 normal ivltests pr509b normal ivltests diff --git a/net_design.cc b/net_design.cc index f5d2eddb6..12c8b14c6 100644 --- a/net_design.cc +++ b/net_design.cc @@ -433,13 +433,7 @@ void NetScope::run_defparams(Design*des) continue; } - bool flag = targ_scope->replace_parameter(perm_name, val, this); - if (! flag) { - cerr << val->get_fileline() << ": warning: parameter " - << perm_name << " not found in " - << scope_path(targ_scope) << "." << endl; - } - + targ_scope->replace_parameter(des, perm_name, val, this); } // If some of the defparams didn't find a scope in the name, @@ -474,12 +468,7 @@ void NetScope::run_defparams_later(Design*des) continue; } - bool flag = targ_scope->replace_parameter(name, val, this); - if (! flag) { - cerr << val->get_fileline() << ": warning: parameter " - << name << " not found in " - << scope_path(targ_scope) << "." << endl; - } + targ_scope->replace_parameter(des, name, val, this); // We'll need to re-evaluate parameters in this scope target_scopes.insert(targ_scope); diff --git a/net_scope.cc b/net_scope.cc index 1a4216069..5abdf0b77 100644 --- a/net_scope.cc +++ b/net_scope.cc @@ -25,6 +25,7 @@ # include "netclass.h" # include "netenum.h" # include "netvector.h" +# include "PExpr.h" # include "PPackage.h" # include # include @@ -381,18 +382,27 @@ bool NetScope::auto_name(const char*prefix, char pad, const char* suffix) * Return false if the parameter does not already exist. * A parameter is not automatically created. */ -bool NetScope::replace_parameter(perm_string key, PExpr*val, NetScope*scope) +void NetScope::replace_parameter(Design *des, perm_string key, PExpr*val, NetScope*scope) { - if (parameters.find(key) == parameters.end()) - return false; + if (parameters.find(key) == parameters.end()) { + cerr << val->get_fileline() << ": error: parameter `" + << key << "` not found in `" + << scope_path(this) << "`." << endl; + des->errors++; + return; + } param_expr_t&ref = parameters[key]; - if (ref.local_flag) - return false; + if (ref.local_flag) { + cerr << val->get_fileline() << ": error: " + << "Cannot override localparam `" << key << "` in `" + << scope_path(this) << "`." << endl; + des->errors++; + return; + } ref.val_expr = val; ref.val_scope = scope; - return true; } bool NetScope::make_parameter_unannotatable(perm_string key) diff --git a/netlist.h b/netlist.h index 85dc8ce8b..a5babeb4e 100644 --- a/netlist.h +++ b/netlist.h @@ -983,7 +983,7 @@ class NetScope : public Definitions, public Attrib { expression with a new expression, without affecting the range or signed_flag. Return false if the name does not exist. */ - bool replace_parameter(perm_string name, PExpr*val, NetScope*scope); + void replace_parameter(Design *des, perm_string name, PExpr*val, NetScope*scope); /* This is used to ensure the value of a parameter cannot be changed at run-time. This is required if a specparam is used