Introduced TypeFeedbackId and BailoutId types.

Introduced TypeFeedbackId and BailoutId types.

This is a refactoring-only CL which improves the typing of IDs associated with
AST nodes. The interesting parts are in utils.h and ast.h, the rest of the CL
basically follows mechanically.

Committed: https://code.google.com/p/v8/source/detail?r=12263

[Michael Starzinger, 2012-08-06 13:03:41]

LGTM (with a few nits). I like this change!

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1205
src/ast.h:1205: TypeFeedbackId LiteralFeedbackId() const { return reuse(id()); }
Can we move that down right after Match()?

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1573
src/ast.h:1573: void RecordTypeFeedback(TypeFeedbackOracle* oracle,
Should fit onto one line.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1579
src/ast.h:1579: TypeFeedbackId CallFeedbackId() const { return reuse(id()); }
Can we move that up to be the first method in this block and prefix this block
with the "// Type feedback information." comments?

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1640
src/ast.h:1640: TypeFeedbackId CallNewFeedbackId() const { return reuse(id()); }
Likewise.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1758
src/ast.h:1758: TypeFeedbackId BinaryOperationFeedbackId() const { return
reuse(id()); }
We should have a consistent ordering, first bailout accessors and then
type-feedback accessors.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1859
src/ast.h:1859: TypeFeedbackId CompareOperationFeedbackId() const { return
reuse(id()); }
Likewise.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/ast.h#newcode1967
src/ast.h:1967: TypeFeedbackId AssignmentFeedbackId() { return reuse(id()); }
Likewise.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/deoptimizer.cc#new...
src/deoptimizer.cc:1582: // TODO(svenpanne) Shouldn't we use
Safepoint::kNoDeoptimizationIndex?
Let's make that a Safepoint::kNoDeoptimizationIndex right away.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/hydrogen.cc#newcod...
src/hydrogen.cc:4009: BailoutId join_id = !default_id.IsNone()
Should fit into one line now.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/parser.cc
File src/parser.cc (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/parser.cc#newcode500
src/parser.cc:500: isolate->set_ast_node_id(BailoutId::Declarations().ToInt() +
1);
Let's turn that into BailoutId::FirstUsable() or something like that.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/type-info.cc
File src/type-info.cc (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/type-info.cc#newco...
src/type-info.cc:74: static uint32_t IdToKey(TypeFeedbackId id) {
Let's use "ast_id" as a name for this local variable throughout the file, for
consistency.

https://chromiumcodereview.appspot.com/10831172/diff/1/src/type-info.h
File src/type-info.h (right):

https://chromiumcodereview.appspot.com/10831172/diff/1/src/type-info.h#newcod...
src/type-info.h:245: bool StoreIsMonomorphicNormal(TypeFeedbackId id);
Let's rename "id" to "ast_id" for consistency.

[Sven Panne, 2012-08-06 14:08:40]

Feedback addressed, landing...

http://codereview.chromium.org/10831172/diff/1/src/ast.h
File src/ast.h (right):

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1205
src/ast.h:1205: TypeFeedbackId LiteralFeedbackId() const { return reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Can we move that down right after Match()?

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1573
src/ast.h:1573: void RecordTypeFeedback(TypeFeedbackOracle* oracle,
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Should fit onto one line.

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1579
src/ast.h:1579: TypeFeedbackId CallFeedbackId() const { return reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Can we move that up to be the first method in this block and prefix this block
> with the "// Type feedback information." comments?

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1640
src/ast.h:1640: TypeFeedbackId CallNewFeedbackId() const { return reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Likewise.

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1758
src/ast.h:1758: TypeFeedbackId BinaryOperationFeedbackId() const { return
reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> We should have a consistent ordering, first bailout accessors and then
> type-feedback accessors.

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1859
src/ast.h:1859: TypeFeedbackId CompareOperationFeedbackId() const { return
reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Likewise.

Done.

http://codereview.chromium.org/10831172/diff/1/src/ast.h#newcode1967
src/ast.h:1967: TypeFeedbackId AssignmentFeedbackId() { return reuse(id()); }
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Likewise.

Done.

http://codereview.chromium.org/10831172/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

http://codereview.chromium.org/10831172/diff/1/src/deoptimizer.cc#newcode1582
src/deoptimizer.cc:1582: // TODO(svenpanne) Shouldn't we use
Safepoint::kNoDeoptimizationIndex?
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Let's make that a Safepoint::kNoDeoptimizationIndex right away.

Done.

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

http://codereview.chromium.org/10831172/diff/1/src/hydrogen.cc#newcode4009
src/hydrogen.cc:4009: BailoutId join_id = !default_id.IsNone()
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Should fit into one line now.

Done.

http://codereview.chromium.org/10831172/diff/1/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/10831172/diff/1/src/parser.cc#newcode500
src/parser.cc:500: isolate->set_ast_node_id(BailoutId::Declarations().ToInt() +
1);
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Let's turn that into BailoutId::FirstUsable() or something like that.

Done.

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

http://codereview.chromium.org/10831172/diff/1/src/type-info.cc#newcode74
src/type-info.cc:74: static uint32_t IdToKey(TypeFeedbackId id) {
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Let's use "ast_id" as a name for this local variable throughout the file, for
> consistency.

Done.

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

http://codereview.chromium.org/10831172/diff/1/src/type-info.h#newcode245
src/type-info.h:245: bool StoreIsMonomorphicNormal(TypeFeedbackId id);
On 2012/08/06 13:03:41, Michael Starzinger wrote:
> Let's rename "id" to "ast_id" for consistency.

Done.