zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
540 stars 52 forks source link

Parser bug? "Non-exhaustive patterns in Just lhs" #133

Closed chrivers closed 3 years ago

chrivers commented 3 years ago

Hello

I'm getting the following error message from sv2v:

$ sv2v -I . bunch-of-files.v
sv2v: src/Language/SystemVerilog/Parser/ParseDecl.hs:264:9-40: Non-exhaustive patterns in Just lhs

At this point, sv2v just exits with status 1. There's no indication where the problem occurs, so it's hard to narrow down to a precise test case. The code is for a client, so I'm not at liberty to share it, unfortunately.

Any idea what might be going on here?

zachjs commented 3 years ago

Indeed this is a parser bug as the error is indicative of an invalid assumption. If you change the line in question from

Just lhs = takeLHS $ init tokens

to

lhs = case takeLHS $ init tokens of
        Just x -> x
        Nothing -> error $ show tokens

you should get some position information rather than above error. Hopefully that will enable you to identify the problematic source and create a minimal test case you can share. Please let me know what you find!

zachjs commented 3 years ago

I've gone ahead and pushed effectively the above change as there are invalid inputs like the one below which can encounter the pattern match failure. I'm guessing your input is not invalid like the below, but the updated error output should help either way.

module top;
    initial begin
        foo ++ = 1;
    end
endmodule
chrivers commented 3 years ago

That was fast - thanks! Now it's easy to see where the problem is:

...
        for(i = 0; i < 16; i = i + 1) begin
            pwm_out[i] <= pwmval[i] <= pwmcnt;   // <--- this line here
        end
...
zachjs commented 3 years ago

This was a great catch. Thanks for reporting it! In contexts where either declarations or procedural assignments could appear (the start of a procedural block), the non-blocking assignment operator (<=) incorrectly used the precedence of the comparison operator. Thus, the line in question was effectively being parsed as:

(pwm_out[i] <= pwmval[i]) <= pwmcnt;

This issue should now be fixed on master. Please let me know if you run into other issues!

chrivers commented 3 years ago

Can confirm - this seems to work on master now. Thanks for the quick turnaround :)

chrivers commented 3 years ago

Sorry, I meant to mark this as "fixed", but I don't think I can do that. Maybe you can relabel?

zachjs commented 3 years ago

Closing the issue suffices for me. Thanks!