chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server

Verible

The Verible project's main mission is to parse SystemVerilog (IEEE 1800-2017) (as standardized in the SV-LRM) for a wide variety of applications, including developer tools.

It was born out of a need to parse un-preprocessed source files, which is suitable for single-file applications like style-linting and formatting. In doing so, it can be adapted to parse preprocessed source files, which is what real compilers and toolchains require.

The spirit of the project is that no-one should ever have to develop a SystemVerilog parser for their own application, because developing a standard-compliant parser is an enormous task due to the syntactic complexity of the language. Verible's parser is also regularly tested against an ever-growing suite of (tool-independent) language compliance tests at https://symbiflow.github.io/sv-tests/.

A lesser (but notable) objective is that the language-agnostic components of Verible be usable for rapidly developing language support tools for other languages.

Installation

For simple installation, we provide regular binary releases for Linux and Windows, including statically linked binaries for x86 and Arm to run on almost any Linux distribution.

There are also some distributions that include Verible

• Nix has binaries for Linux and MacOS for x86 and Arm.
• There is a homebrew package for MacOS.

If you prefer to build and install the binaries locally yourself, see details below in the Developers section.

SystemVerilog Developer Tools

Parser

Learn more about the parser implementation here.

We provide a standalone verible-verilog-syntax tool to help with visualizing the syntax structure as understood by the lexer and parser. This is very useful for troubleshooting and understand the internal representations seen by the other tools.

The tool has an ability of exporting a concrete syntax tree in JSON format, making use of it in external tools easy. There is also a Python wrapper module and a few example scripts.

Style Linter

verible-verilog-lint identifies constructs or patterns in code that are deemed undesirable according to a style guide. The main goal is to relieve humans the burden of reviewing code for style compliance. Many lint rules use syntax tree pattern matching to find style violations.

Features:

• Style guide citations in diagnostics
• Rule deck configurability
• Waiver mechanisms: in-file, external waiver file
• Github SystemVerilog linter action available.

Documentation:

• Style linter user documentation
• Generated lint rule documentation

Formatter

The verible-verilog-format formatter manages whitespace in accordance with a particular style. The main goal is to relieve humans of having to manually manage whitespace, wrapping, and indentation, and to provide a tool that can be integrated into any editor to enable editor-independent consistency.

Features (various degress of work-in-progress):

• Corrects indentation
• Corrects inter-token spacing, with syntax context awareness
• Line-wrapping to a column limit
• Support for incremental formatting, only touched changed lines.
• Interactive formatting: accept or decline formatting changes
• Tabular alignment
• Github SystemVerilog formatter action available.

Language Server

The verible-verilog-ls is a language server that provides the functionalities that come with the Verible command line tools also directly in your editor.

It implements the standardized language server protocol that is supported by a myriad of editors and IDEs.

The language server provides formatting and linting. If possible, it also provides quick-fixes

Lexical Diff

verible-verilog-diff compares two input files for equivalence.

Verible project tool

verible-verilog-project is a multi-tool that operates on whole Verilog projects, consisting of a file list and related configurations. This serves as a diagnostic tool for analyzing (and potentially transforming) project-level sources.

Code Obfuscator

verible-verilog-obfuscate transforms Verilog code by replacing identifiers with obfuscated names of equal length, and preserving all other text, including spaces. Output is written to stdout. The resulting file size is the same as the original. This is useful for preparing potentially sensitive test cases with tool vendors.

Preprocessor

verible-verilog-preprocessor is a collection of preprocessor-like tools, (but does not include a fully-featured Verilog preprocessor yet.)

Source Code Indexer

verible-verilog-kythe-extractor extracts indexing facts fromm SV source code using the Kythe schema, which can then enhance IDEs with linked cross-references for ease of source code navigation.

Developers, Welcome

For source code browsing, we recommend using the fully-indexed and searchable mirror at https://cs.opensource.google/verible/verible.

If you'd like to contribute, check out the contributing guide and the development resources.

Build

Verible's code base is written in C++.

To build, you need the bazel (>= 5.0 and <= 7) build system and a C++17 compatible compiler (e.g. >= g++-10), as well as python3. Note, to build Verible with bazel-7, so you need to add --noenable_bzlmod to every bazel command. A lot of users of Verible have to work on pretty old installations, so we try to keep the requirements as minimal as possible.

Use your package manager to install the dependencies; on a system with the nix package manager simply run nix-shell to get a build environment.

# Build all tools and libraries
# bazel 5/6/7
bazel build -c opt //...

