Issue 2884233002: Use entities in runtime_types

Issue 2884233002: Use entities in runtime_types (Closed)

Created:
3 years, 7 months ago by Johnni Winther

Modified:
3 years, 7 months ago

Reviewers:
Siggi Cherem (dart-lang), sra1

CC:
reviews_dartlang.org

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

Use entities in runtime_types - and make it its own library. R=sigmund@google.com Committed: https://github.com/dart-lang/sdk/commit/b30ff8ed663987eba916be4a2c5e831319206e45

Patch Set 2 : Remove debug print #

Total comments: 8

Patch Set 3 : Updated cf. comments #

Created: 3 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+357 lines, -262 lines)			Patch
M	pkg/compiler/lib/src/elements/resolution_types.dart	View		1 chunk	+10 lines, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/elements/types.dart	View		1 chunk	+7 lines, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/enqueue.dart	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	pkg/compiler/lib/src/frontend_strategy.dart	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/js_backend/backend.dart	View		6 chunks	+8 lines, -9 lines	0 comments	Download
M	pkg/compiler/lib/src/js_backend/codegen_listener.dart	View		2 chunks	+5 lines, -4 lines	0 comments	Download
M	pkg/compiler/lib/src/js_backend/constant_emitter.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	pkg/compiler/lib/src/js_backend/enqueuer.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	pkg/compiler/lib/src/js_backend/impact_transformer.dart	View		2 chunks	+1 line, -1 line	0 comments	Download
M	pkg/compiler/lib/src/js_backend/namer.dart	View		5 chunks	+10 lines, -7 lines	0 comments	Download
M	pkg/compiler/lib/src/js_backend/resolution_listener.dart	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/js_backend/runtime_types.dart	View	1 2	42 chunks	+224 lines, -191 lines	0 comments	Download
M	pkg/compiler/lib/src/js_emitter/code_emitter_task.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	pkg/compiler/lib/src/js_emitter/metadata_collector.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart	View		2 chunks	+7 lines, -7 lines	0 comments	Download
M	pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart	View		3 chunks	+5 lines, -5 lines	0 comments	Download
M	pkg/compiler/lib/src/js_emitter/type_test_registry.dart	View		8 chunks	+14 lines, -9 lines	0 comments	Download
M	pkg/compiler/lib/src/kernel/kernel_strategy.dart	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/kernel/types.dart	View		1 chunk	+11 lines, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/resolution/resolution_strategy.dart	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	pkg/compiler/lib/src/ssa/builder.dart	View		2 chunks	+4 lines, -5 lines	0 comments	Download
M	pkg/compiler/lib/src/ssa/builder_kernel.dart	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	pkg/compiler/lib/src/ssa/codegen.dart	View		1 chunk	+4 lines, -1 line	0 comments	Download
M	pkg/compiler/lib/src/ssa/graph_builder.dart	View		2 chunks	+5 lines, -2 lines	0 comments	Download
M	pkg/compiler/lib/src/ssa/optimize.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	pkg/compiler/lib/src/universe/codegen_world_builder.dart	View		5 chunks	+22 lines, -9 lines	0 comments	Download
M	tests/compiler/dart2js/type_representation_test.dart	View		1 chunk	+3 lines, -2 lines	0 comments	Download

Messages

Total messages: 8 (3 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Siggi Cherem (dart-lang)

lgtm https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js_backend/enqueuer.dart File pkg/compiler/lib/src/js_backend/enqueuer.dart (right): https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js_backend/enqueuer.dart#newcode222 pkg/compiler/lib/src/js_backend/enqueuer.dart:222: _worldBuilder.registerClosurizedMember(element); should "closurized-member" be encoded as part of ...

3 years, 7 months ago (2017-05-17 16:14:11 UTC) #3

Johnni Winther

Committed patchset #3 (id:40001) manually as b30ff8ed663987eba916be4a2c5e831319206e45 (presubmit successful).

3 years, 7 months ago (2017-05-18 08:09:54 UTC) #5

Johnni Winther

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js_backend/enqueuer.dart File pkg/compiler/lib/src/js_backend/enqueuer.dart (right): https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js_backend/enqueuer.dart#newcode222 pkg/compiler/lib/src/js_backend/enqueuer.dart:222: _worldBuilder.registerClosurizedMember(element); On 2017/05/17 16:14:11, Siggi Cherem (dart-lang) wrote: > ...

3 years, 7 months ago (2017-05-18 08:10:56 UTC) #6

Message was sent while issue was closed.

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/enqueuer.dart (right):

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/enqueuer.dart:222:
_worldBuilder.registerClosurizedMember(element);
On 2017/05/17 16:14:11, Siggi Cherem (dart-lang) wrote:
> should "closurized-member" be encoded as part of the impact?

Cannot be done - it's is non-local property:

class C { 
  foo() {}
}
bar() => new C();
baz(o) => o.foo;
main() {
  var c = bar();
  return baz(c);
}

We need the combination of `new C()` and `o.foo` to see that `C.foo` is
closurized.

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/runtime_types.dart (right):

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/runtime_types.dart:822: type
/*.dynamifyMethodTypeVariableType*/,
On 2017/05/17 16:14:11, Siggi Cherem (dart-lang) wrote:
> rewrite as TODO to make it easier to grep for?

Should just have been removed. The dynamification is done in teh
TypeRepresentationGenerator line 991.

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/runtime_types.dart:913:
substitution.parameters.map((type) => declaration(type));
On 2017/05/17 16:14:11, Siggi Cherem (dart-lang) wrote:
> undo?

Can't do (stupid type system!): [declaration] has type
`(TypeVariableType)->jsAst.Expression` and when the `substitution.parameter.map`
object is a `List<ResolutionDartType>` it requires a
`(ResolutionDartType)->?`...

Added a comment/TODO.

https://codereview.chromium.org/2884233002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/runtime_types.dart:991: /// A [TypeVariableType]
from a generic method is replaced by a
On 2017/05/17 16:14:11, Siggi Cherem (dart-lang) wrote:
> this appears to be new - bug fix? should we add a regression test?

This is instead of using [dynamifyMethodTypeVariableType] in line 822.

sra1

3 years, 7 months ago (2017-05-18 17:43:19 UTC) #8

Message was sent while issue was closed.

I notice that this removes three useless classes is swarm.
(Future, Function, _EventSink - they appear to be untested but implemented
interfaces).

Was that intended?

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages