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

ResolveExport's exportStarSet argument disallows cycles that don't seem problematic #708

Closed
ajklein opened this issue Oct 7, 2016 · 4 comments

Comments

@ajklein
Copy link
Contributor

ajklein commented Oct 7, 2016

The exportStarSet passed to ResolveExport is used to cause an error in cases like the following:

A: import {x} from "B";
B: export * from "C";
C: export var y; export {y as x} from "B";

The sequence of ResolveExport calls here is:

  1. B.ResolveExport("x", «», «»)
  2. C.ResolveExport("x", «{B, "x"}», «B»)
  3. B.ResolveExport("y", «{B, "x"}, {C, "x"}», «B»)

where step (3) fails when hitting step 7 of ResolveExport (since B is already in the exportStarSet).

But that would have been perfectly legal without the export *:

A: import {x} from "B";
B: export {x, y} from "C";
C: export var y; export {y as x} from "B";

where the sequence is instead:

  1. B.ResolveExport("x", «», «»)
  2. C.ResolveExport("x", «{B, "x"}», «»)
  3. B.ResolveExport("y", «{B, "x"}, {C, "x"}», «»)
  4. C.ResolveExport("y", «{B, "x"}, {C, "x"}, {B, "y"}», «»)

and step (4) results in a a valid resolution.

What's the rationale for the former being an error?

@GeorgNeis who pointed out this oddity, and @allenwb for any history here.

@ajklein
Copy link
Contributor Author

ajklein commented Oct 18, 2016

@caridy @dherman for more modules expertise in case they happen to know what this restriction is for.

@ajklein
Copy link
Contributor Author

ajklein commented Dec 1, 2016

Recapping my understanding of a whiteboard discussion with @dherman: Dave's guess is that the export star set was meant to disallow circularities entirely within a graph of "export *" declarations, and that this failure just falls out of that. It might suffice to make export star detection a separate phase of checking in ModuleDeclarationInstantiation, and to throw away the exportStarSet once that phase is complete.

@ajklein
Copy link
Contributor Author

ajklein commented Jan 17, 2017

@caridy ping? I think you said you were going to take a look at this one.

@caridy
Copy link
Contributor

caridy commented Jan 18, 2017

Yes, will try to have something for next week.

caridy added a commit to caridy/ecma262 that referenced this issue Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants