Fix access to type variables in initializer expressions

Fix access to type variables in initializer expressions

Allow initializer expressions to capture type arguments.
This used to crash because the receiver is not accessible
in initializer expressions. We don't need the receiver, we
just need to mark it as captured.

Fixes issue 8847.

R=ahe@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25877

[hausner, 2013-08-06 13:27:56]

Thank you.

[ahe, 2013-08-06 15:20:38]

Hi Matthias,

I think you need to explain this to me in person :-)

Cheers,
Peter

[hausner, 2013-08-07 11:18:27]

Here's an updated version. The first revision had the side effect of marking
some variables as captured when they should not have been.

[hausner, 2013-08-07 14:26:53]

Regis, would you mind taking a look at this change to scopes and capturing the
instantiator? Thank you.

[ahe, 2013-08-07 14:49:27]

LGTM!

[hausner, 2013-08-07 15:15:07]

Committed patchset #3 manually as r25877 (presubmit successful).

[regis, 2013-08-07 18:28:26]

LGTM with 2 suggestions.

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc
File runtime/vm/scopes.cc (right):

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc#newc...
runtime/vm/scopes.cc:324: if ((var != NULL) && !var->is_invisible_) {
As far as I understand, if a variable is marked invisible, you are not going to
find it in the parent scope. I guess that most of the time, there is no parent
scope anyway, but still, it would be a bit more efficient to return immediately:
if (var != NULL) {
  if (var->is_invisible_) return NULL;
  ...

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc#newc...
runtime/vm/scopes.cc:345: void LocalScope::CaptureVariable(const String& name) {
I would still return the captured variable or a bool, so that the caller can
assert that the variable was found.

[hausner, 2013-08-08 09:26:06]

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc
File runtime/vm/scopes.cc (right):

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc#newc...
runtime/vm/scopes.cc:324: if ((var != NULL) && !var->is_invisible_) {
On 2013/08/07 18:28:26, regis wrote:
> As far as I understand, if a variable is marked invisible, you are not going
to
> find it in the parent scope. I guess that most of the time, there is no parent
> scope anyway, but still, it would be a bit more efficient to return
immediately:
> if (var != NULL) {
>   if (var->is_invisible_) return NULL;
>   ...

That may be a shortcut that works now, since we use the invisible flag only on
variables in the outermost scopes, as you say. But I think the optimization is
not worth the compromise in correctness (should we ever use "invisible" on other
variables).

https://codereview.chromium.org/22415002/diff/14001/runtime/vm/scopes.cc#newc...
runtime/vm/scopes.cc:345: void LocalScope::CaptureVariable(const String& name) {
On 2013/08/07 18:28:26, regis wrote:
> I would still return the captured variable or a bool, so that the caller can
> assert that the variable was found.

Ok, good point. will do in a follow-on CL.