You can access the generated artifacts under bazel-bin/. For instance the syntax checker will be at bazel-bin/verilog/tools/syntax/verible-verilog-syntax (corresponding to the target name //verilog/tools/syntax:verible-verilog-syntax).

Moreover, if you need statically linked executables that don't depend on your shared libraries, you can use custom config create_static_linked_executables (with this setting bfd linker will be used, instead of default gold linker).

# Generate statically linked executables.
# Uses bfd linker and needs static system libs available.
bazel build -c opt --config=create_static_linked_executables //...

Optionally using local flex/bison for build

Flex and Bison, that are needed for the parser generation, are compiled as part of the build process. But if for any reason you want or need local tools (e.g. if you encounter a compile problem with them - please file a bug then) can choose so by adding --//bazel:use_local_flex_bison to your bazel command line:

# Also append the option '--//bazel:use_local_flex_bison' to test/install commands
bazel build -c opt  --//bazel:use_local_flex_bison //...

Building on Windows

Building on Windows requires LLVM, WinFlexBison 3 and Git-bash to be installed. Using package manager chocolatey, this can be done with

choco install git llvm winflexbison3

Bazel may also require environment variable to use git-bash and LLVM, on powershell

$env:BAZEL_SH="C:\Program Files\Git\git-bash.exe"
$env:BAZEL_LLVM="C:\Program Files\LLVM"

Installation

For simple installation, we provide regular binary releases.

If you prefer to build and install the binaries locally yourself:

# In your home directory
bazel run -c opt :install -- ~/bin

# For a system directory that requires root-access, call with -s option.
# (Do _not_ run bazel with sudo.)
bazel run -c opt :install -- -s /usr/local/bin

(this requies a compliant install utility, otherwise simply copy the binaries from bazel-bin/ to your desired location)

Test

We strongly encourage running the test suite using bazel:

# Run all tests
bazel test -c opt //...

Whenever adding new features in file, say, foo.cc always make sure to also update (or add) the corresponding foo_test.cc. Once you've written the test, you can use .github/bin/generate-coverage-html.sh to double-check that you have covered all code-paths in your test; narrow the coverage run to your test to make sure coverage is not accidentally coming from unrelated tests that happen to use the library:

MODE=coverage .github/bin/build-and-test.sh //foo/bar:foo_test
.github/bin/generate-coverage-html.sh

Mailing Lists

Join the Verible community!

• Developers: [email protected] (join)
• Users: [email protected] (join)

Future

The Verible team is interested in exploring how it can help other tool developers in providing a SystemVerilog front end, for example, emitting an abstract syntax tree (AST) or possibly even provide more higher-level UHDM format. If you are interested in collaborating, contact us.

verible's Issues

[token partitioning] clocking declarations

should:

clocking cb @(posedge clk); 
endclocking 

clocking cb2 @(posedge clk); 
endclocking

is:

clocking cb @(posedge clk); endclocking clocking cb2 @(posedge clk); endclocking

Enforce correct/consistent use of blocking/non-blocking assignments

From our to-be-published style guide (which is consistent with the lowRISC style guide):
https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#blocking-and-non-blocking-assignments

Tighten package-filename rule for _pkg suffix

Fix the package-naming rule:

the optional _pkg suffix is only for filenames: currently, "foo" or "foo_pkg" can be defined in "foo.sv" or "foo_pkg.sv". It should actually be illegal to declare "foo_pkg" in "foo.sv".

Set up Kokoro to build and run Verible tests

I have a WIP pull request at #9

[tabular] Handling of tabular alignment

Tabular alignment is currently not handled by the tool. Below are a couple of test cases for this feature.

example a)

should:

  rand bit [ADDR_WIDTH-1:0]     addr;
  rand rw_e                     read_write;
  rand bit [DATA_WIDTH-1:0]     data;
  rand bit [DATA_WIDTH/8-1:0]   be;
  rand bit [3:0]                gnt_delay;
  rand bit [3:0]                req_delay;
  rand bit [5:0]                rvalid_delay;
  rand bit                      error;

is:

  rand bit [ADDR_WIDTH-1:0]     addr;
  rand rw_e read_write;
  rand bit [DATA_WIDTH-1:0]     data;
  rand bit [DATA_WIDTH/8-1:0]   be;
  rand bit [3:0]                gnt_delay;
  rand bit [3:0]                req_delay;
  rand bit [5:0]                rvalid_delay;
  rand bit error;

example b)

should:

    `uvm_field_int       (addr,             UVM_DEFAULT)
    `uvm_field_enum      (rw_e, read_write, UVM_DEFAULT)
    `uvm_field_int       (be,               UVM_DEFAULT)
    `uvm_field_int       (data,             UVM_DEFAULT)
    `uvm_field_int       (gnt_delay,        UVM_DEFAULT)
    `uvm_field_int       (rvalid_delay,     UVM_DEFAULT)
    `uvm_field_int       (error,            UVM_DEFAULT)

is:

    `uvm_field_int(addr, UVM_DEFAULT)
    `uvm_field_enum(rw_e, read_write, UVM_DEFAULT)
    `uvm_field_int(be, UVM_DEFAULT)
    `uvm_field_int(data, UVM_DEFAULT)
    `uvm_field_int(gnt_delay, UVM_DEFAULT)
    `uvm_field_int(rvalid_delay, UVM_DEFAULT)
    `uvm_field_int(error, UVM_DEFAULT)

