Issue 23447011: A typed selector needs no such method handling if at least one of its implemented subtype (in case …

Issue 23447011: A typed selector needs no such method handling if at least one of its implemented subtype (in case … (Closed)

Created:
7 years, 3 months ago by ngeoffray

Modified:
7 years, 3 months ago

Reviewers:
Johnni Winther, ahe

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

A typed selector needs no such method handling if at least one of its implemented subtype (in case it's not exact) does not implement the selector name. R=johnniwinther@google.com Committed: https://code.google.com/p/dart/source/detail?r=27100

Patch Set 1 : #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Total comments: 3

Created: 7 years, 3 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+49 lines, -3 lines)			Patch
	M	sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart	View	1 2 3	2 chunks	+22 lines, -3 lines	3 comments	Download
	A	tests/language/no_such_method_subtype_test.dart	View		1 chunk	+27 lines, -0 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

Johnni, Peter being aways, could you please take a look?

7 years, 3 months ago (2013-09-04 07:42:58 UTC) #2

Johnni Winther

lgtm https://codereview.chromium.org/23447011/diff/7001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart (right): https://codereview.chromium.org/23447011/diff/7001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart#newcode563 sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:563: : !hasConcreteMatch(cls, selector, compiler); Change to return !cls.isAbstract(compiler) ...

7 years, 3 months ago (2013-09-04 07:58:49 UTC) #3

ngeoffray

Thanks Johnni. https://codereview.chromium.org/23447011/diff/7001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart (right): https://codereview.chromium.org/23447011/diff/7001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart#newcode563 sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:563: : !hasConcreteMatch(cls, selector, compiler); On 2013/09/04 07:58:50, ...

7 years, 3 months ago (2013-09-04 08:06:05 UTC) #4

ngeoffray

Committed patchset #4 manually as r27100 (presubmit successful).

7 years, 3 months ago (2013-09-04 08:37:02 UTC) #5

ahe

I think I know the answer to my question, but I'd like to make sure ...

7 years, 3 months ago (2013-09-04 12:17:48 UTC) #6

ngeoffray

https://codereview.chromium.org/23447011/diff/12001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart (right): https://codereview.chromium.org/23447011/diff/12001/sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart#newcode561 sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:561: return !cls.isAbstract(compiler) On 2013/09/04 12:17:49, ahe wrote: > Why ...

7 years, 3 months ago (2013-09-04 12:32:23 UTC) #7

ahe

7 years, 3 months ago (2013-09-04 12:39:54 UTC) #8

Message was sent while issue was closed.

LGTM!

https://codereview.chromium.org/23447011/diff/12001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart
(right):

https://codereview.chromium.org/23447011/diff/12001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/types/flat_type_mask.dart:561: return
!cls.isAbstract(compiler)
On 2013/09/04 12:32:23, ngeoffray wrote:
> On 2013/09/04 12:17:49, ahe wrote:
> > Why the check for isAbstract?
> 
> If an abstract class does not have a concrete match, that is OK, we know we
> don't need noSuchMethod handling for such a class.

Thank you. Add a comment?

Expand Messages | Collapse Messages