From ea75290f65b8e38ee75536855fd169ea2d0dbdbc Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Mon, 7 Mar 2011 20:44:19 -0500
Subject: [PATCH] Add error on circular parameter definitions, bug329

---
 Changes                            |  2 ++
 src/V3AstNodes.h                   |  7 +++++++
 src/V3Signed.cpp                   |  3 +++
 src/V3Width.cpp                    | 13 ++++++++++++-
 test_regress/t/t_param_circ_bad.pl | 22 ++++++++++++++++++++++
 test_regress/t/t_param_circ_bad.v  | 12 ++++++++++++
 6 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100755 test_regress/t/t_param_circ_bad.pl
 create mode 100644 test_regress/t/t_param_circ_bad.v

diff --git a/Changes b/Changes
index 2fddd295d..54c4429d6 100644
--- a/Changes
+++ b/Changes
@@ -11,6 +11,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **** Accelerate bit-selected inversions.
 
+**** Add error on circular parameter definitions, bug329. [Alex Solomatnikov]
+
 **** Fix concatenates and vectored bufif1, bug326. [Iztok Jeras]
 
 * Verilator 3.811 2011/02/14
diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h
index 85d18f13f..83f07aa24 100644
--- a/src/V3AstNodes.h
+++ b/src/V3AstNodes.h
@@ -559,6 +559,8 @@ private:
     bool	m_attrClockEn:1;// User clock enable attribute
     bool	m_attrIsolateAssign:1;// User isolate_assignments attribute
     bool	m_attrSFormat:1;// User sformat attribute
+    bool	m_didSigning:1;	// V3Signed completed; can skip iteration
+    bool	m_didWidth:1;	// V3Width completed; can skip iteration
     bool	m_fileDescr:1;	// File descriptor
     bool	m_isConst:1;	// Table contains constant data
     bool	m_isStatic:1;	// Static variable
@@ -572,6 +574,7 @@ private:
 	m_sigPublic=false; m_sigModPublic=false; m_sigUserRdPublic=false; m_sigUserRWPublic=false;
 	m_funcLocal=false; m_funcReturn=false;
 	m_attrClockEn=false; m_attrIsolateAssign=false; m_attrSFormat=false;
+	m_didSigning=false; m_didWidth=false;
 	m_fileDescr=false; m_isConst=false; m_isStatic=false;
 	m_trace=false;
     }
@@ -634,6 +637,10 @@ public:
     void	attrScClocked(bool flag) { m_scClocked = flag; }
     void	attrIsolateAssign(bool flag) { m_attrIsolateAssign = flag; }
     void	attrSFormat(bool flag) { m_attrSFormat = flag; }
+    void	didSigning(bool flag) { m_didSigning=flag; }
+    bool	didSigning() const { return m_didSigning; }
+    void	didWidth(bool flag) { m_didWidth=flag; }
+    bool	didWidth() const { return m_didWidth; }
     void	usedClock(bool flag) { m_usedClock = flag; }
     void	usedParam(bool flag) { m_usedParam = flag; }
     void	usedLoopIdx(bool flag) { m_usedLoopIdx = flag; }
diff --git a/src/V3Signed.cpp b/src/V3Signed.cpp
index 49f224ffb..533a86ee8 100644
--- a/src/V3Signed.cpp
+++ b/src/V3Signed.cpp
@@ -149,6 +149,9 @@ private:
 
     // Inherit from others
     virtual void visit(AstVar* nodep, AstNUser*) {
+	// Avoid recursion; can't use user() as they're all full, and anyhow this is often called
+	if (nodep->didSigning()) return;
+	nodep->didSigning(true);
 	nodep->iterateChildren(*this);
 	nodep->signedFrom(nodep->dtypep());
     }
diff --git a/src/V3Width.cpp b/src/V3Width.cpp
index 3e7b30398..443536a41 100644
--- a/src/V3Width.cpp
+++ b/src/V3Width.cpp
@@ -528,7 +528,18 @@ private:
 	//if (debug()) nodep->dumpTree(cout,"  InitPre: ");
 	// Must have deterministic constant width
 	// We can't skip this step when width()!=0, as creating a AstVar
-	// with non-constant range gets size 1, not size 0.
+	// with non-constant range gets size 1, not size 0.  So use didWidth().
+	if (nodep->didWidth()) {  // Early exit if have circular parameter definition
+	    if (!nodep->width()) {
+		if (!nodep->valuep()) nodep->v3fatalSrc("circular, but without value");
+		nodep->v3error("Variable's initial value is circular: "<<nodep->prettyName());
+		pushDeletep(nodep->valuep()->unlinkFrBack());
+		nodep->valuep(new AstConst(nodep->fileline(), 1));
+	    } else {
+		return;
+	    }
+	}
+	nodep->didWidth(true);
 	int width=1; int mwidth=1;
 	// Parameters if implicit untyped inherit from what they are assigned to
 	AstBasicDType* bdtypep = nodep->dtypep()->castBasicDType();
diff --git a/test_regress/t/t_param_circ_bad.pl b/test_regress/t/t_param_circ_bad.pl
new file mode 100755
index 000000000..29c96cd94
--- /dev/null
+++ b/test_regress/t/t_param_circ_bad.pl
@@ -0,0 +1,22 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2008 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 (
+    v_flags2 => ["--lint-only"],
+    fails=>1,
+    verilator_make_gcc => 0,
+    make_top_shell => 0,
+    make_main => 0,
+    expect=>
+'%Error: t/t_param_circ_bad.v:\d+: Variable\'s initial value is circular: X
+%Error: Exiting due to.*',
+    ) if $Self->{v3};
+
+ok(1);
+1;
diff --git a/test_regress/t/t_param_circ_bad.v b/test_regress/t/t_param_circ_bad.v
new file mode 100644
index 000000000..0775dc467
--- /dev/null
+++ b/test_regress/t/t_param_circ_bad.v
@@ -0,0 +1,12 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2011 by Wilson Snyder.
+
+module t (/*AUTOARG*/);
+   sub sub ();
+endmodule
+
+module sub #(parameter WIDTH=X, parameter X=WIDTH)
+   ();
+endmodule