[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: More comments, and the ANTLR code is too complex



On 6/10/13, David A. Wheeler <dwheeler@xxxxxxxxxxxx> wrote:
> There are many ways to try to make the ANTLR code simpler.
>
> One approach is to break up rules into smaller rules.  It's easier to
> *implement*
> some of these rules as a long sequence of conditions, but if that
> presentation
> makes it too hard to explain, then we can of course change it.
> Also, we could remove some of the "greedy=true" statements (which are used
> to eliminate some warning messages).
>
> Below is a first shot at breaking up it_expr, currently 1 long rule, into 2
> rules.
> This could obviously be repeated to make more rules, each one simpler.
> Not saying it's done, but would it help to break the current longer rules
> into more but smaller rules?
>
> --- David A. Wheeler
>
>
> it_expr_normal returns [Object v]
>   : head
>      GROUP_SPLIT hspace*
>       (comment_eol error
>        | /*empty*/ {(monify $head)} )
>      | SUBLIST hspace*
>        (sub_i=it_expr {(append $head (list $sub_i))}
>         | comment_eol error )
>      | comment_eol // Normal case, handle child lines if any:
>        (INDENT children=body {(append $head $children)}
>         | /*empty*/          {(monify $head)} /* No child lines */ )
>     ;
>
> it_expr_prefixed returns [Object v]
>   : (GROUP_SPLIT | scomment) hspace* /* Initial; Interpet as group */
>       (group_i=it_expr {$group_i} /* Ignore initial GROUP/scomment */
>        | comment_eol
>          (INDENT g_body=body {$g_body} /* Normal GROUP use */
>           | same ( g_i=it_expr {$g_i} /* Plausible separator */
>                    /* Handle #!sweet EOL EOL t_expr */
>                    | comment_eol restart=t_expr {$restart} )
>           | DEDENT error ))
>   | SUBLIST hspace* /* "$" first on line */
>     (is_i=it_expr {(list $is_i)}
>      | comment_eol error )
>   | abbrevw hspace*
>       (comment_eol INDENT ab=body
>          {(append (list $abbrevw) $ab)}
>        | ai=it_expr
>          {(list $abbrevw $ai)} ) ;
>
> it_expr returns [Object v]
>   : it_expr_normal   {$it_expr_normal}
>   | it_expr_prefixed {$it_expr_prefixed} ;
>
>

It *might*.  I suggest we need better names.  We don't really need to
prefix with "it_expr", for example.  So maybe "normal_it_expr" and
"special_it_expr" instead.

Having more rules helps in discussing rules, and may help suggest how
to organize a top-down recursive descent parser.

However, we might want to ask Mark H. Weaver directly if this helps
clarify the BNF.

Sincerely,
AmkG