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

Fix use of reexport inside of baremodules #33

Merged
merged 2 commits into from
Aug 24, 2021
Merged

Fix use of reexport inside of baremodules #33

merged 2 commits into from
Aug 24, 2021

Conversation

ararslan
Copy link
Collaborator

As described in #32, the generated code from the macro included references to Base, which notably is not defined in baremodules. We can get around this by moving the part that fetches and filters module names into its own function, ensure that gets used as a GlobalRef by the macro, and swap the eval for Core.eval, which is available in baremodules.

Fixes #32.

As described in issue 32, the generated code from the macro included
references to `Base`, which notably is not defined in `baremodule`s. We
can get around this by moving the part that fetches and filters module
names into its own function, ensure that gets used as a `GlobalRef` by
the macro, and swap the `eval` for `Core.eval`, which is available in
`baremodule`s.
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2021

Codecov Report

Merging #33 (89d2d79) into master (609c8fc) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #33   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           20        24    +4     
=========================================
+ Hits            20        24    +4     
Impacted Files Coverage Δ
src/Reexport.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 609c8fc...89d2d79. Read the comment docs.

@ararslan ararslan merged commit fd87dbf into master Aug 24, 2021
@ararslan ararslan deleted the aa/baremodule branch August 24, 2021 05:41
@torfjelde
Copy link

Hey! I have no idea why, but this PR seems to have broken a bunch of packages. We're currently experiencing the following error in Turing.jl:

ulia> using Turing
[ Info: Precompiling Turing [fce5fe82-541a-59a6-adf8-730c64b5f9a0]
ERROR: LoadError: LoadError: cannot assign a value to variable Core.Core from module Turing
Stacktrace:
 [1] include(mod::Module, _path::String)
   @ Base ./Base.jl:386
 [2] include(x::String)
   @ Turing ~/Projects/public/Turing.jl/src/Turing.jl:1
 [3] top-level scope
   @ ~/Projects/public/Turing.jl/src/Turing.jl:33
 [4] include
   @ ./Base.jl:386 [inlined]
 [5] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::Nothing)
   @ Base ./loading.jl:1235
 [6] top-level scope
   @ none:1
 [7] eval
   @ ./boot.jl:360 [inlined]
 [8] eval(x::Expr)
   @ Base.MainInclude ./client.jl:446
 [9] top-level scope
   @ none:1

which is fixed by restricting to Reexport@1.2.0. Again, I don't know why this occurs but it seems like this PR is somehow the cause 😕

@devmotion
Copy link
Contributor

Ah I assume the problem is that the module in Turing is called Core.

@torfjelde
Copy link

Yeah I figured too, but this is not disallowed, right?

@ararslan
Copy link
Collaborator Author

Really sorry, folks! Reexport v1.2.2 has been released which I've confirmed works with Turing. I'm also yanking Reexport v1.2.1 from the registry.

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.

The Macro "@reexport" Fails in Modules Declared with "baremodule"
4 participants