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

Add at-isdefined #402

Closed
wants to merge 1 commit into from
Closed

Add at-isdefined #402

wants to merge 1 commit into from

Conversation

ararslan
Copy link
Member

Note that this is NOT functionally equivalent to @isdefined on 0.7; that isn't possible on earlier versions. Instead this defines @isdefined to wrap isdefined, which can only check for global variables.

@ararslan
Copy link
Member Author

CI failures on macOS are unrelated. Is this something we want or would it be too misleading to have it only support a very limited subset of its 0.7 functionality on 0.6?

@yuyichao
Copy link
Contributor

The subset where this behaves the same as @isdefined can be written as isdefined(@__MODULE__, ...) so I don't think we should have this.

@ararslan
Copy link
Member Author

Okay.

@ararslan ararslan closed this Sep 29, 2017
@ararslan ararslan deleted the aa/isdefined branch September 29, 2017 18:30
omus added a commit to omus/ODBC.jl that referenced this pull request Nov 27, 2017
quinnj pushed a commit to JuliaDatabases/ODBC.jl that referenced this pull request Nov 28, 2017
* Avoid using `@isdefined` on Julia 0.6

Not supported in Compat: JuliaLang/Compat.jl#402

* Support Sys.is<os> on Julia 0.6

* Add Julia 0.6 to CI

Note: Julia 0.5 was dropped from the CI as the REQUIRE file already
specifies a minimum of Julia 0.6.
@yuyichao yuyichao mentioned this pull request Dec 19, 2017
@nalimilan
Copy link
Member

I think we should definitely have this because when updating code from 0.6 using isdefined, the most direct replacement is @isdefined, not isdefined(@__MODULE__, ...). Indeed that's what the deprecation suggests. Otherwise, the deprecation message should be changed.

@yuyichao
Copy link
Contributor

yuyichao commented Feb 27, 2018

No that (edit: the depwarn on 0.7) is unrelated. @isdefined is the closest replacement on 0.7. It is not on 0.6 and adding a definition on 0.6 that does not do the same thing at all does not help.

@nalimilan
Copy link
Member

If isdefined(@__MODULE__, ...) is a good replacement, why couldn't we define @isdefined to use it on 0.6?

@yuyichao
Copy link
Contributor

couldn't we define @isdefined to use it on 0.6?

Because it's a good replacement of isdefined. It is not a good implementation (replacement) of @isdefined so such a definition will do completely different things on 0.6 and 0.7.

@yuyichao
Copy link
Contributor

In another word, if you really don't want to type @__MODULE__ defining Compat.@isdefined_in_module to do isdefined(@__MODULE__, ...) is certainly fine. The only thing that's wrong is to call it @isdefined.

Also, defining @isdefined on 0.6 is kind of possible. I don't think it can be done with any performance though (you need to try catch). Such a definition is mostly useless due to the performance issue but if the function of it is really useful for someone (i.e. checking isdefined for local var and not global var) it can be added to Compat and called @isdefined.

@yuyichao
Copy link
Contributor

And yet another way you can call it @isdefined is to make sure it either return the correct result or raise an error on old versions, it's not as good as something that is always correct but at least it won't lead to bugs/much confusion.

In this case, this can be done (conservatively) if you make sure the macro is only used in global scope. I'm not sure the best way to do this but here's one of the very old tricks I've used before.

julia> macro assert_global()
           mod = isdefined(Base, Symbol("@__MODULE__")) ? __module__ : current_module()
           @gensym dummy
           quote
               $(esc(dummy)) = nothing
               if !isdefined($mod, $(QuoteNode(dummy)))
                   error("Not in global scope")
               end
           end
       end
@assert_global (macro with 1 method)

julia> @assert_global

julia> let
       @assert_global
       end
ERROR: Not in global scope
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] macro expansion at REPL[0]:7 [inlined]
 [3] top-level scope at REPL[2]:2

It's pretty ugly though...........................

@yuyichao yuyichao mentioned this pull request Aug 9, 2018
jekbradbury added a commit to quinnj/Conda.jl that referenced this pull request Aug 9, 2018
The Compat situation for `isdefined`/`@isdefined` is complicated; see JuliaLang/Compat.jl#402.
stevengj pushed a commit to JuliaPy/Conda.jl that referenced this pull request Aug 10, 2018
* Fixes for 1.0

* Fix another issue in deps/build.jl

* Use `isdefined` on v0.6

The Compat situation for `isdefined`/`@isdefined` is complicated; see JuliaLang/Compat.jl#402.

* Always listen to Yichao

* using Compat inside submodule
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