-
Notifications
You must be signed in to change notification settings - Fork 23
Description
I'm new to aa - starting by reading the README.md. I have the following issues/questions with the grammar described there:
-
Error in
term = tfact bop- stmts bop- stmt- that firstbop-should bebop+? -
Possible error in
term = tfact bop- stmts bop-(prior line): wouldn't that admit anexpr(thereforestmt) where the last symbols are[:]=(per definition ofbop- = ] ]= ]:=)?bop-should be factored into a sole]and abop-compound = ]= ]:=or actually better simply maketerm = tfact bop- stmts bop- [:]= stmtas the rule for arity-3 balanced/split operator. -
For readability in
tfun = {[[type]* ->]? type }introduce a space after the{(this grammar notation is already a bit confusing - though nicely terse! - w.r.t. terminal symbols, especially[and]which are significant tokens in both the grammar and in the target language). Similarly for the(in thettuplerule and the}in thetstructrule. -
Style suggestion? I'm uncomfortable with seeing
ifex = apply [? stmt [: stmt]]. It seems to "promote" the "trinary logic" expression to a level of importance even "higher" than apply. (Not sure quite how to say this, but I'm thinking in terms of the way a user might read the grammar, rather than how an implementer would want it to produce a certain parse tree.) My suggestion would be to factor that rule into 3 rules (similarly to the way the balanced/split operator is factored, e.g.,ifex = apply ifex = apply ? stmt ifex = apply ? stmt : stmt(And if you do that perhaps there's a better name than
ifexwhich IMO enhances the "importance" of the "trinary logic" expression in the eyes of the reader even more than the grammar rules ...) -
Style approval: I especially like
expr = term [binop term]*with the comment "gather all the binops and sort by prec" - exactly! (And same for unary ops.) A shame C didn't do this and C++ followed - embedding binary operator precedence (and in some cases associativity) in the presentation grammar is a mistake IMO, just massively complicates readability of the presentation grammar, and if you have a lot of built-in operators (e.g., C, C++) you need a table for the reader anyway, and then so as long as you have precedence (and associativity) in a separate table - use that table as the definition, not the grammar. (I'm making a distinction between the "presentation" or "reference" grammar and whatever grammar the tooling uses internally which can be different as long as ultimately the tooling accepts the exactly the same language.) -
By the way, speaking of associativity, looking ahead to the examples for short circuit operators you have a note about relative precedence of
&&and||. In my long experience doing code reviews and troubleshooting production problems on code that previously (may have) gone through code review: Unparenthesized expressions involving a mix of&&and||frequently do not do what the programmer thought and most often aren't exhaustively tested in a way that would reveal their flaws. Leaving it to the sad guy trying to solve a ticket. I would strongly suggest that the precedence rules place&&and||on the same precedence level and make mixes of those operators have no associativity! Thereby forcing the programmer to either split the mixed expression (probably a good idea) or at minimum add parenthesis. (I'm not convinced by arguments that suggest that "sum of products" is a well understood convention, or anything of that sort.) My opinion, FWIW, but I consider it important in a language aimed at systems programming and low-level performance code.