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

for-loop in top-level if-block blocks variable re-binding the first time around. #17387

Closed
mauro3 opened this issue Jul 12, 2016 · 6 comments · Fixed by #22984
Closed

for-loop in top-level if-block blocks variable re-binding the first time around. #17387

mauro3 opened this issue Jul 12, 2016 · 6 comments · Fixed by #22984
Assignees
Labels
parallelism Parallel or distributed computation

Comments

@mauro3
Copy link
Contributor

mauro3 commented Jul 12, 2016

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.5.0-dev+5297 (2016-07-11 19:18 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 0572533* (0 days old master)
|__/                   |  x86_64-pc-linux-gnu

julia> if true
           @everywhere a=1
           @show pi = 1
           @show pi==1
           @show typeof(pi)
           theta0 = pi==1 ? 1 : error("Should not get here!")
       end
WARNING: imported binding for pi overwritten in module Main
pi = 1 = 1
pi == 1 = false
typeof(pi) = Irrational{:π}
ERROR: Should not get here!
 in macro expansion; at ./REPL[2]:6 [inlined]
 in anonymous at ./<missing>:?
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

Note that: the if and @everywhere is necessary. Exchanging the if with a let or for stops the bug. Executing the the if-block again does not error again. Works on 0.4.

@mauro3
Copy link
Contributor Author

mauro3 commented Jul 12, 2016

Another, case which seems related:

  _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.5.0-dev+5297 (2016-07-11 19:18 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 0572533* (1 day old master)
|__/                   |  x86_64-pc-linux-gnu

julia> for i=1:2
           if i==1
               tests = [1,2]
           elseif i==2
               @everywhere tests = [1]
               @show tests
           end
       end
ERROR: UndefVarError: tests not defined
 in macro expansion; at ./show.jl:198 [inlined]
 in macro expansion; at ./REPL[1]:6 [inlined]
 in anonymous at ./<missing>:?
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

again running it the second time works. (If this is a different issue, let me know and I'll file it separately.)

@kshyatt kshyatt added the parallelism Parallel or distributed computation label Jul 12, 2016
@amitmurthy
Copy link
Contributor

Since a for-loop allocates local variables freshly for every iteration, http://docs.julialang.org/en/latest/manual/variables-and-scoping/#man-for-loops-scope , the second case has a partial explanation.

In the second loop iteration, @everywhere defines tests in global scope, but the following line still refers to an undefined tests in local scope (from the compilation phase?).

Maybe a different issue from the first.

@amitmurthy
Copy link
Contributor

The following code also has the same issue:

if true
    for _ in 1:1
    end
    @show pi = 1
    @show pi==1
    @show typeof(pi)
    theta0 = pi==1 ? 1 : error("Should not get here!")
end

Something to do with for-loop scoping in top-level if statements.

@amitmurthy amitmurthy changed the title @everywhere in if-block blocks variable re-binding for-loop in top-level if-block blocks variable re-binding the first time around. Jul 13, 2016
@amitmurthy
Copy link
Contributor

What should this be labelled under? lowering?

@JeffBezanson JeffBezanson self-assigned this Jul 13, 2016
@JeffBezanson
Copy link
Member

Might be related to #18933

@vtjnash
Copy link
Member

vtjnash commented Jul 27, 2017

It's partially related to #18933, but actually is behaving per-design and is just part of the current definition of scope (and the invalid use of inference at toplevel with the current scope rules). It should be fixed by #19324.

vtjnash added a commit that referenced this issue Jul 27, 2017
vtjnash added a commit that referenced this issue Jul 27, 2017
vtjnash added a commit that referenced this issue Jul 27, 2017
vtjnash added a commit that referenced this issue Jul 28, 2017
vtjnash added a commit that referenced this issue Jul 28, 2017
vtjnash added a commit that referenced this issue Aug 3, 2017
This changes the meaning of `global` from being a directive when used at toplevel,
which forces the introduction of a new global when used in certain contexts,
to always being just an scope annotation that there should exist a global binding for the given name.

fix #18933
fix #17387 (for the syntactic case)
vtjnash added a commit that referenced this issue Aug 3, 2017
This changes the meaning of `global` from being a directive when used at toplevel,
which forces the introduction of a new global when used in certain contexts,
to always being just an scope annotation that there should exist a global binding for the given name.

fix #18933
fix #17387 (for the syntactic case)
vtjnash added a commit that referenced this issue Aug 5, 2017
This changes the meaning of `global` from being a directive when used at toplevel,
which forces the introduction of a new global when used in certain contexts,
to always being just an scope annotation that there should exist a global binding for the given name.

fix #18933
fix #17387 (for the syntactic case)
vtjnash added a commit that referenced this issue Aug 7, 2017
This changes the meaning of `global` from being a directive when used at toplevel,
which forces the introduction of a new global when used in certain contexts,
to always being just an scope annotation that there should exist a global binding for the given name.

fix #18933
fix #17387 (for the syntactic case)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants