From 5cc11839b5b7d599372ba0957ee6aac8a2fd1d94 Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Mon, 26 Nov 2018 17:58:18 -0500
Subject: [PATCH] Add PROCASSWIRE error on behavioral assignments to wires,
 msg2737.

---
 Changes                                     |  2 ++
 bin/verilator                               | 23 +++++++++------
 src/V3Ast.h                                 | 19 +++++++++----
 src/V3Error.h                               | 27 ++++++++++--------
 src/V3LinkParse.cpp                         | 21 ++++++++------
 src/V3Undriven.cpp                          | 31 +++++++++++++++++++--
 test_regress/t/t_concat_large.v             |  2 +-
 test_regress/t/t_flag_csplit.v              | 11 +++++---
 test_regress/t/t_select_little_pack.v       |  2 +-
 test_regress/t/t_trace_public.v             | 10 +++----
 test_regress/t/t_tri_inout2.v               |  1 -
 test_regress/t/t_unoptflat_simple_2.v       |  8 ++----
 test_regress/t/t_unoptflat_simple_2_bad.out |  6 ++--
 test_regress/t/t_wire_beh_bad.out           |  2 ++
 test_regress/t/t_wire_beh_bad.pl            | 19 +++++++++++++
 test_regress/t/t_wire_beh_bad.v             | 14 ++++++++++
 16 files changed, 140 insertions(+), 58 deletions(-)
 create mode 100644 test_regress/t/t_wire_beh_bad.out
 create mode 100755 test_regress/t/t_wire_beh_bad.pl
 create mode 100644 test_regress/t/t_wire_beh_bad.v

diff --git a/Changes b/Changes
index 136ff26f9..02adbd6e7 100644
--- a/Changes
+++ b/Changes
@@ -10,6 +10,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 ***   In --xml-only show the original unmodified names, and add module_files
       and cells similar to Verilog-Perl, msg2719. [Kanad Kanhere]
 
+****  Add PROCASSWIRE error on behavioral assignments to wires, msg2737. [Neil Turton]
+
 ****  Fix --trace-lxt2 compile error on MinGW, msg2711. [HyungKi Jeong]
 
 ****  Fix hang on bad pattern keys, bug1364. [Matt Myers]
diff --git a/bin/verilator b/bin/verilator
index 04dfefdfb..3da979c51 100755
--- a/bin/verilator
+++ b/bin/verilator
@@ -3455,15 +3455,6 @@ instead intended is to use a casez with C<?>.
 Ignoring this warning will only suppress the lint check, it will simulate
 correctly.
 
-=item COLONPLUS
-
-Warns that a :+ is seen. Likely the intent was to use +: to select a range
-of bits. If the intent was a range that is explicitly positive, suggest
-adding a space, e.g. use ": +".
-
-Ignoring this warning will only suppress the lint check, it will simulate
-correctly.
-
 =item CDCRSTLOGIC
 
 With --cdc only, warns that asynchronous flop reset terms come from other
@@ -3488,6 +3479,15 @@ For example "X > 1" will always be true when X is a single bit wide.
 Ignoring this warning will only suppress the lint check, it will simulate
 correctly.
 
+=item COLONPLUS
+
+Warns that a :+ is seen. Likely the intent was to use +: to select a range
+of bits. If the intent was a range that is explicitly positive, suggest
+adding a space, e.g. use ": +".
+
+Ignoring this warning will only suppress the lint check, it will simulate
+correctly.
+
 =item COMBDLY
 
 Warns that you have a delayed assignment inside of a combinatorial block.
@@ -3693,6 +3693,11 @@ signal.
 Disabled by default as this is a code style warning; it will simulate
 correctly.
 
+=item PROCASSWIRE
+
+Error that a procedural assignment is setting a wire. According to IEEE, a
+var/reg must be used as the target of procedural assignments.
+
 =item REALCVT
 
 Warns that a real number is being implicitly rounded to an integer, with
diff --git a/src/V3Ast.h b/src/V3Ast.h
index 3254e4689..39355d0e5 100644
--- a/src/V3Ast.h
+++ b/src/V3Ast.h
@@ -548,11 +548,20 @@ public:
             "BLOCKTEMP", "MODULETEMP", "STMTTEMP", "XTEMP",
             "IFACEREF"};
         return names[m_e]; }
