Skip to content

Dart 2 frontend uses the wrong scope for type annotations #32752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rmacnak-google opened this issue Apr 3, 2018 · 9 comments
Closed

Dart 2 frontend uses the wrong scope for type annotations #32752

rmacnak-google opened this issue Apr 3, 2018 · 9 comments
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). customer-fuchsia type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@rmacnak-google
Copy link
Contributor

This is a simplification of an issue encountered with generated FIDL2 bindings in Fuchsia. It blocks enabling Dart 2 in Fuchsia.

import 'dart:math' as math;

class Foo {
  List<math.Point> math;
}

main() {
  print(new Foo());
}

works in Dart, but in Dart 2 yields

a.dart:4:8: Error: 'math.Point' can't be used as a type because 'math' doesn't refer to an import prefix.
  List<math.Point> math;
       ^^^^^^^^^^

@peter-ahe-google @a-siva @zanderso

@rmacnak-google rmacnak-google added legacy-area-front-end Legacy: Use area-dart-model instead. customer-fuchsia labels Apr 3, 2018
@zanderso zanderso added P0 A serious issue requiring immediate resolution type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Apr 3, 2018
@eernstg
Copy link
Member

eernstg commented Apr 4, 2018

The behavior that you've seen in Dart 2 is correct, and the behavior that you've seen elsewhere is a bug.

Dart does not have a notion of putting declared names into different namespaces according to the nature of their declaration (with one exception: labels). So math in the type annotation is looked up in the current scope, which is the instance scope of Foo, and in that scope the result will be the instance variable. It is a bug for any Dart tool (Dart 1 as well as 2) to resolve that occurrence of math to the library prefix.

Taking that into account, wouldn't it be possible to generate code which doesn't have that name clash?

@kmillikin
Copy link

For what it's worth, this is a duplicate of #32200

@kmillikin kmillikin removed the legacy-area-front-end Legacy: Use area-dart-model instead. label Apr 4, 2018
@eernstg
Copy link
Member

eernstg commented Apr 4, 2018

Also note #32248 which requests the relevant bug fix in the analyzer.

@zanderso
Copy link
Member

zanderso commented Apr 4, 2018

@eernstg Despite possibly being a bug, this is long-standing behavior of the VM and apparently also the analyzer, so I think we should consider the new behavior to be a breaking change. Maybe we should have a Dart 2 breaking change announcement from the language team for this. /cc @leafpetersen @a-siva

@rmacnak-google Do you think that mangling the generated library prefixes with a leading '$' would be sufficient? Can library prefixes have a leading '$'? /cc @abarth.

@zanderso zanderso added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Apr 4, 2018
@eernstg
Copy link
Member

eernstg commented Apr 4, 2018

Funny, the analyzer isn't even quite sure whether it's looking up math as a prefix:

import 'dart:math' as math;

class Foo {
  List<math.Point> math;
}

main() => print(new Foo());

produces

  hint • Unused import: 'dart:math' at n007.dart:4:8 • unused_import

I understand that even a bug fix can be a breaking change, organizationally. I hope it's manageable! On the other hand, I think we should maintain the property that "Dart has one name space" as far as possible (and nobody knows about labels, anyway ;-).

@leafpetersen
Copy link
Member

Posting an announcement about this seems like a reasonable idea, I can do.

@leafpetersen leafpetersen self-assigned this Apr 6, 2018
@vsmenon
Copy link
Member

vsmenon commented May 17, 2018

Is this still a P0?

@vsmenon
Copy link
Member

vsmenon commented May 17, 2018

Dropping the P0 as it appears @rmacnak-google unblocked Fuchsia. Please bump if I misunderstood.

Is the only open here to communicate the change?

@vsmenon vsmenon removed the P0 A serious issue requiring immediate resolution label May 17, 2018
@leafpetersen
Copy link
Member

I've announced that this is de facto changing. We have a bug (#32248) for the analyzer to fix this, everything else gets this from the CFE. Closing this, but re-open if there are any remaining issues that I've missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). customer-fuchsia type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants