Eliminate the use of %code requires to restore compatibility with older Bison versions.

Eliminate the use of '%code requires' to restore compatibility with older Bison versions.

BUG=449

[Alok Priyadarshi, 2013-08-29 16:41:06]

Minor comments.

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.l
File src/compiler/glslang.l (right):

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.l#newcode45
src/compiler/glslang.l:45: #include "compiler/Common.h"   // Defines TSourceLoc
Is there a reason why Common.h is included after glslang_tab.h?

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.l#newcode46
src/compiler/glslang.l:46: #define YYLTYPE TSourceLoc
nit: add a space between includes and defines.

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode41
src/compiler/glslang.y:41: #include "compiler/Common.h"   // Defines TSourceLoc
this works although unfortunate that the same code-block needs to be duplicated.
Can we put this code block into a header file that can be included in both .l
and .y files?

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode43
src/compiler/glslang.y:43: #define YYLTYPE_IS_DECLARED 1
FYI: For one of the in-progress refactoring, I will need to #define
SH_MAX_TOKEN_LENGTH here as well.

[Jamie Madill, 2013-10-02 15:43:19]

On 2013/08/29 16:41:06, Alok Priyadarshi wrote:
> Minor comments.
> 
> https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y
> File src/compiler/glslang.y (right):
> 
>
https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode41
> src/compiler/glslang.y:41: #include "compiler/Common.h"   // Defines
TSourceLoc
> this works although unfortunate that the same code-block needs to be
duplicated.
> Can we put this code block into a header file that can be included in both .l
> and .y files?

Agree here.

>
https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode43
> src/compiler/glslang.y:43: #define YYLTYPE_IS_DECLARED 1
> FYI: For one of the in-progress refactoring, I will need to #define
> SH_MAX_TOKEN_LENGTH here as well.

SH_MAX_TOKEN_LENGTH is a bit of an oddity. In ES3 there is also a max "symbol"
name length of 1024, not sure it that is the same as "token" length and if we
have to support both for the same purpose.

[nicolas, 2013-10-02 21:10:01]

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.l
File src/compiler/glslang.l (right):

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.l#newcode45
src/compiler/glslang.l:45: #include "compiler/Common.h"   // Defines TSourceLoc
On 2013/08/29 16:41:06, Alok Priyadarshi wrote:
> Is there a reason why Common.h is included after glslang_tab.h?

Common.h is where TSourceLoc is defined, which I wanted to be as close as
possible to its use in the YYLTYPE definition below. It is also why I didn't add
a newline between the include and define. I hope that makes sense. There's no
ideal way to do this without %code requires and this seemed the best alternative
to me that is as clear as possible.

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y
File src/compiler/glslang.y (right):

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode41
src/compiler/glslang.y:41: #include "compiler/Common.h"   // Defines TSourceLoc
On 2013/08/29 16:41:06, Alok Priyadarshi wrote:
> this works although unfortunate that the same code-block needs to be
duplicated.
> Can we put this code block into a header file that can be included in both .l
> and .y files?

Yes this could be put in a header file but it would be really small and not
obvious what goes on in there unless you open it. Since it defines YYLTYPE it
changes the behavior of the lexer and parser so it seemed appropriate to make
that very explicit in each of them. Ideally it should only be defined in the .y
file but unfortunately that only worked with "%code requires" which is precisely
what had to be eliminated.

https://codereview.appspot.com/13339044/diff/1/src/compiler/glslang.y#newcode43
src/compiler/glslang.y:43: #define YYLTYPE_IS_DECLARED 1
On 2013/08/29 16:41:06, Alok Priyadarshi wrote:
> FYI: For one of the in-progress refactoring, I will need to #define
> SH_MAX_TOKEN_LENGTH here as well.

Thanks for the heads up. I'll check whether I'll have to upload a new patch...

[Jamie Madill, 2013-11-20 18:29:02]

LGTM .. given that you've downloaded and re-applied and re-tested the patches,
it's been a while. Also please take to es3proto.

[nicolascapens, 2014-01-03 21:16:45]

This review can be abandoned, as it's no longer an issue. My TransGaming account
was removed so I can't close this myself.

https://code.google.com/p/angleproject/issues/detail?id=449