Issue 8360022: Fix member overriding rules in VM according to latest spec.

Issue 8360022: Fix member overriding rules in VM according to latest spec. (Closed)

Created:
9 years, 2 months ago by regis

Modified:
9 years, 2 months ago

Reviewers:
siva

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

Description

Fix member overriding rules in VM according to latest spec. Fix tests accordingly. Committed: https://code.google.com/p/dart/source/detail?r=607

Patch Set 2 : '' #

Created: 9 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+158 lines, -129 lines)			Patch
M	runtime/vm/class_finalizer.h	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	runtime/vm/class_finalizer.cc	View	1	4 chunks	+124 lines, -106 lines	0 comments	Download
M	runtime/vm/object.h	View	1	1 chunk	+3 lines, -1 line	0 comments	Download
M	runtime/vm/object.cc	View	1	1 chunk	+16 lines, -0 lines	0 comments	Download
M	tests/language/language.status	View	1	3 chunks	+4 lines, -7 lines	0 comments	Download
M	tests/language/src/FauxverrideTest.dart	View	1	2 chunks	+4 lines, -4 lines	0 comments	Download
M	tests/language/src/FieldOverrideTest.dart	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	tests/language/src/OverrideFieldTest.dart	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
M	tests/language/src/OverrideMethodWithFieldTest.dart	View	1	2 chunks	+2 lines, -5 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

siva

LGTM http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc File runtime/vm/class_finalizer.cc (right): http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#newcode557 runtime/vm/class_finalizer.cc:557: const String& name) { I would have preferred ...

9 years, 2 months ago (2011-10-21 17:13:32 UTC) #2

regis

9 years, 2 months ago (2011-10-21 17:33:38 UTC) #3

Thanks!

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:557: const String& name) {
On 2011/10/21 17:13:32, asiva wrote:
> I would have preferred the name FindSuperOwnerOfInstanceMember,
> but that is just my preference. Please feel free to ignore this comment if the
> name does not sound right.

Done.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:578: static RawClass* FunctionSuperOwner(const
Class& cls, const String& name) {
On 2011/10/21 17:13:32, asiva wrote:
> Similarly I would have preferred FindSuperOwnerOfFunction but feel free to
> ignore this comment.

Done.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:655: // TODO(regis): This arity check is still
being debated. Revisit.
On 2011/10/21 17:13:32, asiva wrote:
> Yes this is still being debated, may have to yank out.

OK. Keeping the TODO.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:707: name.ToCString(),
On 2011/10/21 17:13:32, asiva wrote:
> I think this should be function_name.ToCString()

Good catch.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:709: name.ToCString(),
On 2011/10/21 17:13:32, asiva wrote:
> I think this should be function_name.ToCString()

Done.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:719: name.ToCString(),
On 2011/10/21 17:13:32, asiva wrote:
> I think this should be function_name.ToCString()

Done.

http://codereview.chromium.org/8360022/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:721: name.ToCString(),
On 2011/10/21 17:13:32, asiva wrote:
> I think this should be function_name.ToCString()

Done.

Expand Messages | Collapse Messages