Fix some puzzling code in object.cc.

Fix some puzzling code in object.cc.

Jens was looking through object.cc and he spotted:

    if (Original()) ...

where it seems like Original returns a RawField* which is never 0, so
it is always true.  It looks like it was intended to be a call to
IsOriginal().

(See https://codereview.chromium.org/1722733002/diff/120001/runtime/vm/object.cc)

BUG=

Committed: https://github.com/dart-lang/sdk/commit/ec84b87ebfd47c999226b1fe60630110fcdec71f

[Kevin Millikin (Google), 2016-11-22 14:53:15]

kmillikin@google.com changed reviewers:
+ asiva@google.com

[Kevin Millikin (Google), 2016-11-22 14:53:16]

As written, the function will return the owner of a copied field instead of the
owner of the original.  This is only a bug if they can be different.  (I don't
know enough about background compilation to know if it can be the case or what
problems it could cause.)

[Florian Schneider, 2016-11-23 17:46:35]

On 2016/11/22 14:53:16, Kevin Millikin (Google) wrote:
> As written, the function will return the owner of a copied field instead of
the
> owner of the original.  This is only a bug if they can be different.  (I don't
> know enough about background compilation to know if it can be the case or what
> problems it could cause.)

The code is indeed puzzling. The owner_ of a cloned field is the original field,
the owner
of the an original field is the enclosing class, so yes, they can be different.

Your CL lgtm, but I'm not sure RawOwner is actually needed. The name is also
misleading, if it just returned owner_,
it should be called raw_owner(). Otherwise Field::Owner() should handle the case
of cloned fields.

[Kevin Millikin (Google), 2016-11-24 08:10:39]

On 2016/11/23 17:46:35, Florian Schneider wrote:
> On 2016/11/22 14:53:16, Kevin Millikin (Google) wrote:
> > As written, the function will return the owner of a copied field instead of
> the
> > owner of the original.  This is only a bug if they can be different.  (I
don't
> > know enough about background compilation to know if it can be the case or
what
> > problems it could cause.)
> 
> The code is indeed puzzling. The owner_ of a cloned field is the original
field,
> the owner
> of the an original field is the enclosing class, so yes, they can be
different.
> 
> Your CL lgtm, but I'm not sure RawOwner is actually needed. The name is also
> misleading, if it just returned owner_,
> it should be called raw_owner(). Otherwise Field::Owner() should handle the
case
> of cloned fields.

Thanks for the review, Florian.  I agree that the name RawOwner seems
misleading,
or at least ambiguous.

[Kevin Millikin (Google), 2016-11-24 08:13:13]

Description was changed from

==========
Fix some puzzling code in object.cc.

Jens was looking through object.cc and he spotted:

    if (Original()) ...

where it seems like Original returns a RawField* which is never 0, so
it is always true.  It looks like it was intended to be a call to
IsOriginal().

(See
https://codereview.chromium.org/1722733002/diff/120001/runtime/vm/object.cc)

BUG=
==========

to

==========
Fix some puzzling code in object.cc.

Jens was looking through object.cc and he spotted:

    if (Original()) ...

where it seems like Original returns a RawField* which is never 0, so
it is always true.  It looks like it was intended to be a call to
IsOriginal().

(See
https://codereview.chromium.org/1722733002/diff/120001/runtime/vm/object.cc)

BUG=

Committed:
https://github.com/dart-lang/sdk/commit/ec84b87ebfd47c999226b1fe60630110fcdec71f
==========

[Kevin Millikin (Google), 2016-11-24 08:13:15]

Committed patchset #2 (id:20001) manually as
ec84b87ebfd47c999226b1fe60630110fcdec71f (presubmit successful).