-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
sk/reloaded #25455
sk/reloaded #25455
Conversation
a0fec7a
to
6fd083d
Compare
Passed on FreeBSD, Win64. Unrelated failure on Win32 (restarted). Investigating macOS spawn failure. Update: spawn tests pass locally on macOS, so I assume the Travis failure is unrelated. |
6fd083d
to
1c6179c
Compare
src/toplevel.c
Outdated
@@ -165,6 +165,11 @@ jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex) | |||
jl_module_t *newm = jl_new_module(name); | |||
jl_value_t *defaultdefs = NULL, *form = NULL; | |||
JL_GC_PUSH4(&last_module, &defaultdefs, &form, &newm); | |||
// copy parent environment info into submodule | |||
jl_sym_t *uuid_sym = jl_symbol("#uuid"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would probably be better to make this part of the actual structure of the jl_module_t
, rather than making it an cryptic error to define a function named uuid
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's named #uuid
not uuid
so you can't accidentally use this name; we can certainly do that once we get this working, but in the meantime, this was much easier to get going with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions do for their types, though — I believe this was Jameson's point:
julia> @eval $(Symbol("#uuid")) = 1
1
julia> @eval $(Symbol("#uuid"))
1
julia> uuid() = 2
uuid (generic function with 1 method)
julia> @eval $(Symbol("#uuid"))
typeof(uuid)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem solved: 4fe702e. We can make it part of the module once this is all settled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't that conflict with mangled names in lowering now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea. How many hashes do I need to stick in front of this thing to make it safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that depends on how many nested closures you want to support before it clashes?
Note to self: 4fe702e passes on FreeBSD, and Win64 (Win32 failure is a timeout). |
e8efe35
to
f42006a
Compare
io = IOBuffer() | ||
write(io, UInt8(0)) | ||
uuid = pkg.uuid | ||
write(io, uuid === nothing ? UInt128(0) : UInt128(uuid)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could just use UUID(UInt128(0))
as a sentinel value instead of nothing
to avoid the union here.
1c499c7
to
9c1dc41
Compare
f38b87f
to
f446351
Compare
fe0b81e
to
c9c8f73
Compare
Developed together with vtjnash
c9c8f73
to
865c08e
Compare
The failures all seem to be unrelated so I’m pulling the trigger on this finally! |
Whoa!! 🎉 💯 |
For CI testing...