Fix checking for parameter and port connections in the wrong place (#4428)

src/V3Ast.h

@@ -816,6 +816,7 @@ public:
             "BLOCKTEMP", "MODULETEMP", "STMTTEMP",     "XTEMP",   "IFACEREF", "MEMBER"};
         return names[m_e];
     }
+    bool isParam() const { return m_e == GPARAM || m_e == LPARAM; }
     bool isSignal() const {
         return (m_e == WIRE || m_e == WREAL || m_e == IMPLICITWIRE || m_e == TRIWIRE || m_e == TRI0
                 || m_e == TRI1 || m_e == PORT || m_e == SUPPLY0 || m_e == SUPPLY1 || m_e == VAR);

src/V3AstNodeOther.h

@@ -1922,9 +1922,7 @@ public:
     bool isClassMember() const { return varType() == VVarType::MEMBER; }
     bool isStatementTemp() const { return (varType() == VVarType::STMTTEMP); }
     bool isXTemp() const { return (varType() == VVarType::XTEMP); }
-    bool isParam() const VL_MT_SAFE {
-        return (varType() == VVarType::LPARAM || varType() == VVarType::GPARAM);
-    }
+    bool isParam() const { return varType().isParam(); }
     bool isGParam() const { return (varType() == VVarType::GPARAM); }
     bool isGenVar() const { return (varType() == VVarType::GENVAR); }
     bool isBitLogic() const {

src/V3EmitCImp.cpp

@@ -677,7 +677,7 @@ class EmitCTrace final : EmitCFunc {
             string fstvt;
             // Doubles have special decoding properties, so must indicate if a double
             if (nodep->dtypep()->basicp()->isDouble()) {
-                if (vartype == VVarType::GPARAM || vartype == VVarType::LPARAM) {
+                if (vartype.isParam()) {
                     fstvt = "FST_VT_VCD_REAL_PARAMETER";
                 } else {
                     fstvt = "FST_VT_VCD_REAL";

src/V3LinkDot.cpp

@@ -1616,6 +1616,7 @@ private:
             AstPin* const pinp = new AstPin{nodep->fileline(),
                                             -1,  // Pin# not relevant
                                             nodep->name(), exprp};
+            pinp->param(true);
             cellp->addParamsp(pinp);
         }
         VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
@@ -2250,6 +2251,11 @@ private:
         if (classp->isInterfaceClass()) importImplementsClass(nodep, srcp, classp);
         if (!cextp->isImplements()) m_curSymp->importFromClass(m_statep->symsp(), srcp);
     }
+    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();
+    }
 
     // VISITs
     void visit(AstNetlist* nodep) override {
@@ -2344,7 +2350,32 @@ private:
             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";
-            if (!foundp) {
+            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) {
                 if (nodep->name() == "__paramNumber1" && m_cellp
                     && VN_IS(m_cellp->modp(), Primitive)) {
                     // Primitive parameter is really a delay we can just ignore
@@ -2360,19 +2391,6 @@ private:
                               ucfirst(whatp)
                                   << " not found: " << nodep->prettyNameQ() << '\n'
                                   << (suggest.empty() ? "" : nodep->warnMore() + suggest));
-            } else 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 {
-                    nodep->modVarp(refp);
-                    markAndCheckPinDup(nodep, refp, whatp);
-                }
-            } else if (AstParamTypeDType* const refp = VN_CAST(foundp->nodep(), ParamTypeDType)) {
-                nodep->modPTypep(refp);
-                markAndCheckPinDup(nodep, refp, whatp);
-            } else {
-                nodep->v3error(ucfirst(whatp) << " not found: " << nodep->prettyNameQ());
             }
         }
         // Early return() above when deleted

test_regress/t/t_inst_pin_place_bad.out

@@ -0,0 +1,12 @@
+%Warning-PINMISSING: t/t_inst_pin_place_bad.v:21:7: Cell has missing pin: 'pin_1'
+   21 |     ) i_sub (
+      |       ^~~~~
+                     ... 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'
+   22 |         .PARAM_A(1)
+      |          ^~~~~~~
+%Error-PINNOTFOUND: t/t_inst_pin_place_bad.v:20:10: Parameter pin not found: 'pin_1'
+   20 |         .pin_1(1)
+      |          ^~~~~
+%Error: Exiting due to

test_regress/t/t_inst_pin_place_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;

test_regress/t/t_inst_pin_place_bad.v

@@ -0,0 +1,24 @@
+// 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 # (
+    parameter PARAM_A = 1,
+    parameter type PARAM_B = logic
+) (
+    input pin_1
+);
+endmodule
+
+module t;
+    parameter type PARAM_B = string;
+
+    sub #(
+        .PARAM_B(PARAM_B),
+        .pin_1(1)
+    ) i_sub (
+        .PARAM_A(1)
+    );
+endmodule