-    bool isSignal() const  { return (m_e==WIRE || m_e==WREAL || m_e==IMPLICITWIRE
-                                     || m_e==TRIWIRE
-                                     || m_e==TRI0 || m_e==TRI1 || m_e==PORT
-                                     || m_e==SUPPLY0 || m_e==SUPPLY1
-                                     || m_e==VAR); }
+    bool isSignal() const {
+        return (m_e==WIRE || m_e==WREAL || m_e==IMPLICITWIRE
+                || m_e==TRIWIRE
+                || m_e==TRI0 || m_e==TRI1 || m_e==PORT
+                || m_e==SUPPLY0 || m_e==SUPPLY1
+                || m_e==VAR);
+    }
+    bool isProcAssignable() const {
+        return (m_e==GPARAM || m_e==LPARAM || m_e==GENVAR
+                || m_e==VAR
+                || m_e==TRIWIRE || m_e==TRI0 || m_e==TRI1 || m_e==PORT
+                || m_e==BLOCKTEMP || m_e==MODULETEMP || m_e==STMTTEMP
+                || m_e==XTEMP || m_e==IFACEREF);
+    }
   };
   inline bool operator== (AstVarType lhs, AstVarType rhs) { return (lhs.m_e == rhs.m_e); }
   inline bool operator== (AstVarType lhs, AstVarType::en rhs) { return (lhs.m_e == rhs); }
diff --git a/src/V3Error.h b/src/V3Error.h
index 398e49520..6f54b645b 100644
--- a/src/V3Error.h
+++ b/src/V3Error.h
@@ -70,9 +70,9 @@ public:
 	CLKDATA,        // Clock used as data
 	CMPCONST,	// Comparison is constant due to limited range
 	COLONPLUS,	// :+ instead of +:
-	COMBDLY,	// Combinatorial delayed assignment
-	DEFPARAM,	// Style: Defparam
-	DECLFILENAME,	// Declaration doesn't match filename
+        COMBDLY,        // Combinatorial delayed assignment
+        DEFPARAM,       // Style: Defparam
+        DECLFILENAME,   // Declaration doesn't match filename
 	ENDLABEL,	// End lable name mismatch
 	GENCLK,		// Generated Clock
 	IFDEPTH,	// If statements too deep
@@ -87,8 +87,9 @@ public:
 	MULTIDRIVEN,	// Driven from multiple blocks
 	PINMISSING,	// Cell pin not specified
 	PINNOCONNECT,	// Cell pin not connected
-	PINCONNECTEMPTY,// Cell pin connected by name with empty reference: ".name()" (can be used to mark unused pins)
-	REALCVT,	// Real conversion
+        PINCONNECTEMPTY,// Cell pin connected by name with empty reference
+        PROCASSWIRE,    // Procedural assignment on wire
+        REALCVT,        // Real conversion
 	REDEFMACRO,	// Redefining existing define macro
 	SELRANGE,	// Selection index out of range
 	STMTDLY,	// Delayed statement
@@ -132,13 +133,14 @@ public:
 	    "ALWCOMBORDER", "ASSIGNDLY", "ASSIGNIN",
 	    "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE",
 	    "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CLKDATA",
-            "CMPCONST", "COLONPLUS", "COMBDLY", "DEFPARAM", "DECLFILENAME",
-	    "ENDLABEL", "GENCLK",
-	    "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE",
+            "CMPCONST", "COLONPLUS", "COMBDLY",
+            "DEFPARAM", "DECLFILENAME",
+            "ENDLABEL", "GENCLK",
+            "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPURE",
             "INCABSPATH", "INFINITELOOP", "INITIALDLY",
-	    "LITENDIAN", "MODDUP",
-	    "MULTIDRIVEN",
-	    "PINMISSING", "PINNOCONNECT", "PINCONNECTEMPTY",
+            "LITENDIAN", "MODDUP",
+            "MULTIDRIVEN",
+            "PINMISSING", "PINNOCONNECT", "PINCONNECTEMPTY", "PROCASSWIRE",
 	    "REALCVT", "REDEFMACRO",
 	    "SELRANGE", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET",
             "TICKCOUNT",
