On Fri, 2010-12-03 at 15:37 -0800, Carl Worth wrote:
> On Fri, 3 Dec 2010 13:34:09 -0800, Kenneth Graunke <kenn...@whitecape.org>
> wrote:
> > On Friday 03 December 2010 08:01:06 José Fonseca wrote:
> > > parser->space_tokens = 1;
> > >
> > > statement conditional_tokens stanza -- it causes space tokens to be
> > > emitted halfway before reaching the DEFINED token.
> >
> > Indeed. I'm not sure why conditional_tokens sets this. That code was
> > pretty
> > much copy & pasted from pp_tokens, too, so I'm wondering if it can be
> > removed
> > from there as well.
> >
> > Carl, can we safely remove this line from conditional_tokens? What about
> > pp_tokens?
>
> The current glcpp test suite (glcpp-test) says it's safe to remove it
> From conditional_tokens. Take that for what it's worth I guess...
>
> It's definitely not safe to remove it from pp_tokens, (unless you also
> make some other changes). If you just remove that you'll find that
> something like:
>
> #define foo bar baz
> foo
>
> will result in "barbaz" rather than "bar baz" as desired.
>
> One option here that might simplify the implementation considerably is
> to not include space tokens in the replacement list like this, but to
> instead simply insert spaces between all tokens when printing the
> resulting stream.
>
> That would cause some minor expansion of the results. For example input
> of:
> (this,that)
> would become:
> ( this , that )
>
> But that probably doesn't really harm much.
>
> During development of the preprocessor it has been *very* convenient to
> not do expansion like this so that results could be verified based on
> direct comparison with output from "gcc -E", (and attempts to test with
> diff but ignoring amounts of whitespace proved error prone). Now that
> the test suite uses its own files of expected output, this issue
> probably doesn't exist.
>
> > > I don't quite understand space_tokens' role here, or how it is supposed
> > > to work. I confess I'm not very familiar with bison/flex inner-workings:
> > > I though that the tokenizer was producing tokens quite ahead of the
> > > parser, so the parser couldn't change the behavior of the tokenizer in
> > > flight -- but perhaps I'm mixing up with other lexel/grammar generation
> > > tools.
>
> The parser can exert some influence over the tokenizer. But doing this
> reliably is difficult since the tokenizer can do read-ahead. I
> definitely agree that it would be greatly preferable to have the
> tokenizer and parser more cleanly separated. But various "messy" aspects
> of the C preprocessor language do make this difficult.
>
> > Apparently it can. I believe Ian's assessment is right.
>
> The distinction between "#define foo ()" and "#define foo()" is a case
> where the C preprocessor language makes whitespace significant. But the
> current space_tokens control is not used for this. Instead the lexer is
> currently handling this distinction itself.
>
> So space_tokens is really extra code to try to get really clean output,
> (without extra space expansion as shown above). If removing space_tokens
> makes the implementation much cleaner then the slightly less clean
> output might be very well worth it.
>
> I'll leave that for you to decide, Ken.
I suppose it is possible for space to be significant on a conditional
expression, e.g.,
#if foo ()
vs
#if foo
If there isn't an obvious cleaner solution, would you mind I commit my
original patch:
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index 8475e08..3b84c06 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -462,6 +462,10 @@ conditional_token:
int v = hash_table_find (parser->defines, $2) ? 1 : 0;
$$ = _token_create_ival (parser, INTEGER, v);
}
+| DEFINED SPACE IDENTIFIER {
+ int v = hash_table_find (parser->defines, $3) ? 1 : 0;
+ $$ = _token_create_ival (parser, INTEGER, v);
+ }
| DEFINED '(' IDENTIFIER ')' {
int v = hash_table_find (parser->defines, $3) ? 1 : 0;
$$ = _token_create_ival (parser, INTEGER, v);
It doesn't makes things any worse, and it would allow us to focus on
other problems.
Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
