Add class id constants fields to dart:_internal class 'ClassID'. Use the fields in the library (mor…

Add class id constants fields to dart:_internal class 'ClassID'. Use the fields in the library (more uses to come).

R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=37628

[siva, 2014-06-23 21:23:31]

LGTM with some comments.

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/growable_arr...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/growable_arr...
runtime/lib/growable_array.dart:19: if (ClassID.getID(index) != ClassID.cidSmi)
throw new ArgumentError(index);
The previous code was checking for int but now we check only for smi they don't
seem equivalent, are you assuming that index could only be Smi? Do we have
assertion for that elsewhere?

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:61: (ccid != ClassID.cidGrowableObjectArray)) {
cidImmutableObjectArray instead of repeating cidGrowableObjectArray twice ?

https://codereview.chromium.org/351673002/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:1388: field =
Field::New(String::Handle(Symbols::New("cid"#clazz)),                \
This String::Handle can also be hoisted out and this would become
str = Symbols::New("cid"#clazz);                                  \
field = Field::New(str, true, false, true, cls, 0);               \
....
....

[srdjan, 2014-06-23 22:27:36]

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/growable_arr...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/growable_arr...
runtime/lib/growable_array.dart:19: if (ClassID.getID(index) != ClassID.cidSmi)
throw new ArgumentError(index);
On 2014/06/23 21:23:31, siva wrote:
> The previous code was checking for int but now we check only for smi they
don't
> seem equivalent, are you assuming that index could only be Smi? Do we have
> assertion for that elsewhere?

Any index that is not Smi throws an exception, either because it is not an int
or because it is not a Smi (out of range). However, with this change I change
the behavior as instead of RangeError I would throw ArgumentError. Reverting it.

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:61: (ccid != ClassID.cidGrowableObjectArray)) {
On 2014/06/23 21:23:31, siva wrote:
> cidImmutableObjectArray instead of repeating cidGrowableObjectArray twice ?

Done.

https://codereview.chromium.org/351673002/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/351673002/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:1388: field =
Field::New(String::Handle(Symbols::New("cid"#clazz)),                \
On 2014/06/23 21:23:31, siva wrote:
> This String::Handle can also be hoisted out and this would become
> str = Symbols::New("cid"#clazz);                                  \
> field = Field::New(str, true, false, true, cls, 0);               \
> ....
> ....

Done.

[srdjan, 2014-06-23 22:42:05]

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

[Florian Schneider, 2014-06-24 09:39:00]

dbc:

https://codereview.chromium.org/351673002/diff/80001/runtime/vm/symbols.h
File runtime/vm/symbols.h (left):

https://codereview.chromium.org/351673002/diff/80001/runtime/vm/symbols.h#old...
runtime/vm/symbols.h:319: V(ClassId, "get:_classId")                            
                      \
Another example of a change that changes the snapshot format, but unsurprisingly
did not require regenerating the binary snapshot test that's supposed to
indicate changes in the snapshot format.