Refactor location tracking.

Refactor location tracking.

R=kbr@chromium.org

Committed: https://code.google.com/p/angleproject/source/detail?r=2202

[Alok Priyadarshi, 2013-05-08 22:48:31]

Sorry about the big patch. Do not worry about the generated files. I will
re-generate them on appropriate platforms before committing.

[kbr1, 2013-05-08 23:49:52]

That's OK. LGTM with a few questions.

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

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:526: $$ = context->intermediate.addUnaryMath(op,
$1.intermNode, @1, context->symbolTable);
Noticed a difference here. Is this correct?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1849: $3->setLine(@$);
Just checking, this is the correct transformation?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1873: $2->setLine(@$);
Same here?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1881: $$ = context->intermediate.makeAggregate($1, @$);
And here?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1884: $$ = context->intermediate.growAggregate($1, $2,
@$);
And here?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:2033: $$ = context->intermediate.growAggregate($1, $2,
@$);
Correct transformation?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:2142: $$ =
context->intermediate.growAggregate($1.intermAggregate, $3, @$);
Here as well?

https://codereview.appspot.com/9078046/diff/7001/src/compiler/intermediate.h
File src/compiler/intermediate.h (right):

https://codereview.appspot.com/9078046/diff/7001/src/compiler/intermediate.h#...
src/compiler/intermediate.h:209: TIntermNode() {}
Is it worth either adding an initializer here or a constructor to TSourceLoc
zeroing out its fields?

[Alok Priyadarshi, 2013-05-09 04:03:08]

Thanks for the thorough review.

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

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:526: $$ = context->intermediate.addUnaryMath(op,
$1.intermNode, @1, context->symbolTable);
On 2013/05/08 23:49:52, kbr1 wrote:
> Noticed a difference here. Is this correct?

This is correct. We no longer handle a (line == 0) case inside TIntermediate
class, which means we always need to pass a valid location. @1 is the valid
location for this node.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1849: $3->setLine(@$);
On 2013/05/08 23:49:52, kbr1 wrote:
> Just checking, this is the correct transformation?

This is correct. Instead of setting separate start and end line, we are setting
the whole range. @$ represents the range of this reduction.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1873: $2->setLine(@$);
On 2013/05/08 23:49:52, kbr1 wrote:
> Same here?

Same here.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1881: $$ = context->intermediate.makeAggregate($1, @$);
On 2013/05/08 23:49:52, kbr1 wrote:
> And here?

Yes.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:1884: $$ = context->intermediate.growAggregate($1, $2,
@$);
On 2013/05/08 23:49:52, kbr1 wrote:
> And here?

Yes.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:2033: $$ = context->intermediate.growAggregate($1, $2,
@$);
On 2013/05/08 23:49:52, kbr1 wrote:
> Correct transformation?

Yes.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/glslang.y#newco...
src/compiler/glslang.y:2142: $$ =
context->intermediate.growAggregate($1.intermAggregate, $3, @$);
On 2013/05/08 23:49:52, kbr1 wrote:
> Here as well?

Yes.

https://codereview.appspot.com/9078046/diff/7001/src/compiler/intermediate.h
File src/compiler/intermediate.h (right):

https://codereview.appspot.com/9078046/diff/7001/src/compiler/intermediate.h#...
src/compiler/intermediate.h:209: TIntermNode() {}
On 2013/05/08 23:49:52, kbr1 wrote:
> Is it worth either adding an initializer here or a constructor to TSourceLoc
> zeroing out its fields?

I will add an initializer here. TSourceLoc is a member of TPublicType, which in
turn is part of a union. This means TSourceLoc cannot have a constructor. I plan
to get rid of TPublicType and then add a constructor.

[Alok Priyadarshi, 2013-05-09 17:54:47]

Committed patchset #5 manually as r2202 (presubmit successful).