search

B-Lang-org / bsc

Bluespec Compiler (BSC)
Other
926 stars 142 forks source link

SV preprocessor corner cases #584

Open quark17 opened 1 year ago

quark17 commented 1 year ago

As mentioned in issue #469, there are warnings about non-exhaustive pattern matching in SystemVerilogScanner.lhs, in the function scanLinePosDirective which parses `line directives.

The preprocessor accepts two kinds of `line directive: one according to the Verilog/SystemVerilog standards (without parentheses) and one with parentheses (which may be something that Bluespec invented, for preserving position information in the pre-processor output?). The function scanLinePosDirective is for parsing the directives with parentheses. A comment on the function says "One day the gods will smite me for this" -- I don't know if this is for inventing the new syntax or because the function doesn't handle any error conditions!

I wrote a few examples of error situations: closing paren but not enough arguments; EOF before the closing paren; missing closing paren; closing paren (or arguments) on the following line; non-numeric values where numeric arguments expected; etc. Surprisingly, I found that a directive spread over multiple lines will still parse -- I don't know if that's expected. (This is not high priority, but I wanted to record the issue.)

Macro definitions (the `define directive) can also take arguments in parentheses. This is handled in SystemVerilogPreprocess.lhs (which also generates non-exhaustive pattern warnings). I wrote some examples and found that error situations are also not being handled there. (This is more serious, because it's a user-visible feature and an actual part of the SV spec.) One interesting quirk is that, if the close paren is missing, the directive will continue to be parsed on later lines until a close paren is found (and extra text is silently ignored).

I will commit the examples into testsuite/bsc.preprocessor/misc/.

quark17 commented 1 year ago

There are further issues in SystemVerilogPreprocess.lhs. The pattern warnings pointed to the handling of `include directives (in the code here). If the closing delimiter of the file name is missing, a GHC error results. I will add a test to testsuite/bsc.preprocessor/include/. Some of the calculation of updated positions seems wrong -- if we want to be thorough, we could add tests for the updated positions.

Another warning is in the handling of `define (here). This has to do with not handling EOF at the end of a comment line.

Another warning is in the further handling of `line (here). This code tracks open and close parentheses, and it reports an internal error in situations where an end of line is reached without a closing paren -- this could be OK, if earlier code has already guaranteed a close paren (or the parens are auto-generated by BSC, in the case of `line directives), but otherwise should be user errors.