Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(853)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 360493002: Emit declarations for typedefs that are needed by reflection. (Closed)

Created:
6 years, 5 months ago by karlklose
Modified:
6 years, 4 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Emit declarations for typedefs that are needed by reflection. Typedefs are encoded like closure classes, but without a call-method and a superclass. For example, the typedef `typedef int Foo(String s);` is emitted as: Foo: {'^': ':15'}, where `15` is the index into `init.metadata` at which the function type is emitted. BUG= http://dartbug.com/16939 R=johnniwinther@google.com, floitsch@google.com, herhut@google.com Committed: https://code.google.com/p/dart/source/detail?r=37814 Reverted in https://code.google.com/p/dart/source/detail?r=37848 Committed: https://code.google.com/p/dart/source/detail?r=37939 Reverted in https://code.google.com/p/dart/source/detail?r=37940 Committed: https://code.google.com/p/dart/source/detail?r=37945 Committed: https://code.google.com/p/dart/source/detail?r=38891

Patch Set 1 #

Patch Set 2 : Add a test. #

Patch Set 3 : Minor cleanups. #

Total comments: 3

Patch Set 4 : Move shared file and address comment. #

Patch Set 5 : Rebase #

Patch Set 6 : Support unmangled name in minified code. #

Patch Set 7 : Adapt to new way of computing elements needed for reflection. #

Total comments: 2

Patch Set 8 : Address comment. #

Patch Set 9 : Rebased. #

Patch Set 10 : Also emit the precompiled constructor for typedefs; it is needed by CSP mode. #

Total comments: 2

Patch Set 11 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -106 lines) Patch
M sdk/lib/_internal/compiler/implementation/enqueue.dart View 1 2 3 4 5 6 7 8 1 chunk +6 lines, -4 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_backend/backend.dart View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -5 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_backend/native_emitter.dart View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_emitter/class_emitter.dart View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -26 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart View 1 2 3 4 5 6 7 8 9 10 13 chunks +83 lines, -23 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_emitter/js_emitter.dart View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_emitter/metadata_emitter.dart View 1 chunk +2 lines, -14 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/resolution/members.dart View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
A sdk/lib/_internal/compiler/implementation/runtime_data.dart View 1 2 3 4 5 6 7 8 9 10 1 chunk +22 lines, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/world.dart View 1 chunk +2 lines, -0 lines 0 comments Download
M sdk/lib/_internal/lib/js_mirrors.dart View 1 2 3 4 5 6 7 8 4 chunks +26 lines, -25 lines 0 comments Download
M tests/compiler/dart2js/analyze_unused_dart2js_test.dart View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M tests/lib/lib.status View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M tests/lib/mirrors/relation_subclass_test.dart View 1 chunk +6 lines, -7 lines 0 comments Download
A tests/lib/mirrors/typedef_declaration_test.dart View 1 2 3 1 chunk +29 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
karlklose
6 years, 5 months ago (2014-06-27 12:09:17 UTC) #1
floitsch
LGTM. https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart File tests/lib/mirrors/typedef_declaration_test.dart (right): https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart#newcode27 tests/lib/mirrors/typedef_declaration_test.dart:27: Expect.isTrue(barMirror == null, /// 01: ok 01: ok ...
6 years, 5 months ago (2014-06-27 12:39:32 UTC) #2
karlklose
Thanks for the review! https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart File tests/lib/mirrors/typedef_declaration_test.dart (right): https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart#newcode27 tests/lib/mirrors/typedef_declaration_test.dart:27: Expect.isTrue(barMirror == null, /// 01: ...
6 years, 5 months ago (2014-06-30 09:02:09 UTC) #3
floitsch
https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart File tests/lib/mirrors/typedef_declaration_test.dart (right): https://codereview.chromium.org/360493002/diff/2/tests/lib/mirrors/typedef_declaration_test.dart#newcode27 tests/lib/mirrors/typedef_declaration_test.dart:27: Expect.isTrue(barMirror == null, /// 01: ok On 2014/06/30 09:02:09, ...
6 years, 5 months ago (2014-06-30 09:29:12 UTC) #4
karlklose
Committed patchset #4 manually as r37814 (presubmit successful).
6 years, 5 months ago (2014-06-30 09:50:05 UTC) #5
karlklose
I had to revert and fix minified mode. PTAL at patchset 6.
6 years, 5 months ago (2014-07-02 07:57:48 UTC) #6
floitsch
Still LGTM.
6 years, 5 months ago (2014-07-02 09:54:56 UTC) #7
karlklose
Committed patchset #6 manually as r37939 (presubmit successful).
6 years, 5 months ago (2014-07-02 10:51:20 UTC) #8
karlklose
Could you take a look at patchset 7?
6 years, 5 months ago (2014-07-02 12:13:01 UTC) #9
herhut
LGTM (patchset 7) https://codereview.chromium.org/360493002/diff/130001/sdk/lib/_internal/compiler/implementation/enqueue.dart File sdk/lib/_internal/compiler/implementation/enqueue.dart (right): https://codereview.chromium.org/360493002/diff/130001/sdk/lib/_internal/compiler/implementation/enqueue.dart#newcode326 sdk/lib/_internal/compiler/implementation/enqueue.dart:326: compiler.resolver.resolve(element); This code runs both during ...
6 years, 5 months ago (2014-07-02 12:26:12 UTC) #10
karlklose
https://codereview.chromium.org/360493002/diff/130001/sdk/lib/_internal/compiler/implementation/enqueue.dart File sdk/lib/_internal/compiler/implementation/enqueue.dart (right): https://codereview.chromium.org/360493002/diff/130001/sdk/lib/_internal/compiler/implementation/enqueue.dart#newcode326 sdk/lib/_internal/compiler/implementation/enqueue.dart:326: compiler.resolver.resolve(element); On 2014/07/02 12:26:12, herhut wrote: > This code ...
6 years, 5 months ago (2014-07-02 12:30:32 UTC) #11
karlklose
Committed patchset #8 manually as r37945 (presubmit successful).
6 years, 5 months ago (2014-07-02 12:43:46 UTC) #12
karlklose
Johnni, could you take a look at patch set 9?
6 years, 4 months ago (2014-08-04 13:08:12 UTC) #13
karlklose
I meant patch set 10.
6 years, 4 months ago (2014-08-04 13:47:03 UTC) #14
Johnni Winther
lgtm https://codereview.chromium.org/360493002/diff/190001/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart File sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart (right): https://codereview.chromium.org/360493002/diff/190001/sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart#newcode1499 sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart:1499: print('generating output for typedef $typedef'); Remove debug code. ...
6 years, 4 months ago (2014-08-04 13:49:52 UTC) #15
karlklose
6 years, 4 months ago (2014-08-05 07:18:49 UTC) #16
Message was sent while issue was closed. 
Committed patchset #11 manually as 38891 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698