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

Change from isdefined function to @isdefined macro #94

Closed
wants to merge 1 commit into from

Conversation

pbouffard
Copy link

In Julia v0.7.0 the function form is deprecated and results in what looks like a parse error:

julia> Pkg.build("Conda")
INFO: Building Conda
=================================================================[ ERROR: Conda ]==================================================================

LoadError: type CodeInfo has no field def
in expression starting at /Users/patrick/.julia/v0.7/Conda/deps/build.jl:7

===================================================================================================================================================

=================================================================[ BUILD ERRORS ]==================================================================

WARNING: Conda had build errors.

 - packages with build errors remain installed in /Users/patrick/.julia/v0.7
 - build the package(s) and all dependencies with `Pkg.build("Conda")`
 - build a single package by running its `deps/build.jl` script

===================================================================================================================================================

In Julia v0.7.0 the function form is deprecated and results in what looks like a parse error:

```
julia> Pkg.build("Conda")
INFO: Building Conda
=================================================================[ ERROR: Conda ]==================================================================

LoadError: type CodeInfo has no field def
in expression starting at /Users/patrick/.julia/v0.7/Conda/deps/build.jl:7

===================================================================================================================================================

=================================================================[ BUILD ERRORS ]==================================================================

WARNING: Conda had build errors.

 - packages with build errors remain installed in /Users/patrick/.julia/v0.7
 - build the package(s) and all dependencies with `Pkg.build("Conda")`
 - build a single package by running its `deps/build.jl` script

===================================================================================================================================================
```
@pbouffard
Copy link
Author

pbouffard commented Nov 17, 2017

I'm a bit puzzled why this doesn't seem to have impacted others (at least, I can't find evidence of that) since I'd assume that this package would be used by most who are on master of Julia. Aside from that a couple of questions arise:

  1. Shouldn't a deprecation warning also have popped up? Or was that not possible because the compiler was confused and didn't even recognize isdefined as a symbol?
  2. I see that builds with Julia 0.5.0 and 0.6.0 fail because the macro form doesn't exist yet in that version. Does that need to be fixed for this to merge?

My first PR, be gentle ;)

@omus
Copy link
Collaborator

omus commented Nov 18, 2017

I see that builds with Julia 0.5.0 and 0.6.0 fail because the macro form doesn't exist yet in that version. Does that need to be fixed for this to merge?

Typically with Julia the latest syntax is introduced using the Compat.jl package. The @isdefined(x) macro unfortunately cannot be supported in earlier version of Julia. Instead you can use the form isdefined(@__MODULE__, :x) which should work in Julia 0.7 and earlier versions.

I'm a bit puzzled why this doesn't seem to have impacted others (at least, I can't find evidence of that) since I'd assume that this package would be used by most who are on master of Julia.

I'm also unclear on why this issue hasn't appeared for others (including myself). I'm really not sure why you were getting an error here.

@pbouffard
Copy link
Author

Hmm, I think the issue I'm seeing probably has nothing to do with the Conda package. It seems that my build for some reason doesn't like the function form of isdefined, but only in certain contexts:

MacBook-Pro-3:julia patrick$ ./julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-DEV.2517 (2017-11-16 02:46 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 8c3f8b1eeb (3 days old master)
|__/                   |  x86_64-apple-darwin16.7.0

julia> if !isdefined(:ROOTENV); println("not defined"); end
ERROR: type CodeInfo has no field def
Stacktrace:
 [1] firstcaller(::Array{Union{Ptr{Void}, Base.InterpreterIP},1}, ::Tuple{Symbol}) at ./deprecated.jl:100
 [2] firstcaller(::Array{Union{Ptr{Void}, Base.InterpreterIP},1}, ::Symbol) at ./deprecated.jl:84
 [3] depwarn(::String, ::Symbol) at ./deprecated.jl:69
 [4] top-level scope

julia> isdefined(:ROOTENV)
WARNING: `isdefined(:symbol)` is deprecated, use `@isdefined symbol` instead
Stacktrace:
 [1] depwarn(::String, ::Symbol) at ./deprecated.jl:68
 [2] top-level scope
 [3] eval(::Module, ::Expr) at ./repl/REPL.jl:3
 [4] eval_user_input(::Any, ::Base.REPL.REPLBackend) at ./repl/REPL.jl:69
 [5] macro expansion at ./repl/REPL.jl:100 [inlined]
 [6] (::getfield(Base.REPL, Symbol("##1#2")){Base.REPL.REPLBackend})() at ./event.jl:95
in expression starting at no file:0
false

julia> 

@pbouffard pbouffard closed this Nov 19, 2017
@yuyichao
Copy link
Collaborator

Seems to be a base julia bug and the depwarn code may need to be updated for the toplevel frame.

@pbouffard
Copy link
Author

I'm going to pull the latest master of the Julia repo and see if I can still reproduce. If so I'll open a ticket over there.

@pbouffard
Copy link
Author

See JuliaLang/julia#24658

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