From bd1ac03828c09e30bb1ee68ddcd2115b81f20c27 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 31 Jul 2025 18:38:50 -0400 Subject: [PATCH] Add I/O versus data declaration checking. --- Changes | 2 +- src/V3AstNodeOther.h | 4 +++ src/V3LinkDot.cpp | 20 ++++++++++++- test_regress/t/t_func_nansi_bad.out | 44 +++++++++++++++++++++++++++++ test_regress/t/t_func_nansi_bad.py | 16 +++++++++++ test_regress/t/t_func_nansi_bad.v | 38 +++++++++++++++++++++++++ test_regress/t/t_inst_nansi.v | 4 +-- test_regress/t/t_inst_nansi_bad.out | 32 +++++++++++++++++++++ test_regress/t/t_inst_nansi_bad.py | 16 +++++++++++ test_regress/t/t_inst_nansi_bad.v | 30 ++++++++++++++++++++ test_regress/t/t_param_sel.v | 2 +- test_regress/t/t_var_dup_bad.out | 6 ++++ 12 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 test_regress/t/t_func_nansi_bad.out create mode 100755 test_regress/t/t_func_nansi_bad.py create mode 100644 test_regress/t/t_func_nansi_bad.v create mode 100644 test_regress/t/t_inst_nansi_bad.out create mode 100755 test_regress/t/t_inst_nansi_bad.py create mode 100644 test_regress/t/t_inst_nansi_bad.v diff --git a/Changes b/Changes index f19ed3227..71fe31bf2 100644 --- a/Changes +++ b/Changes @@ -16,7 +16,7 @@ Verilator 5.039 devel * Add ENUMITEMWIDTH error, and apply to X-extended and ranged values. * Add NOEFFECT warning, replacing previous `foreach` error. * Add SPECIFYIGN warning for specify constructs that were previously silently ignored. -* Add enum base data type, and wire data type checking per IEEE. +* Add enum base data type, wire data type, and I/O versus data declaration checking per IEEE. * Add error on missing forward declarations (#6206). [Alex Solomatnikov] * Add error when trying to assign class object to variable of non-class types (#6237). [Igor Zaworski, Antmicro Ltd.] * Add `-DVERILATOR=1` definition to compiler flags when using verilated.mk. diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index d4a603ed8..16dcfb6fb 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1987,6 +1987,7 @@ class AstVar final : public AstNode { bool m_attrSFormat : 1; // User sformat attribute bool m_attrSplitVar : 1; // declared with split_var metacomment bool m_fileDescr : 1; // File descriptor + bool m_gotNansiType : 1; // Linker saw Non-ANSI type declaration bool m_isConst : 1; // Table contains constant data bool m_isContinuously : 1; // Ever assigned continuously (for force/release) bool m_hasStrengthAssignment : 1; // Is on LHS of assignment with strength specifier @@ -2034,6 +2035,7 @@ class AstVar final : public AstNode { m_attrSFormat = false; m_attrSplitVar = false; m_fileDescr = false; + m_gotNansiType = false; m_isConst = false; m_isContinuously = false; m_hasStrengthAssignment = false; @@ -2188,6 +2190,8 @@ public: if (flag) m_funcLocalSticky = true; } void funcReturn(bool flag) { m_funcReturn = flag; } + void gotNansiType(bool flag) { m_gotNansiType = flag; } + bool gotNansiType() { return m_gotNansiType; } void hasStrengthAssignment(bool flag) { m_hasStrengthAssignment = flag; } bool hasStrengthAssignment() { return m_hasStrengthAssignment; } void isDpiOpenArray(bool flag) { m_isDpiOpenArray = flag; } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 59182ca99..20ac29fea 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1470,7 +1470,8 @@ class LinkDotFindVisitor final : public VNVisitor { const bool nansiBad = (((findvarp->isDeclTyped() || findvarp->isNet()) && (nodep->isDeclTyped() || nodep->isNet())) - || (findvarp->isIO() && nodep->isIO())); // e.g. !(output && output) + || (findvarp->isIO() && nodep->isIO()) // e.g. !(output && output) + || findvarp->gotNansiType()); // e.g. int x && int x const bool ansiBad = findvarp->isAnsi() || nodep->isAnsi(); // dup illegal with ANSI if (ansiBad || nansiBad) { @@ -1496,6 +1497,7 @@ class LinkDotFindVisitor final : public VNVisitor { } else { findvarp->combineType(nodep); findvarp->fileline()->modifyStateInherit(nodep->fileline()); + findvarp->gotNansiType(true); UASSERT_OBJ(nodep->subDTypep(), nodep, "Var has no type"); if (nodep->subDTypep()->numeric().isSigned() && !findvarp->subDTypep()->numeric().isSigned()) { @@ -1513,6 +1515,22 @@ class LinkDotFindVisitor final : public VNVisitor { newdtypep->unlinkFrBack(); findvarp->childDTypep(newdtypep); } + if (nodep->childDTypep() && findvarp->childDTypep() + && !(VN_IS(nodep->childDTypep(), BasicDType) + && VN_AS(nodep->childDTypep(), BasicDType)->keyword() + == VBasicDTypeKwd::LOGIC_IMPLICIT) + && !(VN_IS(findvarp->childDTypep(), BasicDType) + && VN_AS(findvarp->childDTypep(), BasicDType)->keyword() + == VBasicDTypeKwd::LOGIC_IMPLICIT) + && !nodep->sameTree(findvarp)) { + nodep->v3error("Non-ANSI I/O declaration of signal " + "conflicts with type declaration: " + << nodep->prettyNameQ() << '\n' + << nodep->warnContextPrimary() << '\n' + << findvarp->warnOther() + << "... Location of other declaration\n" + << findvarp->warnContextSecondary()); + } } VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); } else { diff --git a/test_regress/t/t_func_nansi_bad.out b/test_regress/t/t_func_nansi_bad.out new file mode 100644 index 000000000..6ba6f4330 --- /dev/null +++ b/test_regress/t/t_func_nansi_bad.out @@ -0,0 +1,44 @@ +%Error: t/t_func_nansi_bad.v:13:14: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad1' + 13 | shortint bad1; + | ^~~~ + t/t_func_nansi_bad.v:12:18: ... Location of other declaration + 12 | input [15:0] bad1; + | ^~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_func_nansi_bad.v:18:7: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad2' + 18 | T bad2; + | ^~~~ + t/t_func_nansi_bad.v:17:18: ... Location of other declaration + 17 | input [31:0] bad2; + | ^~~~ +%Error: t/t_func_nansi_bad.v:23:15: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad3' + 23 | reg [3:0] bad3; + | ^~~~ + t/t_func_nansi_bad.v:22:17: ... Location of other declaration + 22 | input [7:0] bad3; + | ^~~~ +%Error: t/t_func_nansi_bad.v:28:9: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad4' + 28 | reg bad4; + | ^~~~ + t/t_func_nansi_bad.v:27:17: ... Location of other declaration + 27 | input [7:0] bad4; + | ^~~~ +%Error: t/t_func_nansi_bad.v:29:9: Duplicate declaration of signal: 'bad4' + 29 | reg bad4; + | ^~~~ + t/t_func_nansi_bad.v:27:17: ... Location of original declaration + 27 | input [7:0] bad4; + | ^~~~ +%Error: t/t_func_nansi_bad.v:34:17: Duplicate declaration of signal: 'bad5' + 34 | input [7:0] bad5; + | ^~~~ + t/t_func_nansi_bad.v:33:17: ... Location of original declaration + 33 | input [7:0] bad5; + | ^~~~ +%Error: t/t_func_nansi_bad.v:35:9: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad5' + 35 | reg bad5; + | ^~~~ + t/t_func_nansi_bad.v:33:17: ... Location of other declaration + 33 | input [7:0] bad5; + | ^~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_func_nansi_bad.py b/test_regress/t/t_func_nansi_bad.py new file mode 100755 index 000000000..55203b6c9 --- /dev/null +++ b/test_regress/t/t_func_nansi_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_func_nansi_bad.v b/test_regress/t/t_func_nansi_bad.v new file mode 100644 index 000000000..c69df2b8c --- /dev/null +++ b/test_regress/t/t_func_nansi_bad.v @@ -0,0 +1,38 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +typedef int T; + +module test; + + task t1; + input [15:0] bad1; + shortint bad1; // <--- Error (type doesn't match above) + endtask + + task t2; + input [31:0] bad2; + T bad2; // <--- Error (type doesn't match above) + endtask + + task t3; + input [7:0] bad3; + reg [3:0] bad3; // <--- Error (type doesn't match above) + endtask + + task t4; + input [7:0] bad4; + reg bad4; + reg bad4; // <--- Error (duplicate) + endtask + + task t5; + input [7:0] bad5; + input [7:0] bad5; // <--- Error (duplicate) + reg bad5; + endtask + +endmodule diff --git a/test_regress/t/t_inst_nansi.v b/test_regress/t/t_inst_nansi.v index 651df6f41..370d31894 100644 --- a/test_regress/t/t_inst_nansi.v +++ b/test_regress/t/t_inst_nansi.v @@ -15,10 +15,10 @@ module t(b, si, i, li, w3, w4); output i; // Output after type output li; - input w3; + input [2:0] w3; wire [2:0] w3; wire [3:0] w4; - input w4; + input [3:0] w4; initial begin if ($bits(b) != 8) $stop; diff --git a/test_regress/t/t_inst_nansi_bad.out b/test_regress/t/t_inst_nansi_bad.out new file mode 100644 index 000000000..3576c08e5 --- /dev/null +++ b/test_regress/t/t_inst_nansi_bad.out @@ -0,0 +1,32 @@ +%Error: t/t_inst_nansi_bad.v:14:12: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad1' + 14 | shortint bad1; + | ^~~~ + t/t_inst_nansi_bad.v:13:17: ... Location of other declaration + 13 | output [15:0] bad1; + | ^~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_inst_nansi_bad.v:17:5: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad2' + 17 | T bad2; + | ^~~~ + t/t_inst_nansi_bad.v:16:17: ... Location of other declaration + 16 | output [31:0] bad2; + | ^~~~ +%Error: t/t_inst_nansi_bad.v:20:13: Non-ANSI I/O declaration of signal conflicts with type declaration: 'bad3' + 20 | reg [7:0] bad3; + | ^~~~ + t/t_inst_nansi_bad.v:19:16: ... Location of other declaration + 19 | output [3:0] bad3; + | ^~~~ +%Error: t/t_inst_nansi_bad.v:24:7: Duplicate declaration of signal: 'bad4' + 24 | reg bad4; + | ^~~~ + t/t_inst_nansi_bad.v:22:10: ... Location of original declaration + 22 | output bad4; + | ^~~~ +%Error: t/t_inst_nansi_bad.v:27:10: Duplicate declaration of signal: 'bad5' + 27 | output bad5; + | ^~~~ + t/t_inst_nansi_bad.v:26:10: ... Location of original declaration + 26 | output bad5; + | ^~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_inst_nansi_bad.py b/test_regress/t/t_inst_nansi_bad.py new file mode 100755 index 000000000..55203b6c9 --- /dev/null +++ b/test_regress/t/t_inst_nansi_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_inst_nansi_bad.v b/test_regress/t/t_inst_nansi_bad.v new file mode 100644 index 000000000..b87a602a1 --- /dev/null +++ b/test_regress/t/t_inst_nansi_bad.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +typedef int T; + +module test ( /*AUTOARG*/ + // Outputs + bad1, bad2, bad3, bad4, bad5 + ); + output [15:0] bad1; + shortint bad1; // <--- Error (type doesn't match above) + + output [31:0] bad2; + T bad2; // <--- Error (type doesn't match above due to range) + + output [3:0] bad3; + reg [7:0] bad3; // <--- Error (range doesn't match) + + output bad4; + reg bad4; + reg bad4; // <--- Error (duplicate) + + output bad5; + output bad5; // <--- Error (duplicate) + reg bad5; // <--- Error (duplicate) + +endmodule diff --git a/test_regress/t/t_param_sel.v b/test_regress/t/t_param_sel.v index cf57dfb29..b87db56e2 100644 --- a/test_regress/t/t_param_sel.v +++ b/test_regress/t/t_param_sel.v @@ -90,5 +90,5 @@ module add (/*AUTOARG*/ parameter PASSDOWN = 9999; input [31:0] in; output [31:0] out; - wire out = in + PASSDOWN; + assign out = in + PASSDOWN; endmodule diff --git a/test_regress/t/t_var_dup_bad.out b/test_regress/t/t_var_dup_bad.out index 9af8a58c4..a89c6b69d 100644 --- a/test_regress/t/t_var_dup_bad.out +++ b/test_regress/t/t_var_dup_bad.out @@ -35,6 +35,12 @@ t/t_var_dup_bad.v:31:11: ... Location of original declaration 31 | output oi; | ^~ +%Error: t/t_var_dup_bad.v:36:11: Duplicate declaration of signal: 'og' + 36 | reg og; + | ^~ + t/t_var_dup_bad.v:34:11: ... Location of original declaration + 34 | output og; + | ^~ %Error: t/t_var_dup_bad.v:39:15: Duplicate declaration of signal: 'org' 39 | output reg org; | ^~~