[PATCH] Replaced bool parameters in SkipUntil function with single bit-based parameter.

[Hal Finkel]

----- Original Message -----
> On 06/11/2013 10:10, David Majnemer wrote:
> >   From my experience,
> >   `SkipUntil(EndKind, /*StopAtSemi=*/true, /*DontConsume=*/true);`
> >   is more typical in clang than
> >   `SkipUntil(EndKind, StopAtSemi | DontConsume);`
> >
> >   It seems that some of the calls that you changed were previously
> >   nasty (i.e. `SkipUntil(EndKind, true, true)`) which justifies a
> >   cleanup.
> >   However I'm not sure we want a bitfield here.
> >
> >   What is your justification?
> 
> I like this new SkipUnti(). It's a step in the right direction for
> the
> parser, and a lot more approachable than the rows of bools.
> 
> The old comments-before-arguments trick was never kept up to date and
> too easy to mix up.
> 
> Small nitpick, The names of DontConsume and NoSkipUntilFlags are a
> bit
> jarring (maybe NoConsume would do for the first one?)

I agree. Also, can you please invert the sense of the StopAtSemi option so that 0 can be the default. That way we never need 'NoSkipUntilFlags' in the code.

 -Hal

> 
> Alp.
> 
> 
> >
> >
> > ================
> > Comment at: include/clang/Parse/Parser.h:751
> > @@ -741,3 +750,3 @@
> >    /// token will ever occur, this skips to the next token, or to
> >    some likely
> > -  /// good stopping point.  If StopAtSemi is true, skipping will
> > stop at a ';'
> > -  /// character.
> > +  /// good stopping point.  If Flags has bit set at StopAtSemi,
> > skipping will
> > +  /// stop at a ';' character.
> > ----------------
> > The wording "has bit set" seems strange.
> >
> >
> > http://llvm-reviews.chandlerc.com/D2108
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> --
> http://www.nuanti.com
> the browser experts
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory