Add -Wwarn-style, -Wno-style, and DEFPARAM warnings

Changes

@@ -7,12 +7,17 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **   Add limited support for VPI access to public signals, see docs.
 
+**   Add -Wwarn-style, -Wno-style to enable code style warnings that are
+     added to this release and disabled by default.
+
 ***  Add -F option to read relative option files, bug297. [Neil Hamilton]
 
 ***  Support ++,--,+= etc as standalone statements.  [Alex Solomatnikov]
 
 ***  Suppress WIDTH warnings when adding/subtracting 1'b1.
 
+***  Add optional DEFPARAM warning to find deprecated defparam statements.
+
 **** When running with VERILATOR_ROOT, optionally find binaries under bin.
 
 * Verilator 3.805 2010/11/02

bin/verilator

@@ -301,6 +301,7 @@ descriptions in the next sections for more information.
      -Wfuture-<message>         Disable unknown message warnings
      -Wno-<message>             Disable warning
      -Wno-lint                  Disable all lint warnings
+     -Wno-style                 Disable all style warnings
      -x-assign <mode>           Initially assign Xs to this value
      -y <dir>                   Directory to search for modules
 
@@ -608,6 +609,9 @@ fairly standard across Verilog tools.
 
 Check the files for lint violations only, do not create any other output.
 
+You may also want the -Wwarn-style option to enable messages that are
+considered stylistic and not enabled by default.
+
 If the design is not to be completely Verilated see also the --bbox-sys and
 --bbox-unsup options.
 
@@ -855,19 +859,38 @@ Disable the specified warning message.
 
 =item -Wno-lint
 
-Disable all lint related warning messages.  This is equivalent to
+Disable all lint related warning messages, and all style warnings.  This is
-"-Wno-CASEINCOMPLETE -Wno-CASEOVERLAP -Wno-CASEX -Wno-CASEWITHX
+equivalent to "-Wno-CASEINCOMPLETE -Wno-CASEOVERLAP -Wno-CASEX
--Wno-CMPCONST -Wno-IMPLICIT -Wno-LITENDIAN -Wno-UNDRIVEN -Wno-UNSIGNED
+-Wno-CASEWITHX -Wno-CMPCONST -Wno-IMPLICIT -Wno-LITENDIAN -Wno-UNDRIVEN
--Wno-UNUSED -Wno-VARHIDDEN -Wno-WIDTH".
+-Wno-UNSIGNED -Wno-UNUSED -Wno-VARHIDDEN -Wno-WIDTH" plus the list shown
+for Wno-style.
 
 It is strongly recommended you cleanup your code rather than using this
 option, it is only intended to be use when running test-cases of code
 received from third parties.
 
+=item -Wno-style
+
+Disable all code style related warning messages (note by default they are already disabled).  This is equivalent to
+"-Wno-DEFPARAM".
+
 =item -Wwarn-I<message>
 
 Enables the specified warning message.
 
+=item -Wwarn-lint
+
+Enable all lint related warning messages (note by default they are already
+enabled), but do not affect style messages.  This is equivalent to
+"-Wwarn-CASEINCOMPLETE -Wwarn-CASEOVERLAP -Wwarn-CASEX -Wwarn-CASEWITHX
+-Wwarn-CMPCONST -Wwarn-IMPLICIT -Wwarn-LITENDIAN -Wwarn-UNDRIVEN
+-Wwarn-UNSIGNED -Wwarn-UNUSED -Wwarn-VARHIDDEN -Wwarn-WIDTH".
+
+=item -Wwarn-style
+
+Enable all code style related warning messages.  This is equivalent to
+"-WDEFPARAM".
+
 =item -x-assign 0
 
 =item -x-assign 1
@@ -2360,6 +2383,14 @@ L<http://www.sunburst-design.com/papers/CummingsSNUG2000SJ_NBA_rev1_2.pdf>
 Ignoring this warning may make Verilator simulations differ from other
 simulators.
 
+=item DEFPARAM
+
+Warns that the "defparam" statement was deprecated in Verilog 2001 and all
+designs should now be using the #(...) format to specify parameters.
+
+Disabled by default as this is a code style warning, it will simulate
+correctly.
+
 =item GENCLK
 
 Warns that the specified signal is generated, but is also being used as a

src/V3Error.cpp

@@ -134,6 +134,13 @@ void FileLine::warnLintOff(bool flag) {
     }
 }
 
+void FileLine::warnStyleOff(bool flag) {
+    for (int codei=V3ErrorCode::EC_FIRST_WARN; codei<V3ErrorCode::_ENUM_MAX; codei++) {
+	V3ErrorCode code = (V3ErrorCode)codei;
+	if (code.styleError()) warnOff(code, flag);
+    }
+}
+
 FileLine* FileLine::copyOrSameFileLine() {
     // When a fileline is "used" to produce a node, calls this function.
     // Return this, or a copy of this

src/V3Error.h

@@ -49,6 +49,7 @@ public:
 	E_MULTITOP,	// Error: Multiple top level modules
 	E_TASKNSVAR,	// Error: Task I/O not simple
 	E_BLKLOOPINIT,	// Error: Delayed assignment to array inside for loops
+	//
 	// Warning codes:
 	EC_FIRST_WARN,	// Just a code so the program knows where to start warnings
 	//
@@ -60,6 +61,7 @@ public:
 	CDCRSTLOGIC,	// Logic in async reset path
 	CMPCONST,	// Comparison is constant due to limited range
 	COMBDLY,	// Combinatorial delayed assignment
+	DEFPARAM,	// Style: Defparam
 	STMTDLY,	// Delayed statement
 	SYMRSVDWORD,	// Symbol is Reserved Word
 	GENCLK,		// Generated Clock
@@ -96,10 +98,11 @@ public:
 	    // Errors
 	    "MULTITOP", "TASKNSVAR", "BLKLOOPINIT",
 	    // Warnings
-	    " FIRST_WARN",
+	    " EC_FIRST_WARN",
 	    "BLKANDNBLK",
 	    "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CMPCONST",
-	    "COMBDLY", "STMTDLY", "SYMRSVDWORD", "GENCLK", "IMPERFECTSCH", "IMPLICIT", "IMPURE",
+	    "COMBDLY", "DEFPARAM",
+	    "STMTDLY", "SYMRSVDWORD", "GENCLK", "IMPERFECTSCH", "IMPLICIT", "IMPURE",
 	    "LITENDIAN", "MODDUP",
 	    "MULTIDRIVEN", "REDEFMACRO",
 	    "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNSIGNED", "UNUSED",
@@ -109,7 +112,7 @@ public:
 	return names[m_e];
     }
     // Warnings that default to off
