Merge pull request #270 from DaveMcEwan/V2k1

New rules for compatibility with Verilog 2001

build.rs

@@ -54,6 +54,16 @@ const RENAMED_SYNTAXRULES: &[(&str, &str, &str)] = &[
         "keyword_required_generate",
         "KeywordRequiredGenerate",
     ),
+    (
+        "non_ansi_module",
+        "module_nonansi_forbidden",
+        "ModuleNonansiForbidden",
+    ),
+    (
+        "level_sensitive_always",
+        "general_always_no_edge",
+        "GeneralAlwaysNoEdge",
+    ),
 ];
 
 fn write_rules_rs(

md/manual-introduction.md

@@ -155,7 +155,7 @@ prefix_label = "lab_"
 style_indent = true
 
 [syntaxrules]
-non_ansi_module = true
+module_nonansi_forbidden = true
 keyword_forbidden_wire_reg = true
 ```
 

md/manual-rulesets.md

@@ -137,8 +137,8 @@ If instead the `--config` option was used in wrapper scripts, this could lead
 to confusion where TOML files exist elsewhere in the hierarchy.
 
 It isn't essential for all ruleset scripts to be POSIX compliant, but POSIX
-compliance should be encourage because it allows for consistent behavior across
-the widest range of systems.
+compliance should be encouraged because it allows for consistent behavior
+across the widest range of systems.
 The utilities used in the POSIX wrappers are specified in the current POSIX
 standard (IEEE1003.1-2017, Volume 3: Shell and Utilities).
 Some resources related to these components:

md/ruleset-DaveMcEwan-design.md

@@ -366,7 +366,7 @@ syntaxrules.function_with_automatic = true
 syntaxrules.keyword_forbidden_priority = true
 syntaxrules.keyword_forbidden_unique = true
 syntaxrules.keyword_forbidden_unique0 = true
-syntaxrules.level_sensitive_always = true
+syntaxrules.general_always_no_edge = true
 syntaxrules.operator_case_equality = true
 
 # Common to **ruleset-designintent**.
@@ -375,7 +375,7 @@ syntaxrules.default_nettype_none = true
 syntaxrules.function_same_as_system_function = true
 syntaxrules.keyword_forbidden_always = true
 syntaxrules.keyword_forbidden_wire_reg = true
-syntaxrules.non_ansi_module = true
+syntaxrules.module_nonansi_forbidden = true
 ```
 
 Generally, elaboration-time constants (`parameter`, `localparam`) should be

md/ruleset-designintent.md

@@ -15,7 +15,7 @@ syntaxrules.function_with_automatic = true
 syntaxrules.keyword_forbidden_priority = true
 syntaxrules.keyword_forbidden_unique = true
 syntaxrules.keyword_forbidden_unique0 = true
-syntaxrules.level_sensitive_always = true # Redundant with keyword_forbidden_always.
+#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always.
 syntaxrules.operator_case_equality = true
 ```
 
@@ -32,7 +32,7 @@ syntaxrules.default_nettype_none = true
 syntaxrules.function_same_as_system_function = true
 syntaxrules.keyword_forbidden_always = true
 syntaxrules.keyword_forbidden_wire_reg = true
-syntaxrules.non_ansi_module = true
+syntaxrules.module_nonansi_forbidden = true
 ```
 
 When synthesised into a netlist, generate blocks should have labels so that

md/ruleset-designintentV2001.md

@@ -0,0 +1,125 @@
+
+This ruleset has the same aims as **ruleset-designintent** but with the
+additional aim of only allowing code which is backwards compatible with
+IEEE1364-2001 (Verilog).
+Note that IEEE1364-2001 is not the most recent version (IEEE1364-2005), which
+was released in the same year as the first version of SystemVerilog
+(IEEE1800-2005).
+
+Firstly, let's forbid some things which are only in SystemVerilog, but not
+Verilog.
+```toml
+syntaxrules.keyword_forbidden_always_comb = true
+syntaxrules.keyword_forbidden_always_ff = true
+syntaxrules.keyword_forbidden_always_latch = true
+syntaxrules.keyword_forbidden_priority = true
+syntaxrules.keyword_forbidden_unique = true
+syntaxrules.keyword_forbidden_unique0 = true
+syntaxrules.keyword_forbidden_logic = true
+syntaxrules.operator_incdec = true
+syntaxrules.operator_self_assignment = true
+```
+
+Next, let's use some of the rules in common with **ruleset-simsynth**.
+```toml
+syntaxrules.enum_with_type = true
+syntaxrules.function_with_automatic = true
+syntaxrules.operator_case_equality = true
+syntaxrules.action_block_with_side_effect = true
+syntaxrules.default_nettype_none = true
+syntaxrules.function_same_as_system_function = true
+```
+
+Verilog does allow both ANSI and non-ANSI forms of module declaration, but
+there is a crucial difference for the ANSI form:
+Only `parameter`s are allowed in the list of parameter ports, not
+`localparam`s, meaning that derived parameters are overridable.
+In the following example, there is no way of preventing `PTR_W` from being
+overridden to something incorrect, risking some frustration and wasted time
+when non-obvious effects cause issues later.
+```verilog
+module M
+  #(parameter integer WIDTH = 123
+  , parameter integer PTR_W = clogb2(WIDTH)
+  )
+  ( input  wire [WIDTH-1:0] i_data
+  , output wire [PTR_W-1:0] o_pointer
+  );
+```
+However, using the non-ANSI form allows `PTR_W` to be specified as
+`localparam`, thus preventing overrides and the resulting confusion, i.e:
+```verilog
+module M
+  ( i_data
+  , o_pointer
+  );
+
+  parameter integer WIDTH = 123;
+  localparam integer PTR_W = clogb2(WIDTH);
+
+  input  wire [WIDTH-1:0] i_data;
+  output wire [PTR_W-1:0] o_pointer;
+```
+While this only affects modules which use derived parameters in the port
+declarations, a consistent style is generally easier to work with.
+For these reasons, the non-ANSI form is required.
+```toml
+syntaxrules.module_ansi_forbidden = true
+```
+
+SystemVerilog introduced several keywords which greatly help to clarify intent,
+but these are unavailable.
+Instead of `always_ff @(posedge clk)` and `always_comb`, we can use
+`always @(posedge clk)` and `always @*`.
+That means only the form like `always @(a or b)`, i.e. no edge sensitivities,
+can be forbidden.
+```toml
+syntaxrules.general_always_level_sensitive = true
+```
+On the same theme, guidelines around blocking vs non-blocking assignments also
+need to be altered, but keeping the same general intention.
+Clocked `always` processes should only use non-blocking assignment `<=`, and
+combinatorial `always` processes should only use blocking assignment `=`.
+```toml
+syntaxrules.blocking_assignment_in_always_at_edge = true
+syntaxrules.non_blocking_assignment_in_always_no_edge = true
+```
+
+Verilog doesn't have the same distinction between 2-state and 4-state types as
+SystemVerilog, e.g. `int` and `integer`, but requiring some type is still a
+good idea.
+```toml
+syntaxrules.localparam_explicit_type = true
+syntaxrules.parameter_explicit_type = true
+syntaxrules.parameter_default_value = true
+syntaxrules.parameter_in_generate = true
+```
+
+In IEEE1364-2001, the use of `generate` and `endgenerate` is mandatory, but
+optional in IEEE1364-2005.
+For more compatibility, these keywords are required by this ruleset, as are
+`genvar` declarations outside their generate `for` statements.
+The enablements of these rules are swapped in **ruleset-designintent** to
+reduce visual noise in SystemVerilog.
+```toml
+syntaxrules.genvar_declaration_in_loop = false
+syntaxrules.genvar_declaration_out_loop = true
+syntaxrules.keyword_forbidden_generate = false
+syntaxrules.keyword_required_generate = true
+```
+
+Unlike the in the richer language of SystemVerilog, forbidding sequential
+blocks (between `begin` and `end`) and sequential loops (`for` under `always`)
+is probably too restrictive for Verilog.
+Indeed, there is little point in using `always @*` instead of `assign` if
+`begin` and `end` are forbidden - in SystemVerilog, `always_comb` provides
+extra compile-time checks that `assign` does not.
+```toml
+#syntaxrules.loop_statement_in_always = true # Not implemented.
+#syntaxrules.sequential_block_in_always = true # Not implemented.
+syntaxrules.case_default = true # Applies in functions.
+syntaxrules.explicit_case_default = true # Applies under `always`.
+syntaxrules.explicit_if_else = true
+syntaxrules.multiline_for_begin = true
+syntaxrules.multiline_if_begin = true
+```

md/ruleset-simsynth.md

@@ -16,7 +16,7 @@ syntaxrules.function_with_automatic = true
 syntaxrules.keyword_forbidden_priority = true
 syntaxrules.keyword_forbidden_unique = true
 syntaxrules.keyword_forbidden_unique0 = true
-syntaxrules.level_sensitive_always = true
+syntaxrules.general_always_no_edge = true
 syntaxrules.operator_case_equality = true
 ```
 

md/ruleset-verifintent.md

@@ -30,7 +30,7 @@ syntaxrules.action_block_with_side_effect = true
 syntaxrules.default_nettype_none = true
 syntaxrules.function_same_as_system_function = true
 syntaxrules.keyword_forbidden_wire_reg = true
-syntaxrules.non_ansi_module = true
+syntaxrules.module_nonansi_forbidden = true
 ```
 
 Generally, elaboration-time constant (`parameter`, `localparam`) should be

md/syntaxrules-explanation-blocking_assignment_in_always_at_edge.md

@@ -0,0 +1,35 @@
+Simulator event ordering between blocking and non-blocking assignments
+is undefined, so observed behavior is simulator-dependent.
+Edge-sensitive (usually clocked) processes like, `always @(posedge clk)` should
+only contain non-blocking assignments in order for sampling and variable
+evaluation to operate in a defined order, e.g. `q <= d;`, not `q = d;`.
+
+For SystemVerilog (IEEE1800) code, the keyword `always_ff` (or `always_latch`)
+should be used instead of the general purpose `always` to take advantage of
+extra compile-time checks.
+For code which must be compatible with Verilog (IEEE1364), `always` is the only
+option.
+Therefore, this rule `reg` assignments to be compatible with Verilog like this
+(in conjunction with **non_blocking_assignment_in_always_no_edge**):
+```verilog
+always @(posedge clk) q <= d;       // Clocked to reg (flip-flop)
+always @* a = b + c;                // Combinational to reg (logic gates)
+assign d = e + f;                   // Combinational to wire (logic gates)
+```
+
+See also:
+- **non_blocking_assignment_in_always_no_edge** - Useful companion rule.
+- **blocking_assignment_in_always_ff** - Similar rule, suggested as alternative
+  for SystemVerilog code, but not Verilog.
+- **blocking_assignment_in_always_latch** - Useful companion rule for
+  SystemVerilog, but not Verilog.
+- **non_blocking_assignment_in_always_comb** - Useful companion rule for
+  SystemVerilog, but not Verilog.
+
+The most relevant clauses of IEEE1800-2017 are:
+- 4.9.3 Blocking assignment
+- 4.9.4 Non-blocking assignment
+- 9.4.2 Event control
+- 10.4.1 Blocking procedural assignments
+- 10.4.2 Nonblocking procedural assignments
+- 16.5.1 Sampling

md/syntaxrules-explanation-case_default.md

@@ -19,7 +19,7 @@ When `foo` is non-zero, this example may be interpreted in at least two ways:
 See also:
 - **explicit_case_default** - Useful companion rule.
 - **explicit_if_else** - Useful companion rule.
-- **legacy_always** - Useful companion rule.
+- **keyword_forbidden_always** - Useful companion rule.
 - **sequential_block_in_always_comb** - Useful companion rule.
 
 The most relevant clauses of IEEE1800-2017 are:

md/syntaxrules-explanation-eventlist_comma_always_ff.md

@@ -33,7 +33,7 @@ This rule only applies to event expressions in `always_ff` processes.
 See also:
 - **eventlist_or** - Mutually exclusive rule.
 - **blocking_assignment_in_always_ff** - Useful companion rule.
-- **level_sensitive_always** - Useful companion rule.
+- **general_always_no_edge** - Useful companion rule.
 - **style_keyword_1space** - Useful companion rule.
 
 The most relevant clauses of IEEE1800-2017 are:

md/syntaxrules-explanation-eventlist_or.md

@@ -31,7 +31,7 @@ processes.
 See also:
 - **eventlist_comma_always_ff** - Mutually exclusive rule.
 - **blocking_assignment_in_always_ff** - Useful companion rule.
-- **level_sensitive_always** - Useful companion rule.
+- **general_always_no_edge** - Useful companion rule.
 - **style_keyword_commaleading** - Useful companion rule.
 
 The most relevant clauses of IEEE1800-2017 are:

md/syntaxrules-explanation-explicit_case_default.md

@@ -1,4 +1,4 @@
-The reasoning behind this rule are different between combinatial constructs
+The reasoning behind this is are different between combinatial constructs
 (`always_comb`, `always @*`) vs sequential constructs (`always_ff`,
 `always_latch`).
 The reasoning behind this rule is equivalent to that of **explicit_if_else**.
@@ -19,12 +19,12 @@ and clear through some useful redundancy.
 
 NOTE: The legacy keyword `always` can infer both combinational and sequential
 constructs in the same block, which can be confusing and should be avoided.
-Use of the legacy keyword can be detected with the rule **legacy_always**.
+Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**.
 
 See also:
 - **case_default** - Useful companion rule.
 - **explicit_if_else** - Useful companion rule.
-- **legacy_always** - Useful companion rule.
+- **keyword_forbidden_always** - Useful companion rule.
 - **sequential_block_in_always_comb** - Useful companion rule.
 - **sequential_block_in_always_ff** - Useful companion rule.
 - **sequential_block_in_always_latch** - Useful companion rule.

md/syntaxrules-explanation-explicit_if_else.md

@@ -17,11 +17,11 @@ and clear through some useful redundancy.
 
 NOTE: The legacy keyword `always` can infer both combinational and sequential
 constructs in the same block, which can be confusing and should be avoided.
-Use of the legacy keyword can be detected with the rule **legacy_always**.
+Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**.
 
 See also:
 - **explicit_case_default** - Useful companion rule.
-- **legacy_always** - Useful companion rule.
+- **keyword_forbidden_always** - Useful companion rule.
 - **sequential_block_in_always_comb** - Useful companion rule.
 - **sequential_block_in_always_ff** - Useful companion rule.
 - **sequential_block_in_always_latch** - Useful companion rule.

md/syntaxrules-explanation-general_always_level_sensitive.md

@@ -0,0 +1,39 @@
+This rule is specific to code which must be compatible with Verilog, not
+only SystemVerilog.
+
+In Verilog (IEEE1364), there are two language constructs which can be used to
+model combinatorial logic:
+1. Continuous assignment to `wire` signals is specified with the `assign`
+  keyword.
+2. `reg` signals are assigned to with an `always` block, which is evaluated
+  whenever anything in the sensitivity list changes value.
+
+To ensure that the process correctly sensitive to changes on all driving
+signals, `always @*` should be used instead of providing an explicit
+sensitivity list like `always @(a or b or c)`.
+The `always` keyword can also be used for modelling sequential logic by
+including the edge of a signal in the sensitivity list.
+Providing an explicit sensitivity list is prone to two mistakes:
+1. Forgetting to add a driver to the list, e.g. `always @(b) a = b + c;`
+   instead of `always @(b or c) a = b + c;`.
+2. Forgetting to add and edge specifier, e.g. `always @(clk) q <= d;` instead
+   of `always @(posedge clk) q <= d;`.
+   That makes the process level-sensitive, instead of the edge-sensitive.
+
+This rule requires that general-purpose `always` blocks with an explicit
+sensitivity list which include at least one edge.
+Combinational logic should use the Kleen-star notation,
+e.g. `always @* a = b + c;`
+
+See also:
+- **keyword_forbidden_always** - Related rule forbidding general-purpose
+  `always`, only applicable for SystemVerilog code.
+- **general_always_no_edge** - Related rule forbidding purely combinational
+  logic in `always` processes.
+  While this is straightforward to use with SystemVerilog, this might be overly
+  restrictive for Verilog because all combinational variables must be driven
+  with `assign`.
+
+The most relevant clauses of IEEE1800-2017 are:
+- 9.2.2 Always procedures
+- 9.5 Process execution threads

md/syntaxrules-explanation-level_sensitive_always.md → md/syntaxrules-explanation-general_always_no_edge.md

@@ -29,10 +29,11 @@ meet that requriment.
 The alternative rule **keyword_forbidden_always** has similar reasoning but is
 more strict, completely forbidding the use of general-purpose `always` blocks.
 It is appropriate to use **keyword_forbidden_always** on synthesizable design
-code, but on verification code use **level_sensitive_always** instead.
+code, but on verification code use **general_always_no_edge** instead.
 
 See also:
 - **keyword_forbidden_always** - Alternative rule.
+- **general_always_no_edge** - Similar rule that allows `always @*`.
 
 The most relevant clauses of IEEE1800-2017 are:
 - 9.2.2 Always procedures

md/syntaxrules-explanation-interface_port_with_modport.md

@@ -20,7 +20,7 @@ requires that each interface port includes a modport identifier.
 See also:
 - **inout_with_tri** - Useful companion rule.
 - **input_with_var** - Useful companion rule.
-- **non_ansi_module** - Useful companion rule.
+- **module_nonansi_forbidden** - Useful companion rule.
 - **output_with_var** - Useful companion rule.
 
 The most relevant clauses of IEEE1800-2017 are:

md/syntaxrules-explanation-keyword_forbidden_always.md

@@ -28,17 +28,18 @@ process is a valid and useful way of scheduling events.
 Therefore, this rule is intended only for synthesizable design code, not for
 testbench code.
 
-The alternative rule **level_sensitive_always** has similar reasoning but is
+The alternative rule **general_always_no_edge** has similar reasoning but is
 slightly relaxed, requiring that `always` blocks have an explicit sensitivity
 list including an edge.
 It is possible to construct a full-featured testbench where all `always` blocks
 meet that requriment.
 Therefore, it is appropriate to use **keyword_forbidden_always** on
 synthesizable design code, but on verification code use
-**level_sensitive_always** instead.
+**general_always_no_edge** instead.
 
 See also:
-- **level_sensitive_always** - Alternative rule.
+- **general_always_no_edge** - Alternative rule.
+- **general_always_level_sensitive** - Alternative rule.
 - **sequential_block_in_always_comb** - Useful companion rule.
 - **sequential_block_in_always_if** - Useful companion rule.
 - **sequential_block_in_always_latch** - Useful companion rule.