Complete implementation for handling #define directive.

Complete implementation for handling #define directive.
Committed: http://code.google.com/p/angleproject/source/detail?r=752

[kbr1, 2011-09-07 23:14:07]

LGTM. Only one tiny question.

http://codereview.appspot.com/4963062/diff/2001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/pp.y (right):

http://codereview.appspot.com/4963062/diff/2001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/pp.y:160: }
Is there a more concise way to express the conditional spaces on each side of
the comma?

[Alok Priyadarshi, 2011-09-12 05:45:49]

Parser does not handle whitespace anymore.

http://codereview.appspot.com/4963062/diff/2001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/pp.y (right):

http://codereview.appspot.com/4963062/diff/2001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/pp.y:160: }
On 2011/09/07 23:14:07, kbr1 wrote:
> Is there a more concise way to express the conditional spaces on each side of
> the comma?

I realized that since GLSL does not support stringify(#) or paste(##) operators,
there is no need to preserve whitespace. In the new patch the lexer eats
whitespaces making the parser much simpler.

[kbr1, 2011-09-12 18:23:02]

LGTM again.

http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/pp.l (right):

http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/pp.l:74: {HASH}undef{HSPACE}+ { return HASH_UNDEF;
}
Why does this end in a trailing {HSPACE}+? It seems to me that either that is
unnecessary, or it should end in {IDENTIFIER}.

[Alok Priyadarshi, 2011-09-12 18:29:45]

http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/pp.l (right):

http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/pp.l:74: {HASH}undef{HSPACE}+ { return HASH_UNDEF;
}
On 2011/09/12 18:23:02, kbr1 wrote:
> Why does this end in a trailing {HSPACE}+? It seems to me that either that is
> unnecessary, or it should end in {IDENTIFIER}.

This is here to differentiate between "#undeffoo" and "#undef foo". I could make
it end in {HSPACE}+{IDENTIFIER} but then will have to extract the macro name.

[kbr1, 2011-09-12 18:36:28]

On 2011/09/12 18:29:45, Alok Priyadarshi wrote:
>
http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
> File src/compiler/preprocessor/new/pp.l (right):
> 
>
http://codereview.appspot.com/4963062/diff/6001/src/compiler/preprocessor/new...
> src/compiler/preprocessor/new/pp.l:74: {HASH}undef{HSPACE}+ { return
HASH_UNDEF;
> }
> On 2011/09/12 18:23:02, kbr1 wrote:
> > Why does this end in a trailing {HSPACE}+? It seems to me that either that
is
> > unnecessary, or it should end in {IDENTIFIER}.
> 
> This is here to differentiate between "#undeffoo" and "#undef foo". I could
make
> it end in {HSPACE}+{IDENTIFIER} but then will have to extract the macro name.

Not necessary. Thanks for the info. Please go ahead and land. (Actually, please
also update BUILD_REVISION in src/common/version.h to track the current SVN
version.)

[Alok Priyadarshi, 2011-09-12 18:38:17]

> Not necessary. Thanks for the info. Please go ahead and land. (Actually,
please
> also update BUILD_REVISION in src/common/version.h to track the current SVN
> version.)

Do we need to do that even for code that does not build yet?

[kbr1, 2011-09-12 18:42:36]

On 2011/09/12 18:38:17, Alok Priyadarshi wrote:
> > Not necessary. Thanks for the info. Please go ahead and land. (Actually,
> please
> > also update BUILD_REVISION in src/common/version.h to track the current SVN
> > version.)
> 
> Do we need to do that even for code that does not build yet?

Well, from a practical standpoint, probably not.

[dgkoch, 2011-09-13 12:47:54]

LGTM (for the record)