-    bool defaultsOff() const { return ( m_e==IMPERFECTSCH ); }
+    bool defaultsOff() const { return ( m_e==IMPERFECTSCH || styleError()); }
     // Warnings that warn about nasty side effects
     bool dangerous() const { return ( m_e==COMBDLY ); }
     // Warnings we'll present to the user as errors
@@ -127,6 +130,8 @@ public:
 				      || m_e==UNDRIVEN || m_e==UNSIGNED
 				      || m_e==UNUSED || m_e==VARHIDDEN
 				      || m_e==WIDTH); }
+    // Warnings that are style only
+    bool styleError() const { return ( m_e==DEFPARAM); }
   };
   inline bool operator== (V3ErrorCode lhs, V3ErrorCode rhs) { return (lhs.m_e == rhs.m_e); }
   inline bool operator== (V3ErrorCode lhs, V3ErrorCode::en rhs) { return (lhs.m_e == rhs); }
@@ -265,6 +270,7 @@ public:
     bool warnOff(const string& code, bool flag);  // Returns 1 if ok
     bool warnIsOff(V3ErrorCode code) const;
     void warnLintOff(bool flag);
+    void warnStyleOff(bool flag);
     void warnStateFrom(const FileLine& from) { m_warnOn=from.m_warnOn; }
     void warnResetDefault() { warnStateFrom(s_defaultFileLine); }
 
@@ -276,6 +282,7 @@ public:
 
     // METHODS - Global
     static void globalWarnLintOff(bool flag) { s_defaultFileLine.warnLintOff(flag); }
+    static void globalWarnStyleOff(bool flag) { s_defaultFileLine.warnStyleOff(flag); }
     static void globalWarnOff(V3ErrorCode code, bool flag) { s_defaultFileLine.warnOff(code, flag); }
     static bool globalWarnOff(const string& code, bool flag) { return s_defaultFileLine.warnOff(code, flag); }
 

src/V3Link.cpp

@@ -659,6 +659,7 @@ private:
     virtual void visit(AstDefParam* nodep, AstNUser*) {
 	nodep->iterateChildren(*this);
 	if (m_idState==ID_PARAM) {
+	    nodep->v3warn(DEFPARAM,"Suggest replace defparam with Verilog 2001 #(."<<nodep->name()<<"(...etc...))");
 	    AstNode* foundp = m_curVarsp->findIdUpward(nodep->path());
 	    AstCell* cellp = foundp->castCell();
 	    if (!cellp) {

src/V3Options.cpp

@@ -791,6 +791,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char
 	    else if ( !strncmp (sw, "-Wno-",5) )	{
 		if (!strcmp (sw, "-Wno-lint")) {
 		    FileLine::globalWarnLintOff(true);
+		    FileLine::globalWarnStyleOff(true);
+		}
+		else if (!strcmp (sw, "-Wno-style")) {
+		    FileLine::globalWarnStyleOff(true);
 		}
 		else {
 		    string msg = sw+strlen("-Wno-");
@@ -803,6 +807,9 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char
 		if (!strcmp (sw, "-Wwarn-lint")) {
 		    FileLine::globalWarnLintOff(false);
 		}
+		else if (!strcmp (sw, "-Wwarn-style")) {
+		    FileLine::globalWarnStyleOff(false);
+		}
 		else {
 		    string msg = sw+strlen("-Wwarn-");
 		    if (!(FileLine::globalWarnOff(msg, false))) {

test_regress/t/t_lint_defparam.pl

@@ -0,0 +1,16 @@
+#!/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 (
+	 v_flags2 => ["--lint-only"],
+	 ) if $Self->{v3};
+
+ok(1);
+1;
+

test_regress/t/t_lint_defparam.v

@@ -0,0 +1,15 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2010 by Wilson Snyder.
+
+module t;
+
+   sub sub ();
+   defparam sub.P = 2;
+
+endmodule
+
+module sub;
+   parameter P = 6;
+endmodule

test_regress/t/t_lint_defparam_bad.pl

@@ -0,0 +1,23 @@
+#!/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.
+
+top_filename("t/t_lint_defparam.v");
+
+compile (
+	 v_flags2 => ["--lint-only -Wwarn-style"],
+	 fails=>1,
+	 expect=>
+'%Warning-DEFPARAM: t/t_lint_defparam.v:\d+: Suggest replace defparam with Verilog 2001 #\(.P\(...etc...\)\)
+%Warning-DEFPARAM: Use .* to disable this message.
+%Error: Exiting due to.*',
+	 ) if $Self->{v3};
+
+ok(1);
+1;
+