Support for-in in analyzer2dart.

Support for-in in analyzer2dart.

BUG=
R=paulberry@google.com, sigurdm@google.com

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

[Johnni Winther, 2014-11-03 09:32:58]

johnniwinther@google.com changed reviewers:
+ paulberry@google.com, sigurdm@google.com

[Paul Berry, 2014-11-03 15:05:56]

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
File pkg/analyzer2dart/lib/src/cps_generator.dart (right):

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
pkg/analyzer2dart/lib/src/cps_generator.dart:364: node.identifier.name,
converter.convertElement(currentLibrary));
I'm confused.  The only situation in which accessSemantics.kind should be
AccessKind.DYNAMIC should be something like this:

  class A {
    var x;
    f() {
      for (x in ...) { ... }
    }
  }

In which case, it seems like creating a getter on the current library is the
wrong thing to do.

(Side note: there is currently a bug where the "x" in "for (x in ...) { ... }"
will be incorrectly classified by ACCESS_SEMANTICS_VISITOR as a read access
rather than a write access.  I'll upload a CL to fix that shortly.)

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
pkg/analyzer2dart/lib/src/cps_generator.dart:366: variableElement =
converter.convertElement(accessSemantics.element);
This seems like it will break if the variable in the for loop is undefined,
since in that case accessSemantics.element will be null.

[Johnni Winther, 2014-11-04 12:58:53]

PTAL

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
File pkg/analyzer2dart/lib/src/cps_generator.dart (right):

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
pkg/analyzer2dart/lib/src/cps_generator.dart:364: node.identifier.name,
converter.convertElement(currentLibrary));
On 2014/11/03 15:05:56, Paul Berry wrote:
> I'm confused.  The only situation in which accessSemantics.kind should be
> AccessKind.DYNAMIC should be something like this:
> 
>   class A {
>     var x;
>     f() {
>       for (x in ...) { ... }
>     }
>   }
> 
> In which case, it seems like creating a getter on the current library is the
> wrong thing to do.
> 
> (Side note: there is currently a bug where the "x" in "for (x in ...) { ... }"
> will be incorrectly classified by ACCESS_SEMANTICS_VISITOR as a read access
> rather than a write access.  I'll upload a CL to fix that shortly.)

The selector should have been a setter. Nice catch! Will be tested when we have
support for field declarations (soon-ish).

https://codereview.chromium.org/702453002/diff/20001/pkg/analyzer2dart/lib/sr...
pkg/analyzer2dart/lib/src/cps_generator.dart:366: variableElement =
converter.convertElement(accessSemantics.element);
On 2014/11/03 15:05:56, Paul Berry wrote:
> This seems like it will break if the variable in the for loop is undefined,
> since in that case accessSemantics.element will be null.

Yes. We seem to be missing a statically unresolved access semantics for the case
when implicit this cannot be assumed, i.e. static/top-level context.

[sigurdm, 2014-11-04 13:41:22]

lgtm

[Paul Berry, 2014-11-04 13:50:47]

lgtm

[Johnni Winther, 2014-11-04 14:54:06]

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