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

WIP: make require() load at top level #1809

Closed
wants to merge 3 commits into from
Closed

WIP: make require() load at top level #1809

wants to merge 3 commits into from

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 21, 2012

This is the major piece of work needed to address #1411. Please check, esp. @JeffBezanson. I'm especially worried about the module context in toplevel.c, if the imported file has errors in it. Testing did not result in a segfault, but I doubt the thoroughness of this test.

On both the C side and the julia side this is ugly, but when I tried the more obvious thing in boot.jl I either (1) ran into cconvert not defined errors (presumably the conversion of a bool to an Int32 is not yet automatizable), (2) didn't figure out a way to generate 0::Int32 in a boot.jl-safe way, or (3) got a weird warning when I said import Core.include; include(fname::ByteString, inmain::Bool) = .... Hence I defined import_main. By this point the interface seems kinda dumb (why take a second argument to import_main??), but I suspect there's a cleaner solution, so I decided to leave my fingerprints on it.

Also, a more general interface would be import(fname, mod::Module), but I don't know whether that's desirable.

@staticfloat
Copy link
Member

Looks like we're missing the definition of a function called nproc()?

@timholy
Copy link
Member Author

timholy commented Dec 21, 2012

Rats, sorry about that. Not sure how that happened. I must have forgotten to rebuild before running make testall.

FYI that's failing for me now, but I am pretty sure it's nothing to do with this patch.

     * suitesparse
test error during :( isequal(\(se33, do33), do33) )
Error calling UMFPACK
 in default_handler at /home/tim/src/julia/base/test.jl:18
 in do_test at /home/tim/src/julia/base/test.jl:26
 in load_now at util.jl:238
 in runtests at ./runtests.jl:7
 in load_now at util.jl:238
 in load_now at util.jl:238
 in load_now at util.jl:250
 in runtests at ./runtests.jl:7
 in anonymous at no file:46
 in include at boot.jl:237
at /home/tim/src/julia/test/suitesparse.jl:7
make[1]: *** [all] Error 1
make: *** [testall] Error 2

@staticfloat
Copy link
Member

Are you building with an explicit USE_LIB64? If not, you may have a 32/64-bit mismatch, and need to cd deps; make distclean-suitesparse first. If you don't want to use 64-bit, you should make USE_LIB64=0.

@JeffBezanson
Copy link
Member

This is heading in the right direction, but it is something I will tackle.

@timholy
Copy link
Member Author

timholy commented Dec 21, 2012

OK, thanks. I'll leave this open, but I'll stop working on it.

@timholy
Copy link
Member Author

timholy commented Dec 24, 2012

Jeff, while in my patch I made require work either in the current module or in the main module, now I wonder whether require(filename, SomeModule) might indeed be useful. An example: say I don't want to make the Tk package a strict dependency of the Compose package, but I'd like to provide an optional "sub-package" that inserts a bunch of code into Compose to support Tk. The sub-package "ComposeTk" might look like this:

ComposeTk.jl:

require("Tk")
require("Compose")
require("compose_tk.jl", Compose)

and the file "compose_tk.jl" could be written using the private, not public, interface of Compose.

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.

3 participants