bison-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] More %define/%code encapsulation


From: Joel E. Denny
Subject: Re: [PATCH] More %define/%code encapsulation
Date: Wed, 17 Jan 2007 15:35:47 -0500 (EST)

On Wed, 17 Jan 2007, Paolo Bonzini wrote:

> > How about b4_percent_define_if_true instead?  Given that we have _ifdef, _if
> > seems like it ought to be like m4_if, which is not about booleans.
> 
> Yes.  OTOH we also have b4_flag_if.  Maybe b4_percent_define_flag_if

Fine by me.

> but
> these long macro names are getting more and more annoying to me.
>
> > > - for _ifval, I'm thinking of
> > > 
> > > m4_define([b4_class_name], [b4_get_percent_code([package])dnl
> > > b4_get_percent_code_ifval([package], [.])$1])
> > 
> > Why not the following?
> > 
> >   m4_define([b4_class_name], [b4_percent_code_get([[package]])dnl
> >   m4_ifval(b4_percent_code_get([[package]]), [.])$1])
> 
> Well, just because!  But if I want to be nitpicking, these are my reasoning:
> 
> - you "save more characters" than you show because it's b4_percent_code_ifval
> (you have one more _get).  If the names were shorter (omitting the _percent,
> for example...) the savings would be close to 50%!
>
> - one less pair of parentheses.  m4 has already too many.

I'm more worried about code being intuitive to developers a few years from 
now than about saving a few keystrokes now.

> - quoting the if macros correctly is somewhat counterintuitive.

I fail to see how this:

  b4_get_percent_code_ifval([[package]], [.])

is better than this:

  m4_ifval(b4_percent_code_get([[package]]), [.])

In my opinion, if you want to make life easier for skeleton authors, give 
them less macros to understand.  If they can't grasp how to compose small 
concepts into larger ones, they're in the wrong business, and they should 
at least stay away from any UNIX-like OS.  As far as M4 quoting, they're 
going to have to understand it anyway if they're working with Bison's 
skeletons.  If we try to specialize every m4_FOO for every Bison 
construct, we're going to gradually create a large mess in bison.m4.  
Keep it simple.  Cut the fat now.

> > To be clear, you mean that the no-value %define form:
> > 
> >   %define my_boolean_variable
> > 
> > is equivalent to:
> > 
> >   %define my_boolean_variable "1"
> > 
> > but you want it equivalent to:
> > 
> >   %define my_boolean_variable ""
> 
> Using "1" for true is needlessly C-like.  In m4 (but also in the C
> preprocessor) it's common that "ifdef" marks whether a flag if true or false,
> without looking at the actual value.  Since the grammar's source code will not
> change, and the default is not documented (%define is not documented at
> all!!!), I find this change valuable.
>
> Also, using a default of "" gives the least-surprise (compare with C) when the
> no-value form is used with a non-boolean variable.

Since none of this appears to be documented, why don't we just drop the 
no-value form?  Then there's no potential for surprise at all.  If the 
user wants the empty string, he should specify the empty string.  If he 
wants 1, he should specify 1.  If he wants true, he should specify true.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]