snarkVM icon indicating copy to clipboard operation
snarkVM copied to clipboard

Remove two superfluous calls in `.aleo` parser.

Open acoglio opened this issue 3 years ago • 1 comments

The parsing of the two required entries of record types (owner and gates) was doing a superfluous call to Sanitize::parse() (which skips over comments and whitespace) just before parsing the entry type. But this call was already preceded by a call to Sanitize::parse_whitespace(), which is the appropriate one in this part of the syntax, consistently with the parsing of other (i.e. non-required) record entries in the parse_entry function.

acoglio avatar Oct 13 '22 01:10 acoglio

Yes, we are indeed losing the ability to have comments in that place, but it is consistent with other places in the parser. More detail below.

The current parser allows whitespace and comments only in certain places, essentially just before the constructs that are normally written on one line of text: before each instruction, before each import, before each input or output, etc. In the other places, it only allows whitespace (e.g. between operands). This can be seen more easily in this grammar, which has been derived from the parser and should match its workings: cws (= comment and whitespace) corresponds to Sanitize::parse(), while ws (= whitespace) corresponds to Sanitize::parse_whitespace().

But there are currently a few exceptions to this general pattern. This PR removes one of these exceptions.

There would be of course nothing wrong with allowing comments in more places, maybe even everywhere (i.e. replacing all ws with cws in the grammar). We could even structure the grammar in two levels (lexical and syntactic); I think that the nom parser could realize that still in a single pass, via suitable parser combinators. Or, going further in the other direction, we could modify the grammar and parser to put more requirements on layout. But all of this is a separate design decision.

For now, this PR is consistent with the idea of allowing comments and whitespaces at the beginning of "lines", and just whitespace in the middle of these "lines".

acoglio avatar Oct 13 '22 19:10 acoglio