Record ToBoolean's type information in Hydrogen's HBranch instruction, so we can use it in LCodeG...

Record ToBoolean's type information in Hydrogen's HBranch instruction, so we can use it in LCodeGen::DoBranch later.

[fschneider, 2011-07-25 11:55:18]

LGTM.

http://codereview.chromium.org/7461026/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/7461026/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:939: ToBooleanStub::Types expected_input_types =
ToBooleanStub::Types())
Maybe use a named constant ToBooleanStub::kNoTypes or the like to make clearer
what the default value is.

http://codereview.chromium.org/7461026/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/7461026/diff/1/src/hydrogen.cc#newcode2162
src/hydrogen.cc:2162: ToBooleanStub::Types
expected(builder->oracle()->ToBooleanTypes(test_id));
It would be nicer to just write

ToBooleanStub::Types expected = builder->oracle()->ToBooleanTypes(test_id);

http://codereview.chromium.org/7461026/diff/1/src/type-info.h
File src/type-info.h (right):

http://codereview.chromium.org/7461026/diff/1/src/type-info.h#newcode242
src/type-info.h:242: // cylces in our headers. Death to tons of implementations
in headers!! :-P
It would be a perfect opportunity to untangle one of the cycles!

If you add the comment, please add a todo with a corresponding issue to track.
Otherwise, it will never happen ;)

[Sven Panne, 2011-07-25 14:06:32]

http://codereview.chromium.org/7461026/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/7461026/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:939: ToBooleanStub::Types expected_input_types =
ToBooleanStub::Types())
On 2011/07/25 11:55:19, fschneider wrote:
> Maybe use a named constant ToBooleanStub::kNoTypes or the like to make clearer
> what the default value is.

Done, but because C++ is a bit broken regarding constants of a class type, I'll
use a simple function instead.

http://codereview.chromium.org/7461026/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/7461026/diff/1/src/hydrogen.cc#newcode2162
src/hydrogen.cc:2162: ToBooleanStub::Types
expected(builder->oracle()->ToBooleanTypes(test_id));
On 2011/07/25 11:55:19, fschneider wrote:
> It would be nicer to just write
> 
> ToBooleanStub::Types expected = builder->oracle()->ToBooleanTypes(test_id);

I think that using the combination of an empty constructor plus assignment
operator is actually worse than using the parameterized constructor alone,
because the former potentially wastes effort. In our concrete case, it probably
won't make a big difference, but in general the former is not a good idea IMHO.
(The user of a class shouldn't be required to know that the compiler hopefully
irons out any inefficiencies.)

http://codereview.chromium.org/7461026/diff/1/src/type-info.h
File src/type-info.h (right):

http://codereview.chromium.org/7461026/diff/1/src/type-info.h#newcode242
src/type-info.h:242: // cylces in our headers. Death to tons of implementations
in headers!! :-P
On 2011/07/25 11:55:19, fschneider wrote:
> It would be a perfect opportunity to untangle one of the cycles!
> 
> If you add the comment, please add a todo with a corresponding issue to track.
> Otherwise, it will never happen ;)

Untangling our huge header mess is a separate task which will require a few
days, probably even weeks. I've tried some seemingly innocuous cleanups several
times and almost always completely reverted after half a day or so. Firing up
doxygen on our sources, switching the browser to full-screen and admiring the
enormous header dependency graphs should convince anyone that this cleanup task
is not a minor one... ;-)

I'll add a TODO here.