veryl-lang / veryl

Veryl: A Modern Hardware Description Language
Other
439 stars 20 forks source link

constant_expression #788

Open nblei opened 2 weeks ago

nblei commented 2 weeks ago

SV contains the notion of a constant_expression :

image

This is used in quite a few different parts of the grammar, including pattern matching, generate statements, enum declarations, port declarations, concatenations, etc.

I think it would be a good idea to replicate this in the Veryl syntax. Ensuring constness at the syntax level where it is highly documented and checked may be very useful. Further, this helps ensure that Veyl code will transpile to correct SystemVerilog code.

dalance commented 2 weeks ago

I think it is diffucult problem whether an invalid code should be rejected syntactically or semantically.

In my experience of implementing a SystemVerilog parser (https://github.com/dalance/sv-parser), one of the most complicated issues was expression and constant_expression. expression apears whole over the syntax, and it is large syntax including recursive and operator precedence. constant_expression causes duplication of the large syntax, and increses difficulty of parser implementation.

Additionally, there is readability issue of error message. Syntax error essentially has the form like "XXX token is expected, but YYY token was found". In constast to this, semantic error can provide more readable messages like "This expression is not constant, because variable XXX is not constant".

Therefore I chose the combination of simple syntax which accepts some invalid code and semantic checker which rejects the invalid code.

nblei commented 2 weeks ago

I agree with you that SV syntax is overly verbose.

I do, though, think a ConstantExpression: Expression production can be useful at low cost.

While this doesn't perform const checking at parsing, it does make it easy to implement the checking via a walker and implementing fn constant_expression.

dalance commented 2 weeks ago

Thanks. I misunderstood it like that you meant const_expression should be separated from expression.

I agree that ConstantExpression: Expression is useful to ease semantic check.