Skip to content
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

Library-private final fields get different instances depending on how the library was imported #32601

Closed
pulyaevskiy opened this issue Mar 20, 2018 · 9 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug

Comments

@pulyaevskiy
Copy link
Contributor

I tried to find relevant issues via search but it didn't yield anything similar.

This may be me learning, but I find following behavior really confusing.

For instance, this Dart file:

// lib/src/a.dart
final _a = new A();

class A {}

void doWithA(String from) {
  print('Hash code of A from $from: ${_a.hashCode}');
}

Up until now I assumed that _a would always be the same instance, but it turns out not entirely true and there can actually be multiple instances of _a depending on how import / export statements are organized in a project.

It is a bit verbose to describe in this issue so I created example project here: https://github.com/pulyaevskiy/library-private-issue

Executing dart lib/main.dart there prints something like this for me:

Hash code of A from main: 931022103
Hash code of A from B: 85304837

Combination of export and import statements in that project is probably far from perfect and normally should be cleaned up. However it seems dangerous to even allow such scenarios.

@lrhn
Copy link
Member

lrhn commented Mar 20, 2018

Dart libraries are identified by their import URI. If you import the same file using two different URIs, say file:///something/lib/main.dart and package:library-private-issue/main.dart, then you get two different libraries with the same source code, each with their own separate variables. That's what is happening here.

This is expected behavior. To act differently, Dart would have to somehow recognize that it's the same file (either by content or by resolving the package: URI to a file: URI) which it doesn't do.

My personal rule of thumb is "never have lib in a path". In this case, your main library is specified with a path of /lib/main.dart which is causing your troubles.
I would recommend one of number of solutions:

  • Move the main.dart file to bin/.
  • Move the main method to a separate file from the actual library, one that does imports using package: URIs, and which is never imported again itself.
  • Start the program using dart package:library-private-issue/main.dart.

@pulyaevskiy
Copy link
Contributor Author

Thanks @anders-sandholm , this is really helpful. I wasn't sure which actual part caused this issue.

I must note though that I used lib/main.dart purposely as it is a standard way to organize Flutter apps. Moving this file in a different location will probably break all the tooling.

So in this case replacing relative imports with package: versions is the simplest.

I submitted relevant issue in flutter/flutter#15748 for completeness.

@matanlurey
Copy link
Contributor

Can we get this re-opened to discuss fixing these semantics in say, Dart 3?

@anders-sandholm @lrhn @leafpetersen

@anders-sandholm
Copy link
Contributor

With @zoechi responding "I see such issues very frequently. Either the analyzer and VM need to be changed to recognize such mistakes or as suggested follow pub package conventions by moving entry point files to bin/" to the Flutter issue, we need to do something.

Not clear to me if this is primarily an analyzer/front-end issue (giving better error messages) or a language/library issue.

@lrhn
Copy link
Member

lrhn commented Apr 3, 2018

The current model comes from a time when we expected Dart to run in the browser and import all libraries remotely by URL. It makes much less sense when imports are always files, and all import are file: URIs (or package: URIs resolving to file: URIs).

Changing the language specification is probably doable. We just say that the identity of package: URI-imported libraries is based on the underlying non-package: URI. It's more complexity, but it's definitely both specifiable and implementable (the dart:mirrors library needs to know about it if you look up libraries by URI, and might need to resolve package URIs at runtime, but otherwise it should all be front-end).

We could go even further and allow file: URI imports to base their identity on whether they reference the same file (which would also avoid the Windows-is-case-insensitive issues), and leave it up to tooling to figure out what that actually means. (Follow symbolic links? Recognize the same hard-linked file in different directories? Not just content based!)

In almost most cases, everything will be fine and simple, but we should at least be aware of edge cases.

@kevmoo
Copy link
Member

kevmoo commented Apr 5, 2018

@lrhn @anders-sandholm – which area should this be?

@lrhn
Copy link
Member

lrhn commented Apr 5, 2018

It's a language issue.
The language specifies that a library is identified by the URI it's imported from. That is what needs to change to completely avoid the issue. We can do that. It makes the story less simple, but the problem keeps cropping up, so we seem to have have a pitfall of failure with the current design.

Tools might be able to alleviate the problem by implicitly converting command line file references to package: URIs if the same file can also be referenced using a package URI. That will not solve cases where a bin/main.dart file refers to a package library using ../lib/something.dart, but we should probably issue a warning when we see that.

@lrhn lrhn added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug and removed closed-as-intended Closed as the reported issue is expected behavior area-multi labels Apr 5, 2018
@zoechi
Copy link
Contributor

zoechi commented May 8, 2018

Looks like a dup of #33076 (or the other way around)

@matanlurey
Copy link
Contributor

Duplicate of #33076.

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). type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants