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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 14044007: Add synchronous mirror API. Resurrects invoke, apply, newInstance, getField and setField as synchro… (Closed)

Created:
7 years, 8 months ago by gbracha
Modified:
7 years, 2 months ago
Reviewers:
ahe, srdjan, Ivan Posva
CC:
reviews_dartlang.org, ahe
Visibility:
Public.

Description

Add synchronous mirror API. Resurrects invoke, apply, newInstance, getField and setField as synchronous calls that accept isolate-local object as arguments. Re-org'ed per Ivan's suggestions. Now broken, but uploading so Ivan can land it next week.

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Total comments: 6

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -25 lines) Patch
M runtime/lib/mirrors.cc View 1 2 3 5 chunks +44 lines, -7 lines 0 comments Download
M runtime/lib/mirrors_impl.dart View 1 2 3 9 chunks +60 lines, -12 lines 0 comments Download
M runtime/vm/bootstrap_natives.h View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M sdk/lib/mirrors/mirrors.dart View 1 3 chunks +48 lines, -1 line 0 comments Download
M tests/lib/mirrors/mirrors_test.dart View 5 chunks +24 lines, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
gbracha
Please ignore the initial patch, which had some extra junk in it.
7 years, 8 months ago (2013-04-11 21:21:32 UTC) #1
ahe
I don't see how this change can have any impact on dart2js, so don't worry ...
7 years, 8 months ago (2013-04-11 21:29:08 UTC) #2
srdjan
DBC https://codereview.chromium.org/14044007/diff/4001/runtime/lib/mirrors.cc File runtime/lib/mirrors.cc (right): https://codereview.chromium.org/14044007/diff/4001/runtime/lib/mirrors.cc#newcode202 runtime/lib/mirrors.cc:202: GrowableArray<Dart_Handle>* arg_array) { Strange formatting https://codereview.chromium.org/14044007/diff/4001/runtime/lib/mirrors.cc#newcode216 runtime/lib/mirrors.cc:216: } ...
7 years, 8 months ago (2013-04-11 21:29:43 UTC) #3
gbracha
addressed Srdjan's comments.
7 years, 8 months ago (2013-04-11 21:36:31 UTC) #4
Ivan Posva
https://codereview.chromium.org/14044007/diff/7003/runtime/lib/mirrors.cc File runtime/lib/mirrors.cc (right): https://codereview.chromium.org/14044007/diff/7003/runtime/lib/mirrors.cc#newcode1214 runtime/lib/mirrors.cc:1214: Two lines. https://codereview.chromium.org/14044007/diff/7003/runtime/lib/mirrors.cc#newcode1220 runtime/lib/mirrors.cc:1220: Want to add a comment ...
7 years, 8 months ago (2013-04-12 21:15:26 UTC) #5
Ivan Posva
7 years, 8 months ago (2013-04-16 23:17:08 UTC) #6

Powered by Google App Engine
This is Rietveld 408576698