Re: [PATCH] Factor %FLAG at scan level.

[Joel E. Denny]

On Mon, 6 Apr 2009, Akim Demaille wrote:

> Some day, I would really like to find some means for the user to pass some
> yytext to yyerror_syntax error, so that we can have the genuine look-ahead
> string reported in the error message.

I think that'll be solved when we finally implement %error-report to 
replace yyerror.  That is, the user can then access anything that a 
semantic action can access without dealing with a fluctuating yyerror 
argument list.

> > Besides, I
> > vaguely recall that %printer is eventually going to be used in syntax
> > error messages.  Is that right?
> 
> You're reading my mind even before I started thinking.  Amazing trick :)
> Well, there are a number of issues with %printer that I'd like to discuss at
> some point, but basically it boils down to the fact that it's bound to the
> parser itself, and now I regret this.  In the C++ parsers there is now a
> symbol_type that has (type, value, location).  I plan to import this in the C
> skeletons too, but I don't know when.  I wish I had a proper operator<< and a
> proper destructor for this type, but I can't because %printer and %destructor
> explicitly provide the user with an access to the parser (in particular
> %parse-params).

I can see how users with existing grammar files with existing %printer's 
would benefit if Bison automatically provided an operator<< for 
symbol_type that invoked those %printer's.  Even though that approach is 
not actually possible, the user still has an opportunity to share code 
between the two by specifying that %printer invoke operator<< instead.  
If we make the symbol_type object directly accessible in %printer, the 
user can even write a single %printer that invokes operator<< for all 
symbols, which are specified by <*> and <>.

> > > - should -Derror-verbose and -Derror_verbose be the same?
> > 
> > What if we just accept "%define error-verbose"?  I'd like to convert
> > api.push_pull and lr.keep_unreachable_states to use dashes as well.
> 
> Yes, I agree, that looks much nice.  Will do (at some point :).

Maybe we should go ahead and rename api.push_pull and 
lr.keep_unreachable_states in 2.5.  I'd also like to formally state that 
we don't plan to remove the old names.  Otherwise, users might get very 
frustrated that we can't make up our minds.

> I'm not good at naming, so I'm eager to read any
> suggestion about the names to use.   I should write the documentation, that
> would help;

So far, these are documented (or will be soon) in Decl Summary in the 
manual, where I think all %define variables should be mentioned:

  api.pure (Boolean)
  api.push-pull (pull, push, both)
  lr.type (LALR, IELR, or canonical)
  lr.default-rules (full, consistent-states, accept)
  lr.keep-unreachable-states (Boolean)
  namespace

In most cases, I use namespaces to make the purpose clearer.  Ironically, 
we didn't think of a good namespace for namespace.

> in the meanwhile:
> 
> - %define assert (lalr1.cc)
>   additional runtime assertions to catch non-initialized $$.
>
> - %define debug
>   %debug
>
> - %define error_verbose
>   %error-verbose

These all describe different aspects of the run-time parser behavior.  
Maybe they should be:

  parse.assert
  parse.debug
  parse.error-verbose

Is assert too general?  Will there ever be other kinds of parser 
assertions that should be controlled independently?

> - %define lex_symbol
>   yylex returns a symbol_type instead of taking pointers to value/location and
> returning the type.

I would prefer:

  api.lex-symbol

This could not be used in conjunction with api.pure, right?  I mean, it 
implies purity, so specifying api.pure is illogical.

> - %define locations
>   %locations

This makes an api change and adds a new task to the parse, so I'm not sure 
any specific namespace is appropriate.

> - %define variant (lalr1.cc, and glr.cc in my branch)
>   Use variants instead of union.

Maybe use "variants" for consistency with "locations".  That fits the 
phrase "use variants".  Is it appropriate to think of this as an api 
change?

> > > - we should probably also have priority management so that the grammar
> > > file
> > > can declare %define debug, but be overriden by -Ddebug=false.
> > 
> > Agreed.  We need to generalize Paolo's skeleton_arg and language_argmatch
> > functions.  There was some discussion on correct priority some time ago.
> 
> I don't remember.  I'll look for it.

I found it:

  http://lists.gnu.org/archive/html/bison-patches/2006-12/msg00065.html

But I think the main revelation is below....

> > My recollection is that the command line should have higher priority,
> 
> Yes, definitely.
> 
> > but
> > there should be a warning if a grammar declaration contradicts the command
> > line.  If that's too harsh, the warning could be disabled.
> 
> Hum.  Yes, let's have it disabled by default.

I think there are cases where it would be more helpful if Bison enabled 
the warning by default.

Let's say the user is given a makefile that specifies --skeleton=lalr1.cc.  
Without realizing that's in the makefile, he adds %skeleton "my-lalr1.cc" 
to the grammar file and types make.  It may take him some time to realize 
why the changes he made to lalr1.cc to create my-lalr1.cc have no effect 
on his test cases.  I think Bison needs to be sure to warn about the 
contradiction.

Another example is where the user specifies --defines=token-defs.h without 
realizing that the code base already uses a defines file specified by 
%defines "parser.h".  If the user is lucky, a compilation error results.  
If he's not lucky and an old version of parser.h is still lying around, 
parser.h would be included everywhere and any changes in the definitions 
would not take effect.  Again, I think Bison needs to be sure to warn 
about the contradiction.

Instead of allowing the user to disable the warning globally, I think 
Bison can provide a more careful and flexible solution.  Bison would warn 
if --define contradicts %define, but Bison would not warn if 
--force-define contradicts %define.

With that solution in place, it might be fine to convert the --define 
warning to an error to make sure it's noticed.

What do you think?