diff --git a/Changes b/Changes index 97b65dc0d..5a14c5bb9 100644 --- a/Changes +++ b/Changes @@ -5,7 +5,7 @@ indicates the contributor was also the author of the fix; Thanks! * Verilator 3.81*** -**** Report errors on duplicated pins, bug321. [Christian Leber] +**** Report errors on duplicated or empty pins, bug321. [Christian Leber] **** Throw UNUSED/UNDRIVEN only once per net in a parametrized module. diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index b7764abac..3fab16bff 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -921,7 +921,7 @@ public: bool dotStar() const { return name() == ".*"; } // Special fake name for .* connections until linked int pinNum() const { return m_pinNum; } void exprp(AstNode* nodep) { addOp1p(nodep); } - AstNode* exprp() const { return op1p()->castNode(); } // op1 = Expression connected to pin + AstNode* exprp() const { return op1p()->castNode(); } // op1 = Expression connected to pin, NULL if unconnected AstVar* modVarp() const { return m_modVarp; } // [After Link] Pointer to variable void modVarp(AstVar* varp) { m_modVarp=varp; } bool svImplicit() const { return m_svImplicit; } diff --git a/src/V3Link.cpp b/src/V3Link.cpp index 72152f781..12c336c7a 100644 --- a/src/V3Link.cpp +++ b/src/V3Link.cpp @@ -645,10 +645,19 @@ private: refp->user5p(nodep); } } + if (!nodep->exprp()) { + // It's an empty pin connection, done with it. + // (We used to not create pins for these, but we'd miss + // warns. Perhaps they should live even further...) + pushDeletep(nodep->unlinkFrBack()); nodep=NULL; + return; + } nodep->iterateChildren(*this); } // Deal with implicit definitions - do before ID_RESOLVE stage as may be referenced above declaration - if (m_idState==ID_PARAM && !nodep->svImplicit()) { // SV 19.11.3: .name pins don't allow implicit decls + if (m_idState==ID_PARAM + && nodep->exprp() // Else specifically unconnected pin + && !nodep->svImplicit()) { // SV 19.11.3: .name pins don't allow implicit decls pinImplicitExprRecurse(nodep->exprp()); } // Early return() above when deleted diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index af5a612f0..edbe0d55d 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -193,6 +193,18 @@ private: new V3GraphEdge(&m_graph, vertex(m_modp), vertex(modp), 1, false); } } + // Remove AstCell(AstPin("",NULL)), it's a side effect of how we parse "()" + // the empty middle is identical to the empty rule that must find pins in "(,)". + if (nodep->pinsp() && !nodep->pinsp()->nextp() + && nodep->pinsp()->name() == "" + && !nodep->pinsp()->exprp()) { + pushDeletep(nodep->pinsp()->unlinkFrBackWithNext()); + } + if (nodep->paramsp() && !nodep->paramsp()->nextp() + && nodep->paramsp()->name() == "" + && !nodep->paramsp()->exprp()) { + pushDeletep(nodep->paramsp()->unlinkFrBackWithNext()); + } // Convert .* to list of pins bool pinStar = false; for (AstPin* nextp, *pinp = nodep->pinsp(); pinp; pinp=nextp) { diff --git a/src/verilog.y b/src/verilog.y index 7c84e80f7..a4ec2dc37 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1667,11 +1667,12 @@ cellpinItList: // IEEE: list_of_port_connections + list_of_parameter_assi ; cellpinItemE: // IEEE: named_port_connection + named_parameter_assignment + empty - /* empty: ',,' is legal */ { $$ = NULL; PINNUMINC(); } + // Note empty can match either () or (,); V3LinkCells cleans up () + /* empty: ',,' is legal */ { $$ = new AstPin(CRELINE(),PINNUMINC(),"",NULL); } | yP_DOTSTAR { $$ = new AstPin($1,PINNUMINC(),".*",NULL); } - | '.' idSVKwd { $$ = NULL; PINNUMINC(); } + | '.' idSVKwd { $$ = new AstPin($1,PINNUMINC(),*$2,new AstVarRef($1,*$2,false)); $$->svImplicit(true);} | '.' idAny { $$ = new AstPin($1,PINNUMINC(),*$2,new AstVarRef($1,*$2,false)); $$->svImplicit(true);} - | '.' idAny '(' ')' { $$ = NULL; PINNUMINC(); } + | '.' idAny '(' ')' { $$ = new AstPin($1,PINNUMINC(),*$2,NULL); } // // mintypmax is expanded here, as it might be a UDP or gate primitive | '.' idAny '(' expr ')' { $$ = new AstPin($1,PINNUMINC(),*$2,$4); } //UNSUP '.' idAny '(' expr ':' expr ')' { } diff --git a/test_regress/t/t_lint_pindup_bad.pl b/test_regress/t/t_lint_pindup_bad.pl index 4c453fad1..d0bd790ab 100755 --- a/test_regress/t/t_lint_pindup_bad.pl +++ b/test_regress/t/t_lint_pindup_bad.pl @@ -16,6 +16,9 @@ compile ( expect=> '%Error: t/t_lint_pindup_bad.v:\d+: Duplicate pin connection: i %Error: t/t_lint_pindup_bad.v:\d+: ... Location of original pin connection +%Error: t/t_lint_pindup_bad.v:\d+: Pin not found: __pinNumber4 +%Error: t/t_lint_pindup_bad.v:\d+: Duplicate pin connection: P +%Error: t/t_lint_pindup_bad.v:\d+: ... Location of original pin connection %Error: t/t_lint_pindup_bad.v:\d+: Duplicate pin connection: P %Error: t/t_lint_pindup_bad.v:\d+: ... Location of original pin connection %Error: Exiting due to.*', diff --git a/test_regress/t/t_lint_pindup_bad.v b/test_regress/t/t_lint_pindup_bad.v index 167627b99..6a8779be1 100644 --- a/test_regress/t/t_lint_pindup_bad.v +++ b/test_regress/t/t_lint_pindup_bad.v @@ -11,7 +11,7 @@ module t ); sub - #(.P(2), .P(3)) + #(, .P(2), .P(3)) sub (.o(o), .i(i), .i(i2),