diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index dd9a6548c..1d8cbce88 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -224,8 +224,22 @@ public: // METHODS static string nodeTextType(AstNode* nodep) { - if (VN_IS(nodep, Var)) { - return "variable"; + if (const AstVar* const varp = VN_CAST(nodep, Var)) { + if (varp->isIO() || varp->isIfaceRef()) { + return "port"; + } else if (varp->isGParam()) { + return "parameter"; + } else if (varp->varType() == VVarType::LPARAM) { + return "local parameter"; + } else { + return "variable"; + } + } else if (const AstParamTypeDType* const dtypep = VN_CAST(nodep, ParamTypeDType)) { + if (dtypep->isGParam()) { + return "type parameter"; + } else { + return "local type parameter"; + } } else if (VN_IS(nodep, Cell)) { return "instance"; } else if (VN_IS(nodep, Task)) { @@ -236,8 +250,6 @@ public: return "block"; } else if (VN_IS(nodep, Iface)) { return "interface"; - } else if (VN_IS(nodep, ParamTypeDType)) { - return "parameter type"; } else { return nodep->prettyTypeName(); } @@ -2262,11 +2274,6 @@ private: m_ds.init(srcp); iterate(nodep); } - bool checkPinRef(AstPin* pinp, VVarType refVarType) { - // In instantiations of modules/ifaces, we shouldn't connect port pins to submodule's - // parameters or vice versa - return pinp->param() == refVarType.isParam(); - } void updateVarUse(AstVar* nodep) { // Avoid dotted.PARAM false positive when in a parameter block // that is if ()'ed off by same dotted name as another block @@ -2365,49 +2372,57 @@ private: if (!nodep->modVarp()) { UASSERT_OBJ(m_pinSymp, nodep, "Pin not under instance?"); VSymEnt* const foundp = m_pinSymp->findIdFlat(nodep->name()); - const char* const whatp = nodep->param() ? "parameter pin" : "pin"; - bool pinCheckFail = false; - if (foundp) { - if (AstVar* const refp = VN_CAST(foundp->nodep(), Var)) { - if (!refp->isIO() && !refp->isParam() && !refp->isIfaceRef()) { - nodep->v3error(ucfirst(whatp) - << " is not an in/out/inout/param/interface: " - << nodep->prettyNameQ()); - } else if (!checkPinRef(nodep, refp->varType())) { - pinCheckFail = true; - } else { - nodep->modVarp(refp); - markAndCheckPinDup(nodep, refp, whatp); - } - } else if (AstParamTypeDType* const refp - = VN_CAST(foundp->nodep(), ParamTypeDType)) { - if (!checkPinRef(nodep, refp->varType())) { - pinCheckFail = true; - } else { - nodep->modPTypep(refp); - markAndCheckPinDup(nodep, refp, whatp); - } - } else { - nodep->v3error(ucfirst(whatp) << " not found: " << nodep->prettyNameQ()); - } - } - if (!foundp || pinCheckFail) { + const char* const whatp = nodep->param() ? "parameter" : "pin"; + if (!foundp) { if (nodep->name() == "__paramNumber1" && m_cellp && VN_IS(m_cellp->modp(), Primitive)) { // Primitive parameter is really a delay we can just ignore VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); return; + } else { + const string suggest + = (nodep->param() ? m_statep->suggestSymFlat(m_pinSymp, nodep->name(), + LinkNodeMatcherVarParam{}) + : m_statep->suggestSymFlat(m_pinSymp, nodep->name(), + LinkNodeMatcherVarIO{})); + nodep->v3warn(PINNOTFOUND, + ucfirst(whatp) + << " not found: " << nodep->prettyNameQ() << '\n' + << (suggest.empty() ? "" : nodep->warnMore() + suggest)); + return; } - const string suggest - = (nodep->param() ? m_statep->suggestSymFlat(m_pinSymp, nodep->name(), - LinkNodeMatcherVarParam{}) - : m_statep->suggestSymFlat(m_pinSymp, nodep->name(), - LinkNodeMatcherVarIO{})); - nodep->v3warn(PINNOTFOUND, - ucfirst(whatp) - << " not found: " << nodep->prettyNameQ() << '\n' - << (suggest.empty() ? "" : nodep->warnMore() + suggest)); } + VVarType refVarType = VVarType::UNKNOWN; + bool wrongPinType = false; + if (AstVar* const varp = VN_CAST(foundp->nodep(), Var)) { + if (varp->isIO() || varp->isParam() || varp->isIfaceRef()) { + refVarType = varp->varType(); + nodep->modVarp(varp); + } else { + wrongPinType = true; + } + } else if (AstParamTypeDType* const typep = VN_CAST(foundp->nodep(), ParamTypeDType)) { + refVarType = typep->varType(); + nodep->modPTypep(typep); + } else { + wrongPinType = true; + } + // Don't connect parameter pin to module ports or vice versa + if (nodep->param() != (refVarType == VVarType::GPARAM)) wrongPinType = true; + if (wrongPinType) { + string targetType = LinkDotState::nodeTextType(foundp->nodep()); + targetType = VString::aOrAn(targetType) + ' ' + targetType; + if (nodep->param()) { + nodep->v3error("Instance attempts to override " + << nodep->prettyNameQ() << " as a " << whatp << ", but it is " + << targetType); + } else { + nodep->v3error("Instance attempts to connect to " + << nodep->prettyNameQ() << ", but it is " << targetType); + } + return; + } + markAndCheckPinDup(nodep, foundp->nodep(), whatp); } // Early return() above when deleted } diff --git a/src/V3Param.cpp b/src/V3Param.cpp index ce5ded6a8..8bc5358a6 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -699,8 +699,8 @@ class ParamProcessor final : public VNDeleter { if (!pinp->exprp()) return; // No-connect if (AstVar* const modvarp = pinp->modVarp()) { if (!modvarp->isGParam()) { - pinp->v3error("Attempted parameter setting of non-parameter: Param " - << pinp->prettyNameQ() << " of " << nodep->prettyNameQ()); + pinp->v3fatalSrc("Attempted parameter setting of non-parameter: Param " + << pinp->prettyNameQ() << " of " << nodep->prettyNameQ()); } else if (VN_IS(pinp->exprp(), InitArray) && arraySubDTypep(modvarp->subDTypep())) { // Array assigned to array AstNode* const exprp = pinp->exprp(); @@ -761,8 +761,8 @@ class ParamProcessor final : public VNDeleter { } } } else { - pinp->v3error("Parameter not found in sub-module: Param " - << pinp->prettyNameQ() << " of " << nodep->prettyNameQ()); + pinp->v3fatalSrc("Parameter not found in sub-module: Param " + << pinp->prettyNameQ() << " of " << nodep->prettyNameQ()); } } diff --git a/src/V3String.cpp b/src/V3String.cpp index c23a15705..e42bee71d 100644 --- a/src/V3String.cpp +++ b/src/V3String.cpp @@ -273,6 +273,17 @@ bool VString::endsWith(const string& str, const string& suffix) { if (str.length() < suffix.length()) return false; return str.compare(str.length() - suffix.length(), suffix.length(), suffix) == 0; } +string VString::aOrAn(const char* word) { + switch (word[0]) { + case '\0': return ""; + case 'a': + case 'e': + case 'i': + case 'o': + case 'u': return "an"; + default: return "a"; + } +} //###################################################################### // VHashSha256 diff --git a/src/V3String.h b/src/V3String.h index 33c792757..149ce2d43 100644 --- a/src/V3String.h +++ b/src/V3String.h @@ -130,6 +130,9 @@ public: static bool endsWith(const string& str, const string& suffix); // Return true if char is valid character in word static bool isWordChar(char c) { return isalnum(c) || c == '_'; } + // Return proper article (a/an) for a word. May be inaccurate for some special words + static string aOrAn(const char* word); + static string aOrAn(const string& word) { return aOrAn(word.c_str()); } }; //###################################################################### diff --git a/test_regress/t/t_class_param_bad1.out b/test_regress/t/t_class_param_bad1.out index 756f6e838..55071fadd 100644 --- a/test_regress/t/t_class_param_bad1.out +++ b/test_regress/t/t_class_param_bad1.out @@ -1,9 +1,9 @@ -%Error-PINNOTFOUND: t/t_class_param_bad1.v:12:11: Parameter pin not found: 'PARAMBAD' +%Error-PINNOTFOUND: t/t_class_param_bad1.v:12:11: Parameter not found: 'PARAMBAD' : ... Suggested alternative: 'PARAMB' 12 | Cls #(.PARAMBAD(1)) c; | ^~~~~~~~ ... For error description see https://verilator.org/warn/PINNOTFOUND?v=latest -%Error-PINNOTFOUND: t/t_class_param_bad1.v:13:14: Parameter pin not found: '__paramNumber2' +%Error-PINNOTFOUND: t/t_class_param_bad1.v:13:14: Parameter not found: '__paramNumber2' 13 | Cls #(13, 1) cd; | ^ %Error: Exiting due to diff --git a/test_regress/t/t_class_param_override_local_bad.out b/test_regress/t/t_class_param_override_local_bad.out index dacafe468..37c671895 100644 --- a/test_regress/t/t_class_param_override_local_bad.out +++ b/test_regress/t/t_class_param_override_local_bad.out @@ -1,14 +1,14 @@ -%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:23:30: Parameter pin not found: '__paramNumber1' +%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:23:30: Parameter not found: '__paramNumber1' 23 | class Cls3 implements Icls1#(2), Icls2#(0); | ^ ... For error description see https://verilator.org/warn/PINNOTFOUND?v=latest -%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:23:41: Parameter pin not found: '__paramNumber1' +%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:23:41: Parameter not found: '__paramNumber1' 23 | class Cls3 implements Icls1#(2), Icls2#(0); | ^ -%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:29:23: Parameter pin not found: '__paramNumber1' +%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:29:23: Parameter not found: '__paramNumber1' 29 | automatic Cls1#(bit) cls1 = new; | ^~~ -%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:30:23: Parameter pin not found: '__paramNumber1' +%Error-PINNOTFOUND: t/t_class_param_override_local_bad.v:30:23: Parameter not found: '__paramNumber1' 30 | automatic Cls2#(1) cls2 = new; | ^ %Error: Exiting due to diff --git a/test_regress/t/t_dist_warn_coverage.pl b/test_regress/t/t_dist_warn_coverage.pl index 25b6be901..558a35b3d 100755 --- a/test_regress/t/t_dist_warn_coverage.pl +++ b/test_regress/t/t_dist_warn_coverage.pl @@ -39,7 +39,6 @@ foreach my $s ( 'Array initialization has too few elements, need element ', 'Assigned pin is neither input nor output', 'Assignment pattern with no members', - 'Attempted parameter setting of non-parameter: Param ', 'Can\'t find varpin scope of ', 'Can\'t resolve module reference: \'', 'Cannot write preprocessor output: ', @@ -60,7 +59,6 @@ foreach my $s ( 'Modport not referenced as .', 'Modport not referenced from underneath an interface: ', 'Non-interface used as an interface: ', - 'Parameter not found in sub-module: Param ', 'Parameter type pin value isn\'t a type: Param ', 'Parameter type variable isn\'t a type: Param ', 'Pattern replication value of 0 is not legal.', diff --git a/test_regress/t/t_inst_param_override_bad.out b/test_regress/t/t_inst_param_override_bad.out new file mode 100644 index 000000000..13f027a3b --- /dev/null +++ b/test_regress/t/t_inst_param_override_bad.out @@ -0,0 +1,19 @@ +%Error: t/t_inst_param_override_bad.v:33:23: Instance attempts to override 'PACKED_DATA_WIDTH' as a parameter, but it is a local parameter + 33 | axi_stream_if # (.PACKED_DATA_WIDTH(10)) axis1(clk); + | ^~~~~~~~~~~~~~~~~ +%Error: t/t_inst_param_override_bad.v:35:23: Instance attempts to override 'mytask' as a parameter, but it is a task + 35 | axi_stream_if # (.mytask(10)) axis2(clk); + | ^~~~~~ +%Error: t/t_inst_param_override_bad.v:37:23: Instance attempts to override 'my_genvar' as a parameter, but it is a variable + 37 | axi_stream_if # (.my_genvar(10)) axis3(clk); + | ^~~~~~~~~ +%Error: t/t_inst_param_override_bad.v:39:23: Instance attempts to override 'clk' as a parameter, but it is a port + 39 | axi_stream_if # (.clk(10)) axis4(clk); + | ^~~ +%Error: t/t_inst_param_override_bad.v:41:23: Instance attempts to override 'tvalid' as a parameter, but it is a variable + 41 | axi_stream_if # (.tvalid(10)) axis5(clk); + | ^~~~~~ +%Error: t/t_inst_param_override_bad.v:43:23: Instance attempts to override 'i_sub' as a parameter, but it is an instance + 43 | axi_stream_if # (.i_sub(10)) axis6(clk); + | ^~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_inst_param_override_bad.pl b/test_regress/t/t_inst_param_override_bad.pl new file mode 100755 index 000000000..376c2d2ee --- /dev/null +++ b/test_regress/t/t_inst_param_override_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2023 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_inst_param_override_bad.v b/test_regress/t/t_inst_param_override_bad.v new file mode 100644 index 000000000..8ac842b23 --- /dev/null +++ b/test_regress/t/t_inst_param_override_bad.v @@ -0,0 +1,45 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2023 by Anthony Donlon. +// SPDX-License-Identifier: CC0-1.0 + +module sub(); +endmodule + +interface axi_stream_if #( + parameter int DATA_WIDTH = 64, + parameter type TUSER_TYPE = logic +) ( + input clk +); + + task mytask(); + endtask : mytask + + genvar my_genvar; + + logic tvalid; + + sub i_sub(); + + localparam PACKED_DATA_WIDTH = DATA_WIDTH + DATA_WIDTH / 8 + 1 + $bits(TUSER_TYPE); +endinterface + +module t; + logic clk; + + // overriding a localparam + axi_stream_if # (.PACKED_DATA_WIDTH(10)) axis1(clk); + // overriding a non-var + axi_stream_if # (.mytask(10)) axis2(clk); + // overriding a non-port/interface/param var + axi_stream_if # (.my_genvar(10)) axis3(clk); + // overriding a port + axi_stream_if # (.clk(10)) axis4(clk); + // overriding a signal + axi_stream_if # (.tvalid(10)) axis5(clk); + // overriding an instance + axi_stream_if # (.i_sub(10)) axis6(clk); + +endmodule diff --git a/test_regress/t/t_inst_pin_place_bad.out b/test_regress/t/t_inst_pin_place_bad.out index 0d3f845a6..8cc323ac1 100644 --- a/test_regress/t/t_inst_pin_place_bad.out +++ b/test_regress/t/t_inst_pin_place_bad.out @@ -3,10 +3,10 @@ | ^~~~~ ... For warning description see https://verilator.org/warn/PINMISSING?v=latest ... Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message. -%Error-PINNOTFOUND: t/t_inst_pin_place_bad.v:22:10: Pin not found: 'PARAM_A' +%Error: t/t_inst_pin_place_bad.v:22:10: Instance attempts to connect to 'PARAM_A', but it is a parameter 22 | .PARAM_A(1) | ^~~~~~~ -%Error-PINNOTFOUND: t/t_inst_pin_place_bad.v:20:10: Parameter pin not found: 'pin_1' +%Error: t/t_inst_pin_place_bad.v:20:10: Instance attempts to override 'pin_1' as a parameter, but it is a port 20 | .pin_1(1) | ^~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_lint_pindup_bad.out b/test_regress/t/t_lint_pindup_bad.out index 94b81c87b..7cdb90a6c 100644 --- a/test_regress/t/t_lint_pindup_bad.out +++ b/test_regress/t/t_lint_pindup_bad.out @@ -13,20 +13,20 @@ : ... Suggested alternative: 'exists' 22 | .nexist(i2) | ^~~~~~ -%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:16:9: Parameter pin not found: 'NEXIST' +%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:16:9: Parameter not found: 'NEXIST' : ... Suggested alternative: 'EXIST' 16 | .NEXIST(1), | ^~~~~~ -%Error: t/t_lint_pindup_bad.v:17:9: Duplicate parameter pin connection: 'P' +%Error: t/t_lint_pindup_bad.v:17:9: Duplicate parameter connection: 'P' 17 | .P(2), | ^ - t/t_lint_pindup_bad.v:15:8: ... Location of original parameter pin connection + t/t_lint_pindup_bad.v:15:8: ... Location of original parameter connection 15 | #(, | ^ -%Error: t/t_lint_pindup_bad.v:18:9: Duplicate parameter pin connection: 'P' +%Error: t/t_lint_pindup_bad.v:18:9: Duplicate parameter connection: 'P' 18 | .P(3)) | ^ - t/t_lint_pindup_bad.v:15:8: ... Location of original parameter pin connection + t/t_lint_pindup_bad.v:15:8: ... Location of original parameter connection 15 | #(, | ^ %Error: Exiting due to diff --git a/test_regress/t/t_lint_pinnotfound_bad.out b/test_regress/t/t_lint_pinnotfound_bad.out index 81a6f40e5..6aded1ba0 100644 --- a/test_regress/t/t_lint_pinnotfound_bad.out +++ b/test_regress/t/t_lint_pinnotfound_bad.out @@ -2,7 +2,7 @@ 12 | b b_inst1 (.x(1'b0)); | ^ ... For error description see https://verilator.org/warn/PINNOTFOUND?v=latest -%Error-PINNOTFOUND: t/t_lint_pinnotfound_bad.v:13:6: Parameter pin not found: 'PX' +%Error-PINNOTFOUND: t/t_lint_pinnotfound_bad.v:13:6: Parameter not found: 'PX' 13 | b #(.PX(1'b0)) b_inst2 (); | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_param_implicit_local_bad.out b/test_regress/t/t_param_implicit_local_bad.out index ef6479300..159b4f135 100644 --- a/test_regress/t/t_param_implicit_local_bad.out +++ b/test_regress/t/t_param_implicit_local_bad.out @@ -1,23 +1,23 @@ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:15:20: Parameter pin not found: '__paramNumber2' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:15:20: Parameter not found: '__paramNumber2' 15 | NestedCls #(1, 2) cls; | ^ ... For error description see https://verilator.org/warn/PINNOTFOUND?v=latest -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:17:21: Parameter pin not found: '__paramNumber3' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:17:21: Parameter not found: '__paramNumber3' 17 | mod1 # ( 3, 4, 5 ) i_mod1 (); | ^ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:15: Parameter pin not found: '__paramNumber1' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:15: Parameter not found: '__paramNumber1' 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); | ^ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:18: Parameter pin not found: '__paramNumber2' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:18: Parameter not found: '__paramNumber2' 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); | ^~ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:22: Parameter pin not found: '__paramNumber3' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:19:22: Parameter not found: '__paramNumber3' 19 | mod3 # ( 7, 24, 25 ) i_mod3 (); | ^~ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:20:22: Parameter pin not found: '__paramNumber3' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:20:22: Parameter not found: '__paramNumber3' 20 | intf1 # ( 8, 15, 17 ) i_intf1 (); | ^~ -%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:21:22: Parameter pin not found: '__paramNumber3' +%Error-PINNOTFOUND: t/t_param_implicit_local_bad.v:21:22: Parameter not found: '__paramNumber3' 21 | prgm1 # ( 9, 40, 41 ) i_prgm1 (); | ^~ %Error: Exiting due to