Removed support for unused/deprecated extension - GL_3DL_array_object....

Removed support for unused/deprecated extension - GL_3DL_array_object.
TEST=conformance tests.

[kbr1, 2010-08-31 19:25:07]

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (left):

http://codereview.appspot.com/2043043/diff/1/4#oldcode1293
src/compiler/glslang.y:1293: TIntermNode* intermNode;
Is this else clause supposed to still exist?

http://codereview.appspot.com/2043043/diff/1/4#oldcode1329
src/compiler/glslang.y:1329: TIntermNode* intermNode;
Is this else clause supposed to still exist?

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (right):

http://codereview.appspot.com/2043043/diff/1/4#newcode629
src/compiler/glslang.y:629: $1.setArray(false);
Is this the right transformation? Or was this entire block of code supposed to
be deleted?

http://codereview.appspot.com/2043043/diff/1/4#newcode1493
src/compiler/glslang.y:1493: $1.setArray(false);
Is this code block supposed to exist or be deleted?

[Alok Priyadarshi, 2010-08-31 20:23:09]

It seems the conformance tests are horribly incomplete for arrays. I will write
a few tests to exercise the cases you pointed out.

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (left):

http://codereview.appspot.com/2043043/diff/1/4#oldcode1293
src/compiler/glslang.y:1293: TIntermNode* intermNode;
This else clause gets hit when GL_3DL_array_objects is available and enabled. So
I guess not.

http://codereview.appspot.com/2043043/diff/1/4#oldcode1329
src/compiler/glslang.y:1329: TIntermNode* intermNode;
This else clause gets hit when GL_3DL_array_objects is available and enabled. So
I guess not.

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (right):

http://codereview.appspot.com/2043043/diff/1/4#newcode629
src/compiler/glslang.y:629: $1.setArray(false);
Not sure, but it is functionally equivalent to what was here before. The
conformance tests do not hit this case. I will write a few tests to make sure.

http://codereview.appspot.com/2043043/diff/1/4#newcode1493
src/compiler/glslang.y:1493: $1.setArray(false);
Not sure, but it is functionally equivalent to what was here before. The
conformance tests do not hit this case. I will write a few tests to make sure.

[dgkoch, 2010-09-01 14:14:08]

Yes, you certainly shouldn't assume that the conformance tests have 100%
coverage of the languages features (or api features for that matter).  They
certainly don't by a long shot.

In general, it looks ok, but there are a couple of places where the error
reporting could be improved (noted by comments).  Other than the annotated
places in glslang.y,  the changes there appear to remove some error checking,
but we've verified they are handled correctly elsewhere. 

Daniel

http://codereview.appspot.com/2043043/diff/1/2
File src/compiler/Intermediate.cpp (right):

http://codereview.appspot.com/2043043/diff/1/2#newcode766
src/compiler/Intermediate.cpp:766: infoSink.info.message(EPrefixInternalError,
"Unexpected array type", getLine());
This error message seems a bit consfusing.  Perhaps something like "Invalid
operation for arrays" instead?

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (right):

http://codereview.appspot.com/2043043/diff/1/4#newcode629
src/compiler/glslang.y:629: $1.setArray(false);
I believe the extensionErrorCheck() calls would have added an error message
(though perhaps not very descriptive), while this patch just cases the parsing
to fail without any message.

http://codereview.appspot.com/2043043/diff/1/4#newcode1493
src/compiler/glslang.y:1493: $1.setArray(false);
I believe the extensionErrorCheck() calls would have added an error message
(though perhaps not very descriptive), while this patch just cases the parsing
to fail without any message.

[Alok Priyadarshi, 2010-09-01 20:12:22]

http://codereview.appspot.com/2043043/diff/1/4
File src/compiler/glslang.y (right):

http://codereview.appspot.com/2043043/diff/1/4#newcode629
src/compiler/glslang.y:629: $1.setArray(false);
On 2010/09/01 14:14:08, dgkoch wrote:
> I believe the extensionErrorCheck() calls would have added an error message
> (though perhaps not very descriptive), while this patch just cases the parsing
> to fail without any message.
> 

Done.

http://codereview.appspot.com/2043043/diff/1/4#newcode1493
src/compiler/glslang.y:1493: $1.setArray(false);
On 2010/09/01 14:14:08, dgkoch wrote:
> I believe the extensionErrorCheck() calls would have added an error message
> (though perhaps not very descriptive), while this patch just cases the parsing
> to fail without any message.
> 

Done.

http://codereview.appspot.com/2043043/diff/7001/8003
File src/compiler/glslang.y (left):

http://codereview.appspot.com/2043043/diff/7001/8003#oldcode1273
src/compiler/glslang.y:1273: }
It turns out that these two cases were not needed. You will get a syntax error
if array is initialized.

I am not sure if we should completely remove these cases or keep them and report
errors for future reference.

[kbr1, 2010-09-01 20:19:03]

Looks okay to me but deferring LGTM to Daniel.

Out of curiosity are you checking in your new tests somewhere?

[Alok Priyadarshi, 2010-09-01 20:31:12]

> 
> Out of curiosity are you checking in your new tests somewhere?

I am planning to submit them as gles2 or webgl conformance tests. Up until now I
have been submitting to webgl glsl conformance suite.

[dgkoch, 2010-09-01 20:46:45]

looks ok, other than improving that one error message.

Daniel

http://codereview.appspot.com/2043043/diff/7001/8001
File src/compiler/Intermediate.cpp (right):

http://codereview.appspot.com/2043043/diff/7001/8001#newcode766
src/compiler/Intermediate.cpp:766: infoSink.info.message(EPrefixInternalError,
"Unexpected array type", getLine());
This error message still seems a bit confusing.  Perhaps something like "Invalid
operation for arrays" instead?

[Alok Priyadarshi, 2010-09-01 20:57:21]

Thanks guys. submitting...

http://codereview.appspot.com/2043043/diff/7001/8001
File src/compiler/Intermediate.cpp (right):

http://codereview.appspot.com/2043043/diff/7001/8001#newcode766
src/compiler/Intermediate.cpp:766: infoSink.info.message(EPrefixInternalError,
"Unexpected array type", getLine());
On 2010/09/01 20:46:45, dgkoch wrote:
> This error message still seems a bit confusing.  Perhaps something like
"Invalid
> operation for arrays" instead?

sorry - done now.