chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.38k stars 214 forks source link

[lexer] rejects preprocessor-evaluated string literal #166

Closed imphil closed 4 years ago

imphil commented 4 years ago

Look at this code snippet:

  `ifdef SRAM_INIT_FILE
    localparam MEM_FILE = `"`SRAM_INIT_FILE`";
    initial begin
      $display("Initializing SRAM from %s", MEM_FILE);
      $readmemh(MEM_FILE, mem);
    end
  `endif

https://github.com/lowRISC/ibex/blob/b22fca7bbedb5daa808f475e435632b15c82e4b4/shared/rtl/ram_1p.sv#L77-L83

Running this through Verible master, I get:

ram_1p.sv:78:27: syntax error, rejected "`" (https://github.com/google/verible).

The code is actually valid Verilog and accepted by other tools (Verilator, Vivado, and many more)

I see that it might be hard to parse that without having the defines actually defined. I don't think there's a way of passing defines to verible_lint at this point?

fangism commented 4 years ago

You'll have to pardon my non-expertise of Verilog. I looked up the 2017 LRM on the subject of Compiler Directives (Ch. 22), and didn't see how `"..." is to be interpreted. I could only see references to `text_macro_name where text_macro_name -> text_macro_identifier -> identifier, but no explanation for a string-literal in the position of text_macro_name. Can you provide some references on this and point me in the right direction? I'm probably just reading this wrong.

Verible does have limited support for un-preprocessed code. It lacks a proper preprocessor at this time because it was focused on single-file operations.

If the above code is indeed valid, I just need to update the lexer to handle it. It already accepts MACRO and MACRO(...) where expressions are expected.

fangism commented 4 years ago

I just found this explanation for `": https://www.systemverilog.io/macros

imphil commented 4 years ago

Have a look here:

image

LRM 2017, "22.5.1 `define", page 680

fangism commented 4 years ago

Got it. Looks like all I have to do now (without worrying about any substitution) is to treat `" as another quote character that opens and closes, and leave the text in between un-lexed.

fangism commented 4 years ago

b/148721293