example c)

should:

      if (!std::randomize(gnt_delay) with {
        gnt_delay dist {
          min_grant_delay                         :/ 1,
          [min_grant_delay+1 : max_grant_delay-1] :/ 1,
          max_grant_delay                         :/ 1
        };
      }) begin
        `uvm_fatal(`gfn, $sformatf("Cannot randomize grant"))
      end

is:

      if (!std::randomize(gnt_delay) with {
        gnt_delay dist {
          min_grant_delay :/ 1,
          [min_grant_delay + 1 : max_grant_delay - 1] :/ 1,
          max_grant_delay :/ 1
        };
      }) begin
        `uvm_fatal(`gfn, $sformatf("Cannot randomize grant"))
      end

example d)

should:

        addr       == item.addr;
        read_write == item.read_write;
        data       == item.data;
        be         == item.be;

is:

        addr == item.addr;
        read_write == item.read_write;
        data == item.data;
        be == item.be;

example e)

should:

module alert_handler_ping_timer (
  input                                     clk_i,
  input                                     rst_ni,
  input                                     entropy_i,          // from TRNG
  input                                     en_i,               // enable ping testing
  input        [alert_pkg::NAlerts-1:0]     alert_en_i,         // determines which alerts to ping
  input        [alert_pkg::PING_CNT_DW-1:0] ping_timeout_cyc_i, // timeout in cycles
  output logic [alert_pkg::NAlerts-1:0]     alert_ping_en_o,    // enable to alert receivers
  output logic [alert_pkg::N_ESC_SEV-1:0]   esc_ping_en_o,      // enable to esc senders
  input        [alert_pkg::NAlerts-1:0]     alert_ping_ok_i,    // response from alert receivers
  input        [alert_pkg::N_ESC_SEV-1:0]   esc_ping_ok_i,      // response from esc senders
  output logic                              alert_ping_fail_o,  // any of the alert receivers failed
  output logic                              esc_ping_fail_o     // any of the esc senders failed
);

is:

module alert_handler_ping_timer(
    input clk_i,
    input rst_ni,
    input entropy_i,  // from TRNG
    input en_i,  // enable ping testing
    input [alert_pkg::NAlerts-1:0]     alert_en_i,  // determines which alerts to ping
    input [alert_pkg::PING_CNT_DW-1:0] ping_timeout_cyc_i,  // timeout in cycles
    output logic [alert_pkg::NAlerts-1:0]     alert_ping_en_o,  // enable to alert receivers
    output logic [alert_pkg::N_ESC_SEV-1:0]   esc_ping_en_o,  // enable to esc senders
    input [alert_pkg::NAlerts-1:0]     alert_ping_ok_i,  // response from alert receivers
    input [alert_pkg::N_ESC_SEV-1:0]   esc_ping_ok_i,  // response from esc senders
    output logic alert_ping_fail_o,  // any of the alert receivers failed
    output logic esc_ping_fail_o  // any of the esc senders failed
);

example f)

is:

  // Data load/store vif connection
  assign data_mem_vif.clock   = clk;
  assign data_mem_vif.reset   = ~rst_n;
  assign data_mem_vif.request = dut.data_req_o;
  assign data_mem_vif.we      = dut.data_we_o;
  assign data_mem_vif.be      = dut.data_be_o;
  assign data_mem_vif.addr    = dut.data_addr_o;
  assign data_mem_vif.wdata   = dut.data_wdata_o;

should:

  // Data load/store vif connection
  assign data_mem_vif.clock = clk;
  assign data_mem_vif.reset = ~rst_n;
  assign data_mem_vif.request = dut.data_req_o;
  assign data_mem_vif.we = dut.data_we_o;
  assign data_mem_vif.be = dut.data_be_o;
  assign data_mem_vif.addr = dut.data_addr_o;
  assign data_mem_vif.wdata = dut.data_wdata_o;

example g)

should:

    alert_handler_accu i_accu (
      .clk_i,
      .rst_ni,
      .clr_i        ( reg2hw_wrap.class_clr[k]          ),
      .class_trig_i ( hw2reg_wrap.class_trig[k]         ),
      .thresh_i     ( reg2hw_wrap.class_accum_thresh[k] ),
      .accu_cnt_o   ( hw2reg_wrap.class_accum_cnt[k]    ),
      .accu_trig_o  ( class_accum_trig[k]               )
    );

is:

    alert_handler_accu
        i_accu(
            .clk_i,
            .rst_ni,
            .clr_i(reg2hw_wrap.class_clr[k]),
            .class_trig_i(hw2reg_wrap.class_trig[k]),
            .thresh_i(reg2hw_wrap.class_accum_thresh[k]),
            .accu_cnt_o(hw2reg_wrap.class_accum_cnt[k]),
            .accu_trig_o(class_accum_trig[k])
        );

example h)

this is an example for tabular alignment for ternaries

should:

  assign accu_d = (clr_i)                      ? '0            : // clear
                  (class_trig_i && !(&accu_q)) ? accu_q + 1'b1 : // saturate counter at maximum
                                                 accu_q;

is:

  assign accu_d = (clr_i) ? '0            :  // clear
  (class_trig_i && !(&accu_q)) ? accu_q + 1'b1 :  // saturate counter at maximum
  accu_q;

Provide simple file-list flag

Right now, all files are provided on the command-line, but it might be useful to have also the concept of a file-list. That way, it is possible to read e.g. an already preprocessed list of files to be further processed by verible (all tools).

The standardized way (common in verilog processing) looks like it is just to have a file that contains a list of command-line options, so not only a list of files, but everything that can be put on the command-line. Common flag name seems to be -f ?

Maybe there is already a simple way for the ABSL-flag library to read options from a file ?

[line breaks, wraps] Multiline string initializer

should:

  task body();
    string seq_names[] = {"uart_sanity_vseq",
                          "uart_tx_rx_vseq",
                          "uart_fifo_full_vseq",
                          "uart_fifo_overflow_vseq",
                          "uart_fifo_reset_vseq",
                          "uart_common_vseq", // for intr_test
                          "uart_intr_vseq",
                          "uart_noise_filter_vseq",
                          "uart_rx_start_bit_filter_vseq",
                          "uart_rx_parity_err_vseq",
                          "uart_tx_ovrd_vseq",
                          "uart_perf_vseq",
                          "uart_loopback_vseq"};
  endtask : body

is:

  task body();
    string
        seq_names[] = {"uart_sanity_vseq", "uart_tx_rx_vseq", "uart_fifo_full_vseq",
                       "uart_fifo_overflow_vseq", "uart_fifo_reset_vseq", "uart_common_vseq",
        // for intr_test
        "uart_intr_vseq", "uart_noise_filter_vseq", "uart_rx_start_bit_filter_vseq",
            "uart_rx_parity_err_vseq", "uart_tx_ovrd_vseq", "uart_perf_vseq", "uart_loopback_vseq"};
  endtask : body

nonblocking assignments shouldn't be joined onto one line

Reduced test case, formatter produces:

task t;
  a <= b; c <= d;
endtask

verilog_format --show_token_partition_tree shows:

Full token partition tree:
{ ([<auto>])
  { ([<auto>])
    { ([task t ;]) }
  }
  { (  [<auto>])
    { (  [a <= b ; c <= d ;]) }
  }
  { ([endtask]) }
}

verilog_syntax --printtree contains:

  Node (tag: kTaskDeclaration) {
    Node (tag: kTaskHeader) {
      (#"task" @89-93: "task")
      Node (tag: kUnqualifiedId) {
        (#SymbolIdentifier @94-95: "t")
      }
      (#';' @95-96: ";")
    }
    Node (tag: kStatementList) {
      Node (tag: kNonblockingAssignmentStatement) {
...

Fix is simple, just need to add a case to verilog/formatting/tree_unwrapper.cc.

[keyword] Verible breaks when parsing a function that is defining “new”

The following code causes verible to throw an error and stop processing.

`ifndef uvm_component_new
  `define uvm_component_new \
    function new (string name="", uvm_component parent=null); \
      super.new(name, parent); \
    endfunction : new
`endif

The associated error message is:

Mismatch in token sequence lengths: 693 vs. 744
First mismatched token [69]: ("<<`define-tokens>>") (#261: "\
    function new (string name=""); \
      super.new(name); \
    endfunction : new") vs. ("<<`define-tokens>>") (#261: "")
Formatted output is lexically different from the input.  Please file go/verilogformat-bug.
Error lex/parsing-ing formatted output.  Please file go/verilogformat-bug.
First error: token: "new" at 43:14-16

DPI imports in modules need own partition

Example bad formatter output:

module m;
  import "DPI-C" function int add(); import "DPI-C" function int sleep(
      input int secs
  );
endmodule

Each import should start own line.

Lexer fails to recognize newline during formatter verification

The following input causes the formatter to report an output mismatch:

task tgood;
if ((data_in_update_queue[$].needs_update == 1'b1 &&
((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1 ))begin
end
endtask

task tbad;
if (data_in_update_queue[$].needs_update == 1'b1 &&
((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1 )begin
end
endtask

stdout/stderr:

$ verilog_format ~/testing/verilog/formatting/queue.sv
Mismatch in token sequence lengths: 75 vs. 53
First mismatched token [52]: (TK_BinDigits) (#306: "1") vs. (TK_OTHER) (#750: "
")
Formatted output is lexically different from the input.  Please file go/verilogformat-bug.
Error lex/parsing-ing formatted output.  Please file go/verilogformat-bug.
First error: token: "
" at 8:50-9:0
Problematic formatter output is:
task tgood;
  if ((data_in_update_queue[$].needs_update == 1
       'b1 && ((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1))begin
  end
endtask

task tbad;
  if (data_in_update_queue[$].needs_update == 1'b
      1 && ((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1)begin
  end
endtask
<<EOF>>
task tgood;
if ((data_in_update_queue[$].needs_update == 1'b1 &&
((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1 ))begin
end
endtask

task tbad;
if (data_in_update_queue[$].needs_update == 1'b1 &&
((crnt_time - data_in_update_queue[$].eval_time) / clk_period) > 1 )begin
end
endtask

(The part after <> is the original code.)

It looks like the lexer should have recognized a TK_NEWLINE (which is ignored for comparison) between "1" and "'b1", but instead, got a TK_OTHER (which is not ignored).

Set up Travis-CI to build verible and run the tests

@hzeller is working on this.

[line breaks, module, intf] Interface and module port list formatting

This is how systemverilog interface and module declarations should look like:

module my_mod (
  inout clk,
  inout rst_n
);

endmodule : my_mod

module my_intf (
  inout clk,
  inout rst_n
);

endmodule : my_intf

module my_mod #(
  parameter first_param = 1,
  parameter second_param = 2
) (
  inout clk,
  inout rst_n
);

endmodule : my_mod

module my_intf #(
  parameter first_param = 1,
  parameter second_param = 2
) (
  inout clk,
  inout rst_n
);

endmodule : my_intf

Verible currently formats them as follows:

module my_mod(inout clk, inout rst_n);

endmodule : my_mod

module my_intf(inout clk, inout rst_n);

endmodule : my_intf

module my_mod #(parameter first_param = 1, parameter second_param = 2) (inout clk, inout rst_n);

endmodule : my_mod

module my_intf #(parameter first_param = 1, parameter second_param = 2) (inout clk, inout rst_n);

endmodule : my_intf

[line breaks, wraps] Line break tuning

should:

if ($urandom_range(0, 1)) begin
  bit [TL_DW-1:0] wr_data;
  wr_data = $urandom_range((1 << intr_state_csrs[test_index[i]].get_n_used_bits()) - 1);
  exp_intr_state[test_index[i]] &= (~wr_data);
  csr_wr(.csr(intr_state_csrs[test_index[i]]), .value(wr_data));
end

is:

if ($urandom_range(0, 1)) begin
  bit [TL_DW-1:0] wr_data;
  wr_data = $urandom_range((1 << intr_state_csrs[test_index[i]].get_n_used_bits()) -
                           1); exp_intr_state[test_index[i]] &= (~wr_data);
  csr_wr(.csr(intr_state_csrs[test_index[i]]), .value(wr_data));
end

Forbid anonymous enums, require enums to be typedef'd.

https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#enumerations

For this assignment, just check flag bare anonymous enums.

signals must be named lower_snake_case-style

Task: enforce that signal names (at declaration site) are named lower_snake_case style.
https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#construct-naming

[token partitioning] else statements need to start own partition

should:

      if (trans_collected.read_write == WRITE)  item_collected_port.write(trans_collected); 
      else                                      collect_data_queue.put(trans_collected);

is:

      if (trans_collected.read_write == WRITE)
        item_collected_port.write(trans_collected); else collect_data_queue.put(trans_collected);

Make sure cla-bot is working

This repository requires a CLA (see https://opensource.google/docs/releasing/contributions/#cla). Make sure that bot is working.

modport declarations should start on own line

Formatter input:

interface foo_if;
modport tb (output a, b, input c, d);
modport dut (output a, b, input c, d);
endinterface

currently produces:

interface foo_if;
  modport tb(output a, b, input c, d); modport dut(output a, b, input c, d);
endinterface

GNU autotools build

Request for canonical GNU-style configure ... && make.

Function-like (callable) keywords should not have space before '('

Formatter should ideally produce:

wait(a == b); not wait (a == b);
randomize() not randomize ()
foo.shuffle() not foo.shuffle ()

other keywords: sum, assert, etc.

module instantiation should have space after instance identifier

Where mt is the module type and mi is the module instance name, the correct formatting should be: mt mi (...); (space after 'mi'), but the formatter currently removes that space.

But we also have to be careful to not inject spaces in the named port connections, e.g.

  mt mi (.x(x), .y(y), ...);

This is now possible to implement, thanks to syntax-context aware formatting decisions.

[token partitioning] group else-if together

Filed on behalf of @fangism.

bad:

function void foo();
    if ($value$plusargs("target_mabist_group=%s", target_mabist_group)) begin
      if (target_mabist_group == "bo") begin
        target_mabist = $urandom_range(kMabistBo0, kMabistBo10);
      end else
      if (target_mabist_group == "cc") begin
        target_mabist = $urandom_range(kMabistCc0, kMabistCc2);
      end else
      if (target_mabist_group == "cpu") begin
        target_mabist = kMabistCpu;
      end else    ...

good:

function void foo();
    if ($value$plusargs("target_mabist_group=%s", target_mabist_group)) begin
      if (target_mabist_group == "bo") begin
        target_mabist = $urandom_range(kMabistBo0, kMabistBo10);
      end else if (target_mabist_group == "cc") begin
        target_mabist = $urandom_range(kMabistCc0, kMabistCc2);
      end else if (target_mabist_group == "cpu") begin
        target_mabist = kMabistCpu;
      end else begin    ...

Require structs and unions to be typedef'd, never anonymous

Similar to #16 report anonymous structs and unions as lint violations.
You may choose to implement this as one or two rules.

Suggestion: match on struct declaration, and search context to see if it inside a typedef.

Incorrect indentation of $display() after #delay

Example formatter output:

module m;
  initial begin
    #10
  $display("mask: %b", mask);
    $display("TEST PASSED");
  end
endmodule

Need space between ':' and ';' (default case null statement)

The following input:

function f;
case (x)
default :;  // this line, note spacing
endcase
endfunction

should really be formatted to:

function f;
  case (x)
    default: ;  // this line, note spacing
  endcase
endfunction

There are two spacing issues, this one is about spacing between ':' and ';'.

Unwanted space in "default :", should be "default:"

The following input:

function f;
case (x)
default :;  // this line, note spacing
endcase
endfunction

should really be formatted to:

function f;
  case (x)
    default: ;  // this line, note spacing
  endcase
endfunction

There are two spacing issues, this one is about spacing between 'default' and ':'.

Task formal arguments put on a new line

The issue is currently onliy occuring when a task/function has more than 1 arg.

should:

  virtual task check_next_core_status(core_status_t core_status, 
                                      string error_msg = "",
                                      int timeout = 9999999);

is:

  virtual task check_next_core_status(
      core_status_t core_status, string error_msg = "", int timeout = 9999999
  );

Integrate with FuseSoC

https://github.com/olofk/fusesoc

FuseSoC is an award-winning package manager and a set of build tools for HDL (Hardware Description Language) code.

Its main purpose is to increase reuse of IP (Intellectual Property) cores and be an aid for creating, building and simulating SoC solutions.

FuseSoC makes it easier to

• reuse existing cores
• create compile-time or run-time configurations
• run regression tests against multiple simulators
• Port designs to new targets
• let other projects use your code
• set up continuous integration

Would be good to have FuseSoc able to use Verible as a linter and code formatter.

[space] & binary operator needs space on both sides

should:

a = b & ~(c << d);   

is:

a = b &~(c << d);    // missing space after &

[uvm macros, indentation] Indenting uvm macros with begin/end semantic

should:

`uvm_object_utils_begin(ibex_mem_intf_seq_item)
  `uvm_field_int(addr, UVM_DEFAULT)
  `uvm_field_enum(rw_e, read_write, UVM_DEFAULT)
  `uvm_field_int(be, UVM_DEFAULT)
  `uvm_field_int(data, UVM_DEFAULT)
  `uvm_field_int(gnt_delay, UVM_DEFAULT)
  `uvm_field_int(rvalid_delay, UVM_DEFAULT)
  `uvm_field_int(error, UVM_DEFAULT)
`uvm_object_utils_end `uvm_object_new

is:

`uvm_object_utils_begin(ibex_mem_intf_seq_item)
`uvm_field_int(addr, UVM_DEFAULT)
`uvm_field_enum(rw_e, read_write, UVM_DEFAULT)
`uvm_field_int(be, UVM_DEFAULT)
`uvm_field_int(data, UVM_DEFAULT)
`uvm_field_int(gnt_delay, UVM_DEFAULT)
`uvm_field_int(rvalid_delay, UVM_DEFAULT)
`uvm_field_int(error, UVM_DEFAULT)
`uvm_object_utils_end `uvm_object_new

Unwanted space after # in delay expression

Example formatter test case.

task foo;
  # 0;   // unwanted space after #
endtask

b/143479005

missing space around ':' in end labels

Example formatter output:

module top;
initial begin :begin_label
if (1) begin :if_label
end :if_label
end :begin_label
endmodule :top

There should be spaces padding ':' in the above cases.

indent statement comments inside tasks and functions

Formatter outputs:

function f;
// comment
endfunction

but the following would be better:

function f;
  // comment
endfunction

Same for tasks.

module declaration should have space before '(' for ports

Currently the formatter removes the space after foo in module foo(...), but the style committee actually resolved to want the space, as in module foo (...).

This is now possible to do because the formatter is syntax-context aware.

Unexpected indentation of macro after `define

The following input:

`define FOO BAR
`FOO(bar)

gets formatted into:

`define FOO BAR
    `FOO(bar)

but the macro call should not be indented.

verilog_format --show_token_partition_tree shows:

{ ([<auto>]) @{}
  { ([`define FOO BAR `FOO ( bar )]) }
}

The partition should be split into two.

case-like constructs should not have space before ':'

Example input:

module m;
initial begin
case (x)
State0 : a = b;
State1 : begin
  a = b;
end
endcase
end
endmodule

should be formatted:

module m;
  initial begin
    case (x)
      State0: a = b;
      State1: begin
        a = b;
      end
    endcase
  end
endmodule

There should be no space after 'State0' and 'State1'.

Give mithro access to repository

Please :-P

[macro] Macro argument alignment/placement

indentation error after tl_host_single_seq tl_seq;

should:

  virtual task tl_access_sub();
    `DV_SPINWAIT(
        // thread to read/write tlul
        tl_host_single_seq  tl_seq;
        `uvm_create_on(tl_seq, p_sequencer.tl_sequencer_h)
        if (cfg.zero_delays) begin
          tl_seq.min_req_delay = 0;
          tl_seq.max_req_delay = 0;
        end
        if (!write) data = tl_seq.rsp.d_data;
        if (check_rsp) begin
          `DV_CHECK_EQ(tl_seq.rsp.d_error, exp_err_rsp, "unexpected error response")
        end
        csr_utils_pkg::decrement_outstanding_access();,
        // thread to check timeout
        $sformatf("Timeout waiting tl_access : addr=0x%0h", addr))
  endtask

is:

  virtual task tl_access_sub();
    `DV_SPINWAIT(
    // thread to read/write tlul
    tl_host_single_seq  tl_seq;
        `uvm_create_on(tl_seq, p_sequencer.tl_sequencer_h)
        if (cfg.zero_delays) begin
          tl_seq.min_req_delay = 0;
          tl_seq.max_req_delay = 0;
        end
        if (!write) data = tl_seq.rsp.d_data;
        if (check_rsp) begin
          `DV_CHECK_EQ(tl_seq.rsp.d_error, exp_err_rsp, "unexpected error response")
        end
        csr_utils_pkg::decrement_outstanding_access();
        ,
    // thread to check timeout
    $sformatf("Timeout waiting tl_access : addr=0x%0h", addr))
  endtask

Ban use of defparam

Our to-be-published style guide explicitly bans the use of the defparam construct.
This feature request aims to enforce just that.

Assignment:
Implement this check as a rule inside here:
https://github.com/google/verible/tree/master/verilog/analysis/checkers

Include unit-tests, positive and negative.

Add a TODO to enable by default here:
https://github.com/google/verible/blob/master/verilog/analysis/default_rules.h

Add an integration test here:
https://github.com/google/verible/blob/master/verilog/tools/lint/BUILD

Suggestion:
Use a syntax tree matcher, starts with 'Nodek', defined in https://github.com/google/verible/blob/master/verilog/CST/verilog_matchers.h

Questions? ask in [email protected]!
https://groups.google.com/forum/#!forum/verible-dev

package-filename rule: consider the first dot-component of filename

Fix the package-naming rule:

we should ignore everything after the dot when extracting package names. For example, "foo_pkg.generated.sv" can contain "foo" or "foo_pkg".

[line breaks, wraps] Parameterized module instantiation

should:

  ibex_core_tracing #(
    .DmHaltAddr(`BOOT_ADDR + 'h0),
    .DmExceptionAddr(`BOOT_ADDR + 'h4)
  ) dut (
    .clk_i(clk),
    .rst_ni(rst_n),
    .test_en_i(1'b1),
    .instr_rdata_i(instr_mem_vif.rdata),
    .instr_err_i(instr_mem_vif.error)
  );

is:

ibex_core_tracing #(.DmHaltAddr(`BOOT_ADDR + 'h0), .DmExceptionAddr(`BOOT_ADDR + 'h4))
      dut(
          .clk_i(clk),
          .rst_ni(rst_n),
          .test_en_i(1'b1),
          .instr_rdata_i(instr_mem_vif.rdata),
          .instr_err_i(instr_mem_vif.error)
      );

[Indentation] Multiline list parameter

should:

// Implemented previlieged CSR list
parameter privileged_reg_t implemented_csr[] = {
  // Machine mode mode CSR
  MVENDORID,        // Vendor ID
  MARCHID,          // Architecture ID
  DSCRATCH0,        // Debug scratch register 0
  DSCRATCH1         // Debug scratch register 1
};

is:

// Implemented previlieged CSR list
parameter privileged_reg_t implemented_csr[] = {
// Machine mode mode CSR
MVENDORID,  // Vendor ID
MARCHID,  // Architecture ID
DSCRATCH0,  // Debug scratch register 0
DSCRATCH1  // Debug scratch register 1
};

[indentation] 4 space vs 2 space indentation in portlists

The opentitan style guide https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md uses 2 spaces to indent the port lists of a module, whereas the current implementation of the formatter uses 4 spaces.

E.g.

import my_mod (
  input clk_i,
  input rst_ni
);

versus

import my_mod (
    input clk_i,
    input rst_ni
);

We should discuss whether the OT style should align with 4 spaces, or whether the tool should support multiple indentation styles.

[space] Unneeded space in ternaries

should:

  assign usb_tx_rready = sys_loopback ? lb_data_move : usb_if_tx_read;
  assign usb_rx_wvalid = sys_loopback ? lb_data_move : usb_if_rx_write;
  assign usb_rx_fifo_wdata = sys_loopback ? usb_tx_fifo_rdata : usb_if_rx_fifo_wdata;

is:

  assign usb_tx_rready = sys_loopback ? lb_data_move      : usb_if_tx_read;
  assign usb_rx_wvalid = sys_loopback ? lb_data_move      : usb_if_rx_write;
  assign usb_rx_fifo_wdata = sys_loopback ? usb_tx_fifo_rdata : usb_if_rx_fifo_wdata;

Awkward $sformatf() line break

should:

      trans_collected.be   = vif.be;
      `uvm_info(get_full_name(), $sformatf("Detect request with address: %0x",
                trans_collected.addr), UVM_HIGH)

is:

      trans_collected.be = vif.be;
      `uvm_info(get_full_name(), $sformatf("Detect request with address: %0x", trans_collected.addr
                ), UVM_HIGH)

macro call arguments not sufficiently partitioned

should:

    `ASSERT(MioWarl_A,
            padctrl.reg2hw.mio_pads[mio_sel].qe |=>
        !(|padctrl.mio_attr_q[mio_sel][padctrl_reg_pkg::AttrDw-1:2]),
            clk_i, !rst_ni)

is:

    `ASSERT(MioWarl_A,
            padctrl.reg2hw.mio_pads[mio_sel].qe |=>
        !(|padctrl.mio_attr_q[mio_sel][padctrl_reg_pkg::AttrDw-1:2])
            , clk_i, !rst_ni)

[line breaks, wraps] Multiline parameter break tuning

should:

parameter exception_cause_t implemented_exception[] = {INSTRUCTION_ACCESS_FAULT, 
                                                       ILLEGAL_INSTRUCTION, BREAKPOINT,
                                                       LOAD_ACCESS_FAULT, STORE_AMO_ACCESS_FAULT, 
                                                       ECALL_MMODE};

is:

parameter exception_cause_t
    implemented_exception[] = {INSTRUCTION_ACCESS_FAULT, ILLEGAL_INSTRUCTION, BREAKPOINT,
                               LOAD_ACCESS_FAULT, STORE_AMO_ACCESS_FAULT, ECALL_MMODE};

Note that this "compact" enumeration style is different than from the "long list style" mentioned in #32

struct/union typedef names should be lower_snake_case_t

Check that the typedef names of structs and unions are lower_snake_case style and end with '_t'.
Checking for use in typedef (non-anonymous) is covered by #54 .
Report a violation on the offending identifier of the type declaration.

Some helper functions on naming style can be found at:
https://github.com/google/verible/blob/master/common/strings/naming_utils.h

[token partitioning] covergroup declaration should fit on one line, but is split

Filed on behalf of @fangism.

Test case:

class c;
covergroup pending_req_on_rst_cg(string name) with function sample(bit pending);
endgroup
endclass

formats to:

class c;
  covergroup pending_req_on_rst_cg(
      string name
  ) with function sample (
      bit pending
  );
  endgroup
endclass

$ verilog_format --show_token_partition_tree ~/testing/verilog/formatting/covergroup.sv

Full token partition tree:
{ ([<auto>]) @{}
  { ([<auto>]) @{0}
    { ([class c ;]) }
  }
  { (  [<auto>]) @{1}
    { (  [covergroup pending_req_on_rst_cg (]) }
    { (      [<auto>]) @{1,1}
      { (      [string name]) }
    }
    { (  [) with function sample (]) }
    { (      [<auto>]) @{1,3}
      { (      [bit pending]) }
    }
    { (  [) ;]) }
    { (  [endgroup]) }
  }
  { ([endclass]) }
}

Probably want a level of partitioning just for the covergroup declaration header, just like we did for modules headers.