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

VM fails language/prefix_core_test #3638

Closed
peter-ahe-google opened this issue Jun 14, 2012 · 11 comments
Closed

VM fails language/prefix_core_test #3638

peter-ahe-google opened this issue Jun 14, 2012 · 11 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@peter-ahe-google
Copy link
Contributor

The VM fails this test:

import('dart:core', prefix:'core');

class Object {
}

main() => core.print(new Object());

@DartBot
Copy link

DartBot commented Aug 17, 2012

This comment was originally written by @mhausner


The test is meaningless with the new scoping rules for imported names. The local definition for Object is shadowing the one in the core library.

Also, print is not defined in dart:core but in dart:builtin.

Removed the test.


Set owner to @mhausner.
Added Fixed label.

@peter-ahe-google
Copy link
Contributor Author

I wouldn't say the test is meaningless. The VM fails the test, but dart2js passes. So we have a problem.


Added label.

@DartBot
Copy link

DartBot commented Aug 19, 2012

This comment was originally written by @mhausner


The problem seems to be that the VM and dart2js have a different set of library names. The VM defines print() in a library called dart:builtin. If the names of the libraries must match, then this should be specified in the spec.

@peter-ahe-google
Copy link
Contributor Author

It is already specified by the specification. See section "13.2 Imports":

"The dart core library dart:core is implicitly imported into every dart library other than itself via an import clause of the form

  import ‘dart:core’;

unless the importing library explicitly imports dart:core."

This means that dart:core is implicitly imported. Since the specification doesn't say that other libraries are imported implicitly, it means that dart:core is also the only library to be imported implicitly.


Added Triaged label.

@iposva-google
Copy link
Contributor

As comment 2 points out there is a mismatch between whether print is considered part of the core library or not. I do not recall any definite decision on this matter. I have filed issue #4806 to resolve this matter. One could argue that dart2js is the one at fault to a similar degree.

As far as I understand prefixing core for functions and classes actually defined in dart:core works.


Added Fixed label.

@peter-ahe-google
Copy link
Contributor Author

Please restore the test until this is resolved.


Added Triaged label.

@andersjohnsen
Copy link

This code is now working. Should we close?

@peter-ahe-google
Copy link
Contributor Author

We can close this issue if the test has been restored.

@iposva-google
Copy link
Contributor

The original cannot be restored as it is incorrect. When implementing the latest language semantics we added language/import_core_prefix_test which is essentially the equivalent, but correct test according to the language spec.


Added AssumedStale label.

@peter-ahe-google
Copy link
Contributor Author

The tests are not equivalent, and there is nothing wrong with the original test (except that it uses the old library syntax which language/import_core_prefix_test also does).

In particular, language/import_core_prefix_test does not test what happens when you make a conflict with a core class, also, language/import_core_prefix_test does not test that print is available in the core library.


Added Triaged label.

@DartBot
Copy link

DartBot commented Nov 9, 2012

This comment was originally written by @mhausner


Updated language/import_core_prefix.dart with the test cases of the original test.
r14719.


Added Fixed label.

@peter-ahe-google peter-ahe-google added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Nov 9, 2012
copybara-service bot pushed a commit that referenced this issue Nov 11, 2022
Also removes package:oauth2 from DEPS (it is now vendored in pub).

See dart-lang/oauth2#137.

Changes:
```
> git log --format="%C(auto) %h %s" 65c7f3e..6ac42d7
 https://dart.googlesource.com/pub.git/+/6ac42d76 Use package:vendor to vendor package:tar and package:oauth2 (#3638)
 https://dart.googlesource.com/pub.git/+/817fcf13 blast_repo fixes (#3646)
 https://dart.googlesource.com/pub.git/+/738d963c blast_repo fixes (#3644)
 https://dart.googlesource.com/pub.git/+/a73598b5 Refactor HTTP retries (#3325) (#3590)
 https://dart.googlesource.com/pub.git/+/5527068c New command `dart pub cache preload` (#3636)

```

Diff: https://dart.googlesource.com/pub.git/+/65c7f3e528f3f9978c4330cbd471070f17370f65~..6ac42d7644dedfcc500147ab47886eecab4b1b38/
Change-Id: I6d2dffcac67b4bd1c1c91be952cca65c84d85493
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/269301
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
Reviewed-by: Jonas Jensen <jonasfj@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants