Make the new preprocessor backwards compatible with Bison 2.3.

Make the new preprocessor backwards compatible with Bison 2.3.

[mvujovic, 2012-07-12 21:17:30]

Hi Alok,

Could you review this please?

This patch makes ExpressionParser.y consumable by both Bison 2.3 and Bison
2.4.2.

I put up the Bison 2.3 generated code here to show one place where it created a
naming conflict, described below. I will actually commit the Bison 2.4.2
generated code.

Thanks!
Max

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
File src/compiler/preprocessor/new/ExpressionParser.cpp (right):

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
src/compiler/preprocessor/new/ExpressionParser.cpp:90: #pragma warning(disable:
4065 4701)
Bison 2.3 creates these #define directives that conflict with the enum names
defined in Token.h, which is included later in this file. I prefixed them with
"TOK_" to avoid the conflict. (Bison 2.4.2 does not create these #define
directives and doesn't have this problem).

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
File src/compiler/preprocessor/new/ExpressionParser.y (right):

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
src/compiler/preprocessor/new/ExpressionParser.y:59: %pure-parser
Use %pure-parser instead of %define api.pure.

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
src/compiler/preprocessor/new/ExpressionParser.y:69: %token TOK_CONST_INT
Prefix these token names with "TOK_" to avoid a naming conflict with the enum
names in Token.h.

[kbr1, 2012-07-12 21:26:07]

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
File src/compiler/preprocessor/new/ExpressionParser.y (right):

https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
src/compiler/preprocessor/new/ExpressionParser.y:59: %pure-parser
On 2012/07/12 21:17:30, mvujovic wrote:
> Use %pure-parser instead of %define api.pure.

You're sure this has the desired effect in both versions of Bison?

[mvujovic, 2012-07-12 21:42:37]

On 2012/07/12 21:26:07, kbr1 wrote:
>
https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
> File src/compiler/preprocessor/new/ExpressionParser.y (right):
> 
>
https://codereview.appspot.com/6356098/diff/2001/src/compiler/preprocessor/ne...
> src/compiler/preprocessor/new/ExpressionParser.y:59: %pure-parser
> On 2012/07/12 21:17:30, mvujovic wrote:
> > Use %pure-parser instead of %define api.pure.
> 
> You're sure this has the desired effect in both versions of Bison?

From what I understand, yes.

%define api.pure was introduced in Bison 2.3b (which is GPLv3, btw). Here's the
note from the GNU mailing list email introducing it:
"""
* The directive `%pure-parser' is now deprecated in favor of:

    %define api.pure

  which has the same effect except that Bison is more careful to warn about
  unreasonable usage in the latter case.
"""
(http://lists.gnu.org/archive/html/info-gnu/2008-05/msg00015.html)

The current Bison 2.5 manual similarly says:
"""
— Directive: %pure-parser
Deprecated version of %define api.pure (see api.pure), for which Bison is more
careful to warn about unreasonable usage.
"""
(http://www.gnu.org/software/bison/manual/html_node/Decl-Summary.html)

Btw, I did run the preprocessor tests without issue. I'm not quite sure how I
could verify the non-reentrancy of the parser in a test.

[Alok Priyadarshi, 2012-07-12 21:46:46]

There is something funny going on when I try to view the diff for
ExpressionParser.cpp. Could you please upload the final patch once again.

[mvujovic, 2012-07-12 21:54:52]

On 2012/07/12 21:46:46, Alok Priyadarshi wrote:
> There is something funny going on when I try to view the diff for
> ExpressionParser.cpp. Could you please upload the final patch once again.

Hmm. The Side-by-side diff is broken for ExpressionParser.cpp, but the if you
click the filename, the Unified diff works at least.

I tried making a new ChangeList from scratch and uploading in a new issue, but
it still has the same issue (https://codereview.appspot.com/6353096).

[mvujovic, 2012-07-12 21:58:37]

On 2012/07/12 21:54:52, mvujovic wrote:
> On 2012/07/12 21:46:46, Alok Priyadarshi wrote:
> > There is something funny going on when I try to view the diff for
> > ExpressionParser.cpp. Could you please upload the final patch once again.
> 
> Hmm. The Side-by-side diff is broken for ExpressionParser.cpp, but the if you
> click the filename, the Unified diff works at least.
> 
> I tried making a new ChangeList from scratch and uploading in a new issue, but
> it still has the same issue (https://codereview.appspot.com/6353096).

Looks like there's on open bug on Rietveld for the "Can't parse the patch to
chunks" message:
http://code.google.com/p/rietveld/issues/detail?id=92

My local repo is in sync, which is what that bug suggests to check.

[mvujovic, 2012-07-12 22:55:03]

On 2012/07/12 21:58:37, mvujovic wrote:
> On 2012/07/12 21:54:52, mvujovic wrote:
> > On 2012/07/12 21:46:46, Alok Priyadarshi wrote:
> > > There is something funny going on when I try to view the diff for
> > > ExpressionParser.cpp. Could you please upload the final patch once again.
> > 
> > Hmm. The Side-by-side diff is broken for ExpressionParser.cpp, but the if
you
> > click the filename, the Unified diff works at least.
> > 
> > I tried making a new ChangeList from scratch and uploading in a new issue,
but
> > it still has the same issue (https://codereview.appspot.com/6353096).
> 
> Looks like there's on open bug on Rietveld for the "Can't parse the patch to
> chunks" message:
> http://code.google.com/p/rietveld/issues/detail?id=92
> 
> My local repo is in sync, which is what that bug suggests to check.

Sorry, Alok. I can't get the side-by-side diff to work easily. I just tried a
git patch instead of svn and using Bison 2.3 in CygWin, but I also had issues.

[Alok Priyadarshi, 2012-07-13 17:35:30]

Could you just send me the new ExpressionParser.cpp generated using bison 2.3. I
am curious to see why renaming of tokens was necessary, not a big deal btw, just
curious.

[mvujovic, 2012-07-13 17:39:43]

On 2012/07/13 17:35:30, Alok Priyadarshi wrote:
> Could you just send me the new ExpressionParser.cpp generated using bison 2.3.
I
> am curious to see why renaming of tokens was necessary, not a big deal btw,
just
> curious.

Absolutely. I'll send it to you shortly.

[Alok Priyadarshi, 2012-07-13 17:56:50]

lgtm

Please make sure to check-in the file generated by Bison 2.4.2 into ANGLE.

[mvujovic, 2012-07-13 18:46:32]

On 2012/07/13 17:56:50, Alok Priyadarshi wrote:
> lgtm
> 
> Please make sure to check-in the file generated by Bison 2.4.2 into ANGLE.

Thanks, Alok. Committed Bison 2.4.2 ExpressionParser.cpp and ExpressionParser.y
in r1224.