Re: Don't hardcode a limited set of markup signatures. (issue969046)

[dak]

http://codereview.appspot.com/969046/diff/7001/8002
File lily/lexer.ll (right):

http://codereview.appspot.com/969046/diff/7001/8002#newcode545
lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS.
On 2010/05/01 19:56:08, hanwenn wrote:
> wouldnt it be clearer to have a function

>    void translate_markup_signature(SCM predicate_list,
>       vector<int> expect_tokens);

> which generates the [expect_scm, expect_scm, expect_markup] (possibly
with
> expect_no_more_args) in a vector, and then push the tokens one by one
in the
> desired order?

The tokens _are_ pushed one by one in the desired order.  So it makes no
sense to allocate additional storage just to do the same job.

> Also, I think it is better to not use fall-through switches, as they
are a
> uncommon and tricky idiom.

Sigh.  We have essentially a state machine where the state is determined
by the kind of already processed tokens, and depending on the state of
the current token, we transition to a new state with different allowed
new tokens/transitions.

This sort of state machine can usefully only programmed using goto
(unless one uses explicit state tables which are even less readable).
The switch statement is basically the same as gotoizing a state machine,
with the case labels substituting for goto labels.

Most state machines (including this one) don't have a structure readily
represented in linear structured code.

Moving them into a separate procedure does not change that.

My changes already make for the bulk of comment lines in this file.  So
I decided against commenting this even more in violating of the
established standards in the rest of the file.

Instead I shifted the responsibility of checking the order of arguments
to definition time instead of runtime.  It has the advantage of
triggering prospective errors at a more expected point of time.

http://codereview.appspot.com/969046/diff/7001/8002#newcode545
lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS.
On 2010/05/01 19:56:08, hanwenn wrote:
> wouldnt it be clearer to have a function

>    void translate_markup_signature(SCM predicate_list,
>       vector<int> expect_tokens);

> which generates the [expect_scm, expect_scm, expect_markup] (possibly
with
> expect_no_more_args) in a vector, and then push the tokens one by one
in the
> desired order?

> Also, I think it is better to not use fall-through switches, as they
are a
> uncommon and tricky idiom.

> I would expect something like

>   if(pred == bla("markup?")
>     token = EXPECT_MARKUP;
>   else if (pred == bla("markup-list?")
>     token = EXPECT_MARKUP_LIST;
>   else if (pred == bla("scm?")
>     token = EXPECT_SCM;



Done.

http://codereview.appspot.com/969046/show