Issue 11413219: Canonicalize raw type

Issue 11413219: Canonicalize raw type (Closed)

Created:
8 years ago by Johnni Winther

Modified:
8 years ago

Reviewers:
karlklose, ahe

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Canonicalize raw type Committed: https://code.google.com/p/dart/source/detail?r=15448

Patch Set 2 : Revert isSubtype for malformed types to false #

Total comments: 3

Patch Set 4 : Updated cf. comment #

Total comments: 6

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+149 lines, -57 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/dart_backend/backend.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/dart_backend/placeholder_collector.dart	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/elements/elements.dart	View		6 chunks	+69 lines, -10 lines	2 comments	Download
M	sdk/lib/_internal/compiler/implementation/enqueue.dart	View		2 chunks	+8 lines, -4 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart	View		2 chunks	+3 lines, -4 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart	View		1 chunk	+7 lines, -5 lines	1 comment	Download
M	sdk/lib/_internal/compiler/implementation/native_handler.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/resolution/members.dart	View		4 chunks	+13 lines, -6 lines	1 comment	Download
M	sdk/lib/_internal/compiler/implementation/ssa/builder.dart	View	1	5 chunks	+13 lines, -6 lines	2 comments	Download
M	sdk/lib/_internal/compiler/implementation/ssa/codegen.dart	View		1 chunk	+10 lines, -6 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/typechecker.dart	View	1 2 3	9 chunks	+20 lines, -10 lines	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/world.dart	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Johnni Winther

Second try. The change from the previous CL is in the delta between patch set ...

8 years ago (2012-11-28 12:24:35 UTC) #1

karlklose

LGTM. https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/typechecker.dart File sdk/lib/_internal/compiler/implementation/typechecker.dart (right): https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/typechecker.dart#newcode517 sdk/lib/_internal/compiler/implementation/typechecker.dart:517: return false; Please add a TODO here.

8 years ago (2012-11-28 12:28:50 UTC) #2

Johnni Winther

https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/typechecker.dart File sdk/lib/_internal/compiler/implementation/typechecker.dart (right): https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/typechecker.dart#newcode517 sdk/lib/_internal/compiler/implementation/typechecker.dart:517: return false; On 2012/11/28 12:28:50, karlklose wrote: > Please ...

8 years ago (2012-11-28 12:36:32 UTC) #3

ahe

Comments so far. Don't wait for me. https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/elements/elements.dart File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right): https://codereview.chromium.org/11413219/diff/15/sdk/lib/_internal/compiler/implementation/elements/elements.dart#newcode843 sdk/lib/_internal/compiler/implementation/elements/elements.dart:843: * Canonicalize ...

8 years ago (2012-11-28 12:47:38 UTC) #4

ahe

8 years ago (2012-11-29 10:09:08 UTC) #5

Message was sent while issue was closed.

Nice clean up!

LGTM, but I think you need to revisit the use of isRaw from ssa/builder.dart.

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:1436: * This
type is computed together with [thisType] in [computeType].
Awesome comments.

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:1478: });
Code duplication detected. This loop can be shared with typedef element.

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart
(right):

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart:1173:
_typeArguments = <TypeMirror>[];
This list is not immutable.  Also, you can share a const empty array for the
common case: raw or non-generic.

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:1147: type =
cls.rawType;
cls.declaration.rawType?

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3165: if
(!type.isRaw) {
if (!type.canElideArguments) { ...

https://codereview.chromium.org/11413219/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3239: if
(!type.isRaw) {
ditto

Expand Messages | Collapse Messages