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

Better re-export handling #1418

Merged
merged 4 commits into from
Aug 25, 2015
Merged

Better re-export handling #1418

merged 4 commits into from
Aug 25, 2015

Conversation

garyb
Copy link
Member

@garyb garyb commented Aug 24, 2015

Resolves #1244. Resolves #291 in a slightly different way too (if A explicitly imports values from B and then re-exports B, only the values it imported will be re-exported).

@garyb
Copy link
Member Author

garyb commented Aug 24, 2015

All green at last!

paf31 added a commit that referenced this pull request Aug 25, 2015
@paf31 paf31 merged commit 002af23 into master Aug 25, 2015
@paf31 paf31 deleted the 1244-reexports branch August 25, 2015 03:40
@paf31
Copy link
Contributor

paf31 commented Aug 25, 2015

👍 Thanks!

Edit: Actually, after I cabal installed this, transformers test suite no longer builds - it complains about things not being in scope, but they should be because of module reexports. Not sure what's going on, but I think we should figure it out before releasing the compiler.

@garyb
Copy link
Member Author

garyb commented Aug 25, 2015

They run okay for me now, did you make those changes for v0.7.0-rc.2 after posting this?

The changes you made look right to me - you can't export a module anymore unless you import it, the reasoning for this being only the specific values imported into the current module are re-exported when you export a module.

@paf31
Copy link
Contributor

paf31 commented Aug 25, 2015

The tests run ok? I get Unknown value callCC since callCC is imported via a module reexport in the tests. This is with what's on master.

@garyb
Copy link
Member Author

garyb commented Aug 25, 2015

Yep. Tried cleaning the output folder?

@garyb
Copy link
Member Author

garyb commented Aug 25, 2015

Yup, definitely works for me - I just installed the master version of psc on my Mac, and with a fresh checkout of transformers the tests run fine.

@paf31
Copy link
Contributor

paf31 commented Aug 25, 2015

Yes, I just removed output and I get the same error. I'll reinstall psc to be sure.

@paf31
Copy link
Contributor

paf31 commented Aug 25, 2015

Yes, I don't see module reexports in the externs files. Is that an issue?

@paf31
Copy link
Contributor

paf31 commented Aug 25, 2015

Seems like it would be an issue if we want to support separate compilation without looking at original source files.

@garyb
Copy link
Member Author

garyb commented Aug 25, 2015

#1425 should sort that out.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 55ba265 on 1244-reexports into ** on master**.

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

Successfully merging this pull request may close these issues.

None yet

3 participants