@@ -158,7 +160,8 @@ public:
     // Later -Werror- options may make more of these.
     bool pretendError() const { return ( m_e==ASSIGNIN || m_e==BLKANDNBLK
 					 || m_e==BLKLOOPINIT
-					 || m_e==IMPURE); }
+                                         || m_e==IMPURE
+                                         || m_e==PROCASSWIRE); }
     // Warnings to mention manual
     bool mentionManual() const { return ( m_e==EC_FATALSRC || m_e==SYMRSVDWORD
 					  || pretendError() ); }
diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp
index 10836fcdb..42aca8dc6 100644
--- a/src/V3LinkParse.cpp
+++ b/src/V3LinkParse.cpp
@@ -203,15 +203,18 @@ private:
             else if (!m_ftaskp && nodep->isNonOutput()) {
 		nodep->v3error("Unsupported: Default value on module input: "<<nodep->prettyName());
 		nodep->valuep()->unlinkFrBack()->deleteTree();
-	    } // 2. Under modules, it's an initial value to be loaded at time 0 via an AstInitial
-	    else if (m_valueModp) {
-		nodep->addNextHere
-		    (new AstInitial
-                     (fl, new AstAssign(fl, new AstVarRef(fl, nodep->name(), true),
-                                        nodep->valuep()->unlinkFrBack())));
-	    } // 3. Under blocks, it's an initial value to be under an assign
-	    else {
-		nodep->addNextHere
+            }  // 2. Under modules, it's an initial value to be loaded at time 0 via an AstInitial
+            else if (m_valueModp) {
+                // Making an AstAssign (vs AstAssignW) to a wire is an error, suppress it
+                FileLine* newfl = new FileLine (fl);
+                newfl->warnOff(V3ErrorCode::PROCASSWIRE, true);
+                nodep->addNextHere
+                    (new AstInitial
+                     (newfl, new AstAssign(newfl, new AstVarRef(newfl, nodep->name(), true),
+                                           nodep->valuep()->unlinkFrBack())));
+            }  // 3. Under blocks, it's an initial value to be under an assign
+            else {
+                nodep->addNextHere
                     (new AstAssign(fl, new AstVarRef(fl, nodep->name(), true),
                                    nodep->valuep()->unlinkFrBack()));
 	    }
diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp
index beddd9203..f6bdb7d5f 100644
--- a/src/V3Undriven.cpp
+++ b/src/V3Undriven.cpp
@@ -232,7 +232,8 @@ private:
 
     // STATE
     std::vector<UndrivenVarEntry*> m_entryps[3];  // Nodes to delete when we are finished
-    bool		m_inBBox;	// In black box; mark as driven+used
+    bool                m_inBBox;       // In black box; mark as driven+used
+    bool                m_inProcAssign;  // In procedual assignment
     AstNodeFTask*       m_taskp;        // Current task
     AstAlways*          m_alwaysCombp;  // Current always if combo, otherwise NULL
 
@@ -319,6 +320,13 @@ private:
     }
     virtual void visit(AstNodeVarRef* nodep) {
 	// Any variable
+        if (nodep->lvalue()
+            && !VN_IS(nodep, VarXRef)) {  // Ignore interface variables and similar ugly items
+            if (m_inProcAssign && !nodep->varp()->varType().isProcAssignable()) {
+                nodep->v3warn(PROCASSWIRE, "Procedural assignment to wire, perhaps intend var (IEEE 2017 6.5): "
+                              +nodep->prettyName());
+            }
+        }
         for (int usr=1; usr<(m_alwaysCombp?3:2); ++usr) {
             UndrivenVarEntry* entryp = getEntryp(nodep->varp(), usr);
             bool fdrv = nodep->lvalue() && nodep->varp()->attrFileDescr();  // FD's are also being read from
@@ -341,6 +349,24 @@ private:
 	m_inBBox = prevMark;
     }
 
+    virtual void visit(AstAssign* nodep) {
+        bool prevBeh = m_inProcAssign;
+        {
+            m_inProcAssign = true;
+            iterateChildren(nodep);
+            m_inProcAssign = false;
+        }
+        m_inProcAssign = prevBeh;
+    }
+    virtual void visit(AstAssignDly* nodep) {
+        bool prevBeh = m_inProcAssign;
+        {
+            m_inProcAssign = true;
+            iterateChildren(nodep);
+            m_inProcAssign = false;
+        }
+        m_inProcAssign = prevBeh;
+    }
     virtual void visit(AstAlways* nodep) {
         AstAlways* prevAlwp = m_alwaysCombp;
         {
@@ -379,7 +405,8 @@ private:
 public:
     // CONSTUCTORS
     explicit UndrivenVisitor(AstNetlist* nodep) {
-	m_inBBox = false;
+        m_inBBox = false;
+        m_inProcAssign = false;
         m_taskp = NULL;
         m_alwaysCombp = NULL;
         iterate(nodep);
diff --git a/test_regress/t/t_concat_large.v b/test_regress/t/t_concat_large.v
index ab2f17e33..5e52ce2af 100644
--- a/test_regress/t/t_concat_large.v
+++ b/test_regress/t/t_concat_large.v
@@ -5,7 +5,7 @@
 
 module t (/*AUTOARG*/);
 
-   wire [32767:0] a;
+   reg [32767:0] a;
 
    initial begin
       // verilator lint_off WIDTHCONCAT
diff --git a/test_regress/t/t_flag_csplit.v b/test_regress/t/t_flag_csplit.v
index f31a14ad6..462666a9a 100644
--- a/test_regress/t/t_flag_csplit.v
+++ b/test_regress/t/t_flag_csplit.v
@@ -13,19 +13,22 @@ module t (/*AUTOARG*/
 
    parameter CNT = 10;
 
-   wire [31:0] 	w [CNT:0];
+   wire [31:0]  w [CNT:0];
 
    generate
       for (genvar g=0; g<CNT; g++)
-	sub sub (.clk(clk), .i(w[g]), .z(w[g+1]));
+        sub sub (.clk(clk), .i(w[g]), .z(w[g+1]));
    endgenerate
 
+   reg [31:0]   w0;
+   assign w[0] = w0;
+
    // Test loop
    always @ (posedge clk) begin
       cyc <= cyc + 1;
       if (cyc==0) begin
-	 // Setup
-	 w[0] = 32'h1234;
+         // Setup
+         w0 = 32'h1234;
       end
       else if (cyc<90) begin
       end
diff --git a/test_regress/t/t_select_little_pack.v b/test_regress/t/t_select_little_pack.v
index 161423041..cf2b8b37c 100644
--- a/test_regress/t/t_select_little_pack.v
+++ b/test_regress/t/t_select_little_pack.v
@@ -10,7 +10,7 @@ module t (/*AUTOARG*/
    input clk;
 
    // No endian warning here
-   wire [7:0] pack [3:0];
+   reg [7:0] pack [3:0];
 
    initial begin
       pack[0] = 8'h78;
diff --git a/test_regress/t/t_trace_public.v b/test_regress/t/t_trace_public.v
index d59aa8158..1f848a876 100644
--- a/test_regress/t/t_trace_public.v
+++ b/test_regress/t/t_trace_public.v
@@ -6,7 +6,7 @@
 module t (
     input  wire CLK,
     output reg  RESET
-	  );
+          );
 
    neg neg (.clk(CLK));
    little little (.clk(CLK));
@@ -31,20 +31,20 @@ endmodule
 
 module glbl();
 `ifdef PUB_FUNC
-   wire GSR;
+   reg GSR;
    task setGSR;
       /* verilator public */
       input value;
       GSR = value;
    endtask
 `else
-   wire GSR /*verilator public*/;
+   reg GSR /*verilator public*/;
 `endif
 endmodule
 
 module neg (
    input clk
-	    );
+            );
 
    reg [0:-7] i8; initial i8 = '0;
    reg [-1:-48] i48; initial i48 = '0;
@@ -59,7 +59,7 @@ endmodule
 
 module little (
    input clk
-	    );
+            );
 
    // verilator lint_off LITENDIAN
    reg [0:7] i8; initial i8 = '0;
diff --git a/test_regress/t/t_tri_inout2.v b/test_regress/t/t_tri_inout2.v
index 62b532681..4c9512cdc 100644
--- a/test_regress/t/t_tri_inout2.v
+++ b/test_regress/t/t_tri_inout2.v
@@ -22,7 +22,6 @@ module t (/*AUTOARG*/
    ChildA childa ( .A(a), .B(b), .en(en), .Y(y),.Yfix(y_fixed) );
 
    initial in=0;
-   initial en=0;
 
    // Test loop
    always @ (posedge clk) begin
diff --git a/test_regress/t/t_unoptflat_simple_2.v b/test_regress/t/t_unoptflat_simple_2.v
index 13000264c..d3b50d32d 100644
--- a/test_regress/t/t_unoptflat_simple_2.v
+++ b/test_regress/t/t_unoptflat_simple_2.v
@@ -13,10 +13,6 @@ module t (/*AUTOARG*/
 
    wire [2:0] x;
 
-   initial begin
-      x = 3'b000;
-   end
-
    assign x[1:0] = { x[0], clk };
    assign x[2:1] = { clk, x[1] };
 
@@ -27,8 +23,8 @@ module t (/*AUTOARG*/
 `endif
 
       if (x[1] != 0) begin
-	 $write("*-* All Finished *-*\n");
-	 $finish;
+         $write("*-* All Finished *-*\n");
+         $finish;
       end
    end
 
diff --git a/test_regress/t/t_unoptflat_simple_2_bad.out b/test_regress/t/t_unoptflat_simple_2_bad.out
index 3901a8db4..bffe4c2f8 100644
--- a/test_regress/t/t_unoptflat_simple_2_bad.out
+++ b/test_regress/t/t_unoptflat_simple_2_bad.out
@@ -1,10 +1,10 @@
 %Warning-UNOPTFLAT: t/t_unoptflat_simple_2.v:14: Signal unoptimizable: Feedback to clock or circular logic: t.x
 %Warning-UNOPTFLAT: Use "/* verilator lint_off UNOPTFLAT */" and lint_on around source to disable this message.
 %Warning-UNOPTFLAT:      Example path: t/t_unoptflat_simple_2.v:14:  t.x
-%Warning-UNOPTFLAT:      Example path: t/t_unoptflat_simple_2.v:20:  ASSIGNW
+%Warning-UNOPTFLAT:      Example path: t/t_unoptflat_simple_2.v:16:  ASSIGNW
 %Warning-UNOPTFLAT:      Example path: t/t_unoptflat_simple_2.v:14:  t.x
 %Warning-UNOPTFLAT:      Widest candidate vars to split:
-%Warning-UNOPTFLAT:           t/t_unoptflat_simple_2.v:14:  t.x, width 3, fanout 13
+%Warning-UNOPTFLAT:           t/t_unoptflat_simple_2.v:14:  t.x, width 3, fanout 12
 %Warning-UNOPTFLAT:      Most fanned out candidate vars to split:
-%Warning-UNOPTFLAT:           t/t_unoptflat_simple_2.v:14:  t.x, width 3, fanout 13
+%Warning-UNOPTFLAT:           t/t_unoptflat_simple_2.v:14:  t.x, width 3, fanout 12
 %Error: Exiting due to
diff --git a/test_regress/t/t_wire_beh_bad.out b/test_regress/t/t_wire_beh_bad.out
new file mode 100644
index 000000000..95acb9018
--- /dev/null
+++ b/test_regress/t/t_wire_beh_bad.out
@@ -0,0 +1,2 @@
+%Error-PROCASSWIRE: t/t_wire_beh_bad.v:12: Procedural assignment to wire, perhaps intend var (IEEE 2017 6.5): w
+%Error: Exiting due to
diff --git a/test_regress/t/t_wire_beh_bad.pl b/test_regress/t/t_wire_beh_bad.pl
new file mode 100755
index 000000000..3ce8c1ef2
--- /dev/null
+++ b/test_regress/t/t_wire_beh_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
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+
+scenarios(vlt => 1);
+
+compile(
+    v_flags2 => ["--lint-only"],
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_wire_beh_bad.v b/test_regress/t/t_wire_beh_bad.v
new file mode 100644
index 000000000..42696fa9b
--- /dev/null
+++ b/test_regress/t/t_wire_beh_bad.v
@@ -0,0 +1,14 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2018 by Wilson Snyder.
+
+module t (/*AUTOARG*/);
+
+   wire w;
+   reg  r;
+
+   assign r = 1'b1;
+   always_comb w = 1'b0;
+
+endmodule