- Remove deprecated StackZone constructor with Isolate parameter.

- Remove deprecated StackZone constructor with Isolate parameter.

R=srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/0a0b101fbd6acff77dd0774a68b4cefd4f6c96e3

[Ivan Posva, 2015-09-02 21:51:16]

iposva@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2015-09-02 22:24:10]

LGTM with questions

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc#newco...
runtime/vm/service.cc:40: #define Z (T->zone())
Instead I would add Zone* Z = T->zone() in method were it is use, or maybe
delete it?

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc#newco...
runtime/vm/service.cc:541: Instance& reply_port = Instance::Handle(Z);
Why not zone instead of Z?

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service_isolate.cc
File runtime/vm/service_isolate.cc (right):

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service_isolate....
runtime/vm/service_isolate.cc:551: const String& url_str = String::Handle(Z,
Symbols::DartVMService().raw());
ditto

[Ivan Posva, 2015-09-02 22:59:26]

Thanks,
-Ivan

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc#newco...
runtime/vm/service.cc:40: #define Z (T->zone())
On 2015/09/02 22:24:09, srdjan wrote:
> Instead I would add Zone* Z = T->zone() in method were it is use, or maybe
> delete it?

See explanation below why you cannot cache the current zone in a Z.

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service.cc#newco...
runtime/vm/service.cc:541: Instance& reply_port = Instance::Handle(Z);
On 2015/09/02 22:24:09, srdjan wrote:
> Why not zone instead of Z?

Because zone is a StackZone defined above on line 538 and not a Zone. I could
also type T->zone() everywhere, but Siva and I have started using the T, I and Z
conventions for these. Caching the current zone in a Z variable is dangerous as
the zone can change as you progress with the code, but T->zone() will always be
correct. Hence the Z macro at the beginning of the file.

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service_isolate.cc
File runtime/vm/service_isolate.cc (right):

https://codereview.chromium.org/1312813006/diff/1/runtime/vm/service_isolate....
runtime/vm/service_isolate.cc:551: const String& url_str = String::Handle(Z,
Symbols::DartVMService().raw());
On 2015/09/02 22:24:09, srdjan wrote:
> ditto

ditto

[Ivan Posva, 2015-09-02 23:07:54]

Committed patchset #1 (id:1) manually as
0a0b101fbd6acff77dd0774a68b4cefd4f6c96e3 (presubmit successful).