steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.75k stars 516 forks source link

Uninformative "syntax error" message when an undefined package name is used #1139

Open Kreijstal opened 1 month ago

Kreijstal commented 1 month ago

So, I have the following testbench:

module student_rlight_tb;
  logic clk;
  logic rst_n;
  logic [7:0] led;
  tlul_pkg::tl_d2h_t tl_d2h;
  tlul_pkg::tl_h2d_t tl_h2d;

  // 50 MHz
  always begin
    clk = '1;

I am running it like this:

$ iverilog -g2012 -Isrc/rtl/tlul/pkg -o student_rlight_tb.vvp src/fv/student_rlight_tb.sv
src/fv/student_rlight_tb.sv:5: syntax error
src/fv/student_rlight_tb.sv:5: error: Invalid module instantiation
src/fv/student_rlight_tb.sv:6: error: Invalid module instantiation

But already at line 5, iverilog says that I have a syntax error... However this seems to work on the other sims, what is the problem?

martinwhitaker commented 1 month ago

You haven't compiled the package definition. -I just sets the search path for include files; it won't automatically include anything.

We could do with a better error message in this situation, so I'll label this as an enhancement.

Kreijstal commented 1 month ago

Okay, yeah, I am new to iverilog I think that was the problem I thought syntax error meant there was something wrong with syntax itself but it seems it is a semantic error, not a syntactic one, right?

Kreijstal commented 1 month ago

Not sure if I should make a new issue or edit this one but I get this now:

  // Register width information to check illegal writes
  parameter logic [3:0] STUDENT_DMA_PERMIT [6] = '{
    4'b 0001, // index[0] STUDENT_DMA_STATUS
    4'b 1111, // index[1] STUDENT_DMA_NOW_DADR
    4'b 0001, // index[2] STUDENT_DMA_CMD
    4'b 1111, // index[3] STUDENT_DMA_LENGTH
    4'b 1111, // index[4] STUDENT_DMA_SRC_ADR
    4'b 1111  // index[5] STUDENT_DMA_DST_ADR
  };

with the error: error: parameter declared outside parameter port list must have a default value. But I think there is already a default value..

larsclausen commented 1 month ago

Sorry, unpacked array parameters are not yet supported. See #846.