diff --git a/Changes b/Changes index b97cde980..14e4a42aa 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,8 @@ indicates the contributor was also the author of the fix; Thanks! *** Add --pins-bv option to use sc_bv for all ports. [Brian Small] +**** Report errors when duplicate labels are used, bug72. [Vasu Kandadi] + **** Fix the SC_MODULE name() to not include __PVT__. [Bob Fredieu] * Verilator 3.701 2009/02/26 diff --git a/src/V3Link.cpp b/src/V3Link.cpp index 9a4ddd61d..9176d0360 100644 --- a/src/V3Link.cpp +++ b/src/V3Link.cpp @@ -29,6 +29,7 @@ #include "verilatedos.h" #include #include +#include #include #include #include @@ -49,10 +50,11 @@ private: // AstModule::user1p() // V3SymTable* Module's Symbol table // AstNodeFTask::user1p() // V3SymTable* Local Symbol table // AstBegin::user1p() // V3SymTable* Local Symbol table - // AstVar::user1p() // V3SymTable* Table used to create this variable // AstVar::user2p() // bool True if port set for this variable + // AstVar/Module::user3p() // V3SymTable* Table used to create this variable AstUser1InUse m_inuser1; AstUser2InUse m_inuser2; + AstUser3InUse m_inuser3; // ENUMS enum IdState { // Which loop through the tree @@ -69,6 +71,7 @@ private: V3SymTable* m_curVarsp; // Symbol table of variables and tasks under table we're inserting into V3SymTable* m_cellVarsp; // Symbol table of variables under cell's module int m_beginNum; // Begin block number, 0=none seen + bool m_inGenerate; // Inside a generate vector m_delSymps; // Symbol tables to delete static int debug() { @@ -90,6 +93,12 @@ private: } } + void symsInsert(const string& name, AstNode* nodep) { + // Insert into symbol table, and remember what table the node is in + m_curVarsp->insert(name, nodep); + nodep->user3p(m_curVarsp); + } + void linkVarName (AstVarRef* nodep) { if (!nodep->varp()) { AstVar* varp = m_curVarsp->findIdName(nodep->name())->castVar(); @@ -103,9 +112,43 @@ private: else if (nodep->castCell()) what = "cell"; else if (nodep->castTask()) what = "task"; else if (nodep->castFunc()) what = "function"; + else if (nodep->castBegin()) what = "block"; return what; } + string ucfirst(const string& text) { + string out = text; + out[0] = toupper(out[0]); + return out; + } + + void checkDuplicate(AstNode* nodep, AstNode* foundp) { + // Check if there's a duplicate and report error if so + V3SymTable* nodeSymsp = (V3SymTable*)nodep->user3p(); + V3SymTable* foundSymsp = (V3SymTable*)foundp->user3p(); + // nodeSymsp may not have been set yet because it wasn't inserted yet, + // if so, pretend for a moment it is inserted right here. + if (!nodeSymsp) nodeSymsp = m_curVarsp; + // + if (nodep==foundp) { // Good. + } else if ((nodep->castBegin() || foundp->castBegin()) + && nodeSymsp != foundSymsp) { + // Conflicts with begin at *different* level. Thus it isn't really a duplicate + // if "foo" is under a block called "foo" - it's "foo.foo"; that's fine. + } else if ((nodep->castBegin() || foundp->castBegin()) + && m_inGenerate) { + // Begin: ... blocks often replicate under genif/genfor, so simply suppress duplicate checks + // See t_gen_forif.v for an example. + } else if (nodep->type() == foundp->type()) { + nodep->v3error("Duplicate declaration of "<prettyName()); + foundp->v3error("... Location of original declaration"); + } else { + nodep->v3error("Unsupported in C: "<prettyName()); + foundp->v3error("... Location of original declaration"); + } + } + void createImplicitVar (AstVarRef* forrefp, bool noWarn) { // Create implicit after warning linkVarName(forrefp); @@ -154,6 +197,18 @@ private: m_curVarsp = NULL; m_modp = NULL; } + + virtual void visit(AstGenerate* nodep, AstNUser*) { + // Begin: ... blocks often replicate under genif/genfor, so simply suppress duplicate checks + // See t_gen_forif.v for an example. + bool lastInGen = m_inGenerate; + { + m_inGenerate = true; + nodep->iterateChildren(*this); + } + m_inGenerate = lastInGen; + } + virtual void visit(AstVar* nodep, AstNUser*) { // Var: Remember its name for later resolution if (!m_curVarsp) nodep->v3fatalSrc("Var not under module??\n"); @@ -177,7 +232,7 @@ private: <prettyName()); } else if (findvarp != nodep) { UINFO(4,"DupVar: "<user1p() == m_curVarsp) { // Only when on same level + if (findvarp->user3p() == m_curVarsp) { // Only when on same level if ((findvarp->isIO() && nodep->isSignal()) || (findvarp->isSignal() && nodep->isIO())) { findvarp->combineType(nodep); @@ -198,11 +253,10 @@ private: } } if (ins) { - m_curVarsp->insert(nodep->name(), nodep); - nodep->user1p(m_curVarsp); + symsInsert(nodep->name(), nodep); if (nodep->isGParam()) { m_paramNum++; - m_curVarsp->insert("__paramNumber"+cvtToStr(m_paramNum), nodep); + symsInsert("__paramNumber"+cvtToStr(m_paramNum), nodep); } } } @@ -250,7 +304,7 @@ private: newvarp->attrIsolateAssign(funcp->attrIsolateAssign()); funcp->addFvarp(newvarp); // Explicit insert required, as the var name shadows the upper level's task name - m_curVarsp->insert(newvarp->name(), newvarp); + symsInsert(newvarp->name(), newvarp); } } m_ftaskp = nodep; @@ -260,14 +314,10 @@ private: m_curVarsp = upperVarsp; if (m_idState==ID_FIND) { AstNode* findidp = m_curVarsp->findIdName(nodep->name()); - AstNodeFTask* findtaskp = findidp->castNodeFTask(); if (!findidp) { - m_curVarsp->insert(nodep->name(), nodep); - } else if (!findtaskp) { - nodep->v3error("Unsupported in C: Task/function has same name as " - <prettyName()); - } else if (findtaskp!=nodep) { - nodep->v3error("Duplicate declaration of task/function: "<prettyName()); + symsInsert(nodep->name(), nodep); + } else { + checkDuplicate(nodep,findidp); } } } @@ -282,6 +332,17 @@ private: ++m_beginNum; nodep->name(nodep->name()+cvtToStr(m_beginNum)); } + // Check naming (we don't really care, but some tools do, so better to warn) + if (m_idState==ID_FIND) { + // Note we only check for conflicts at the same level; it's ok if one block hides another + // We also wouldn't want to not insert it even though it's lower down + AstNode* foundp = m_curVarsp->findIdNameThisLevel(nodep->name()); + if (!foundp) { + symsInsert(nodep->name(), nodep); + } else { + checkDuplicate(nodep, foundp); + } + } // Recurse int oldNum = m_beginNum; m_beginNum = 0; @@ -310,14 +371,10 @@ private: if (m_idState==ID_FIND) { // Add to list of all cells, for error checking and defparam's AstNode* findidp = m_curVarsp->findIdName(nodep->name()); - AstCell* findcellp = findidp->castCell(); if (!findidp) { - m_curVarsp->insert(nodep->name(), nodep); - } else if (!findcellp) { - nodep->v3error("Unsupported in C: Cell has same name as " - <prettyName()); - } else if (findcellp != nodep) { - nodep->v3error("Duplicate name of cell: "<prettyName()); + symsInsert(nodep->name(), nodep); + } else { + checkDuplicate(nodep, findidp); } } if (!nodep->modp()) { @@ -348,7 +405,7 @@ private: } else if (!refp->isIO()) { nodep->v3error("Pin is not a in/out/inout: "<prettyName()); } else { - m_curVarsp->insert("__pinNumber"+cvtToStr(nodep->pinNum()), refp); + symsInsert("__pinNumber"+cvtToStr(nodep->pinNum()), refp); refp->user2(true); } // Ports not needed any more @@ -439,6 +496,7 @@ public: m_ftaskp = NULL; m_paramNum = 0; m_beginNum = 0; + m_inGenerate = false; // rootp->accept(*this); } diff --git a/src/V3SymTable.h b/src/V3SymTable.h index 7372bddd4..223f6a85f 100644 --- a/src/V3SymTable.h +++ b/src/V3SymTable.h @@ -56,11 +56,16 @@ class V3SymTable : public AstNUser { m_idNameMap.insert(make_pair(name, nodep)); } } - AstNode* findIdName(const string& name) { + AstNode* findIdNameThisLevel(const string& name) { //UINFO(9, " SymFind "<second); + return NULL; + } + AstNode* findIdName(const string& name) { + // First, scan this begin/end block or module for the name + if (AstNode* nodep = findIdNameThisLevel(name)) return nodep; // Then scan the upper begin/end block or module for the name if (m_upperp) return m_upperp->findIdName(name); return NULL; diff --git a/test_regress/t/t_assert_dup_bad.pl b/test_regress/t/t_assert_dup_bad.pl new file mode 100755 index 000000000..d315bdd6f --- /dev/null +++ b/test_regress/t/t_assert_dup_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# General Public License or the Perl Artistic License. + +compile ( + v_flags2 => ["--lint-only"], + fails=>1, + expect=> +'%Error: t/t_assert_dup_bad.v:\d+: Duplicate declaration of block: covlabel +%Error: t/t_assert_dup_bad.v:\d+: ... Location of original declaration +%Error: Exiting due to.*', + ); + +ok(1); +1; diff --git a/test_regress/t/t_assert_dup_bad.v b/test_regress/t/t_assert_dup_bad.v new file mode 100644 index 000000000..6edaa221c --- /dev/null +++ b/test_regress/t/t_assert_dup_bad.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2007 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + integer cyc = 0; + + covlabel: + cover property (@(posedge clk) cyc==5); + covlabel: // Error: Duplicate block_identifier + cover property (@(posedge clk) cyc==5); + +endmodule diff --git a/test_regress/t/t_var_bad_sameas.pl b/test_regress/t/t_var_bad_sameas.pl index 7fc14aaab..55666b350 100755 --- a/test_regress/t/t_var_bad_sameas.pl +++ b/test_regress/t/t_var_bad_sameas.pl @@ -9,12 +9,16 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di compile ( fails=>1, expect=> -'%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Cell has same name as variable: varfirst -%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Task/function has same name as variable: varfirst +'%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Cell has the same name as variable: varfirst +%Error: t/t_var_bad_sameas.v:\d+: ... Location of original declaration +%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Task has the same name as variable: varfirst +%Error: t/t_var_bad_sameas.v:\d+: ... Location of original declaration %Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Variable has same name as cell: cellfirst -%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Task/function has same name as cell: cellfirst +%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Task has the same name as cell: cellfirst +%Error: t/t_var_bad_sameas.v:\d+: ... Location of original declaration %Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Variable has same name as task: taskfirst -%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Cell has same name as task: taskfirst +%Error: t/t_var_bad_sameas.v:\d+: Unsupported in C: Cell has the same name as task: taskfirst +%Error: t/t_var_bad_sameas.v:\d+: ... Location of original declaration %Error: Exiting due to.*', );