Fix package import preventing local var, bug599.

Changes

@@ -17,6 +17,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 ****  Fix package import of package imports, partial bug592.  [Jeremy Bennett]
 
+****  Fix package import preventing local var, bug599.  [Jeremy Bennett]
+
 
 * Verilator 3.843 2012/12/01
 

src/V3LinkDot.cpp

@@ -153,6 +153,8 @@ public:
 	    // Not found, will add in a moment.
 	} else if (nodep==fnodep) {  // Already inserted.
 	    // Good.
+	} else if (foundp->imported()) {  // From package
+	    // We don't throw VARHIDDEN as if the import is later the symbol table's import wouldn't warn
 	} else if (nodep->castBegin() && fnodep->castBegin()
 		   && nodep->castBegin()->generate()) {
 	    // Begin: ... blocks often replicate under genif/genfor, so simply suppress duplicate checks
@@ -262,7 +264,7 @@ public:
 	symp->fallbackp(abovep);
 	nodep->user1p(symp);
 	checkDuplicate(abovep, nodep, name);
-	abovep->insert(name, symp);
+	abovep->reinsert(name, symp);
 	return symp;
     }
     static bool existsModScope(AstNodeModule* nodep) {
@@ -669,7 +671,8 @@ private:
 	    } else if (findvarp != nodep) {
 		UINFO(4,"DupVar: "<<nodep<<" ;; "<<foundp->nodep()<<endl);
 		UINFO(4,"    found  cur=se"<<(void*)m_curSymp<<" ;; parent=se"<<(void*)foundp->parentp()<<endl);
-		if (foundp && foundp->parentp() == m_curSymp) {  // Only when on same level
+		if (foundp && foundp->parentp() == m_curSymp  // Only when on same level
+		    && !foundp->imported()) {  // and not from package
 		    if ((findvarp->isIO() && nodep->isSignal())
 			|| (findvarp->isSignal() && nodep->isIO())) {
 			findvarp->combineType(nodep);
@@ -732,7 +735,8 @@ private:
 	    ins=true;
 	} else if (findvarp != nodep) {
 	    UINFO(4,"DupVar: "<<nodep<<" ;; "<<foundp<<endl);
-	    if (foundp && foundp->parentp() == m_curSymp) {  // Only when on same level
+	    if (foundp && foundp->parentp() == m_curSymp  // Only when on same level
+		&& !foundp->imported()) {  // and not from package
 		nodep->v3error("Duplicate declaration of enum value: "<<nodep->prettyName()<<endl
 			       <<findvarp->warnMore()<<"... Location of original declaration");
 	    } else {
@@ -759,7 +763,7 @@ private:
 	    }
 	}
 	m_curSymp->import(m_statep->symsp(), srcp, nodep->name());
-	UINFO(2,"    Link Done: "<<nodep<<endl);
+	UINFO(9,"    Link Done: "<<nodep<<endl);
 	// No longer needed, but can't delete until any multi-instantiated modules are expanded
     }
 

src/V3SymTable.h

@@ -152,6 +152,20 @@ public:
 	if (m_fallbackp) return m_fallbackp->findIdFallback(name);
 	return NULL;
     }
+private:
+    bool importOneSymbol(VSymGraph* graphp, const string& name, const VSymEnt* srcp) {
+	if (srcp->exported()
+	    && !findIdFlat(name)) {  // Don't insert over existing entry
+	    VSymEnt* symp = new VSymEnt(graphp, srcp);
+	    symp->exported(false);  // Can't reimport an import without an export
+	    symp->imported(true);
+	    reinsert(name, symp);
+	    return true;
+	} else {
+	    return false;
+	}
+    }
+public:
     bool import(VSymGraph* graphp, const VSymEnt* srcp, const string& id_or_star) {
 	// Import tokens from source symbol table into this symbol table
 	// Returns true if successful
@@ -159,21 +173,12 @@ public:
 	if (id_or_star != "*") {
 	    IdNameMap::const_iterator it = srcp->m_idNameMap.find(id_or_star);
 	    if (it != m_idNameMap.end()) {
-		if (it->second->exported()) {
-		    VSymEnt* symp = new VSymEnt(graphp, it->second);
-		    symp->exported(false);  // Can't reimport an import without an export
-		    reinsert(it->first, symp);
-		}
+		importOneSymbol(graphp, it->first, it->second);
 	    }
 	    any = true;  // Legal, though perhaps lint questionable to import nothing
 	} else {
 	    for (IdNameMap::const_iterator it=srcp->m_idNameMap.begin(); it!=srcp->m_idNameMap.end(); ++it) {
-		if (it->second->exported()) {
-		    VSymEnt* symp = new VSymEnt(graphp, it->second);
-		    symp->exported(false);  // Can't reimport an import without an export
-		    reinsert(it->first, symp);
-		    any = true;
-		}
+		if (importOneSymbol(graphp, it->first, it->second)) any = true;
 	    }
 	}
 	return any;

test_regress/t/t_package_param.pl

@@ -0,0 +1,18 @@
+#!/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
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+compile (
+    );
+
+execute (
+    check_finished=>1,
+    );
+
+ok(1);
+1;

test_regress/t/t_package_param.v

@@ -0,0 +1,38 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// IEEE 1800-2009 requires that any local definitions take precedence over
+// definitions in wildcard imported packages (section 26.3). Thus the code
+// below is valid SystemVerilog.
+//
+// This file ONLY is placed into the Public Domain, for any use, without
+// warranty, 2013 by Jie Xu
+
+package defs;
+   parameter NUMBER = 8;
+   localparam NUM = NUMBER;
+endpackage
+
+
+module t(/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+   import defs::*;
+
+   // This also fails if we use localparam
+   parameter NUM = 32;
+
+   // Check we have the right definition
+   always @(posedge clk) begin
+      if (NUM == 32) begin
+	 $write("*-* All Finished *-*\n");
+	 $finish;
+      end
+      else begin
+	 $stop;
+      end
+   end
+
+endmodule