-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Fix wrong use of isdefined
#21492
#21539
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
Conversation
`isdefined` will immediatly resolve the binding. Since we are only trying to skip the path lookup the weaker version `isbindingresolved` is enough. `isbindingresolved` is also used for this purpose in `read_verify_mod_list` in `src/dump.c`.
any quick and/or simple within-base way of testing this? |
I have been thinking about it, but nothing that is simple and wouldn't poison the test worker. |
You can have a test that runs outside of the main workers set. See how boundscheck_exec.jl and distributed_exec.jl are run. |
thanks that pointed me into the right direction. |
(this fixes the problems I had in #21492 (comment), thanks!) |
@tkelman Is the test sufficient? My only worry is that it will stop testing what it needs to test if |
better than no test! (should be 4 space indent though) |
Done :) one might think that by now I just would have fixed my editor.
…On Tue, 25 Apr 2017, 19:46 Tony Kelman, ***@***.***> wrote:
better than no test!
(should be 4 space indent though)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#21539 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAI3au_nXX69vHMNOA3cut8etiD9CWdpks5rzc74gaJpZM4NG6UJ>
.
|
@vtjnash Good to merge? |
Is the |
The warning is expected, but I don't know how to suppress it, since it is
coming from a different process.
…On Wed, 26 Apr 2017 at 21:44 Tony Kelman ***@***.***> wrote:
Is the WARNING: replacing module Test6c92f26 expected here? If so, should
it use @test_warn to suppress and check for that?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#21539 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAI3asQgTCCi_1CbV-gLsmeXV4P5yuhYks5rzzw8gaJpZM4NG6UJ>
.
|
set its stdout and/or stderr? |
This is the immediate fix from #21537.