- Create an all static TypedDataView class which uses implicit field offset

- Create an all static TypedDataView class which uses implicit field offset
  values to get direct access to the fields of a typed data view object.

- Added a verification step after class finalization to ensure that the
  implicit field offsets in TypedDataView match the actual values in the
  dart instance.
Committed: https://code.google.com/p/dart/source/detail?r=20948

[siva, 2013-04-02 23:51:03]

Florian, I have added this all static TypedDataView class in the VM to get
direct access to the field offsets of typed data view instances. You may find it
useful in for intrinsification of the []= and [] methods of TypedDataView
objects.

[siva, 2013-04-04 01:05:49]

synched up to tot.

[Florian Schneider, 2013-04-04 09:33:14]

LGTM.

How is _ByteDataView different from _Uint8ArrayView? Could they maybe be merged?

https://codereview.chromium.org/13472019/diff/12001/runtime/vm/class_finalize...
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/13472019/diff/12001/runtime/vm/class_finalize...
runtime/vm/class_finalizer.cc:1969: // First verify field offsets of
'_ByteDataView' class.
Drop 'First' from comment.

[siva, 2013-04-05 02:01:56]

If we merged _ByteArrayView and _Uint8ArrayView then all the getInt16,
getUint16, getInt32.... and setInt16, setUint16,
setInt32 .... methods would be available to _Uint8ArrayView
which we do not want.

https://codereview.chromium.org/13472019/diff/12001/runtime/vm/class_finalize...
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/13472019/diff/12001/runtime/vm/class_finalize...
runtime/vm/class_finalizer.cc:1969: // First verify field offsets of
'_ByteDataView' class.
On 2013/04/04 09:33:14, Florian Schneider wrote:
> Drop 'First' from comment.

Done.

[Ivan Posva, 2013-04-05 04:23:55]

Please address this comment before committing.

-Ivan

https://codereview.chromium.org/13472019/diff/18001/runtime/vm/class_finalize...
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/13472019/diff/18001/runtime/vm/class_finalize...
runtime/vm/class_finalizer.cc:1994: ASSERT(field.Offset() ==
TypedDataView::data_offset());
You probably also want to check the name of the field, otherwise this test does
not make a lot of sense. The first field in the list should always have the
offset of 2 * wordSize.

[siva, 2013-04-05 05:07:52]

Added verification of field names.

[Ivan Posva, 2013-04-05 05:15:58]

LGTM -ip

[siva, 2013-04-05 05:19:43]

Committed patchset #7 manually as r20948 (presubmit successful).

[regis, 2013-04-09 20:27:39]

https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
runtime/vm/class_finalizer.cc:170: VerifyImplicitFieldOffsets();
I am getting a crash here in the case class finalization reported an error.
Moving this call just after line 162 solves the problem, but is that correct?
Is the system in an inconsistent state after an error is reported?
You can reproduce by generating the snapshot with the flag
--error_on_malformed_type at r21166, because there is an error in the core lib.

[siva, 2013-04-09 21:30:29]

https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
runtime/vm/class_finalizer.cc:170: VerifyImplicitFieldOffsets();
I guess in your case a type error occurred before the classes in dart:typeddata
got finalized, as a result of the error the field offsets are not set for the
typeddata classes and this assertion fails.

We could move the code to the successful case as you indicate, what about the
other pieces like RemoveOptimizedCode etc. should that be run only of there are
no errors?

Another option might be to do the verification only if the dart:typedata classes
are marked as being 'finalized'. 

On 2013/04/09 20:27:39, regis wrote:
> I am getting a crash here in the case class finalization reported an error.
> Moving this call just after line 162 solves the problem, but is that correct?
> Is the system in an inconsistent state after an error is reported?
> You can reproduce by generating the snapshot with the flag
> --error_on_malformed_type at r21166, because there is an error in the core
lib.

[regis, 2013-04-09 22:41:50]

On 2013/04/09 21:30:29, siva wrote:
>
https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
> File runtime/vm/class_finalizer.cc (right):
> 
>
https://codereview.chromium.org/13472019/diff/26006/runtime/vm/class_finalize...
> runtime/vm/class_finalizer.cc:170: VerifyImplicitFieldOffsets();
> I guess in your case a type error occurred before the classes in
dart:typeddata
> got finalized, as a result of the error the field offsets are not set for the
> typeddata classes and this assertion fails.
> 
Correct.

> We could move the code to the successful case as you indicate, what about the
> other pieces like RemoveOptimizedCode etc. should that be run only of there
are
> no errors?
> 
The only code being executed after finalization is RemoveOptimizedCode, the one
you mention. Since it only refers to classes that were loaded and finalized
before the last run of finalization, I think this is OK to run.

> Another option might be to do the verification only if the dart:typedata
classes
> are marked as being 'finalized'. 
> 
Either way is fine with me. 

> On 2013/04/09 20:27:39, regis wrote:
> > I am getting a crash here in the case class finalization reported an error.
> > Moving this call just after line 162 solves the problem, but is that
correct?
> > Is the system in an inconsistent state after an error is reported?
> > You can reproduce by generating the snapshot with the flag
> > --error_on_malformed_type at r21166, because there is an error in the core
> lib.