Julia allows rewriting imported bindings

[fcard]

265 2: son of 265 (featuring world-famous star: the global variable)

f() = string
g() = global string = "abc"

julia> f()
string (generic function with 14 methods)

julia> g()
WARNING: imported binding for string overwritten in module Main
"abc"

julia> f()
string (generic function with 14 methods)

And this:

f(x::typeof(string)) = 1
f(x::String) = 2
g() = (global string = "abc"; f(string))

julia> g()
WARNING: imported binding for string overwritten in module Main
1

Already reported in #11723 but the given explanation didn't convince me, so I am bringing it up again as my own bug report. This still looks like a language breaking bug to me.

[yuyichao]

You didn't break the language, you introduce undefined behavior in your own module. Givent an error is easy but it'll make interactive development harder. If you do care about this, you just shouldn't ignore the warning to begin with.

[fcard]

*Shrug* Looks like definable behavior to me, but what do I know. (nothing)

[Keno]

idk about "won't fix", seems like we can fix it eventually (e.g. once we start making speculative assumptions about globals when generating code). Given the warning though, it's not necessarily a priority though.

[yuyichao]

We've already forbidden any overwirte that changes the type and that's likely the speculative assumption we're going to make about globals. Also, some constant globals are being used in inner loops and it'll be a huge regression unless we always make non-speculative assumptions about their values so speculative assuptions about globals won't help this.

This is the kind of thing that shouldn't be allowed in runtime bug is useful for debugging/testing/interactive development (not limited to Main) so I think the current behavior is the best option. The user just shouldn't be ignoring warnings, they really mean that something the user don't want to happen could happen.

[fcard]

Ok.

[StefanKarpinski]

I'm reopening this because what a global binding refers to – imported or not – should be fully determined by parsing the module, without executing any code. We've had this "bug" for quite a long time and I've discussed it with @JeffBezanson, @vtjnash and others occasionally. The fact that g's definition assigns to global string should dictate that string is the global that g defines, not Base.string. Which means that calling f() before g() should be an undef ref error instead of the current behavior.

[yuyichao]

should be fully determined by parsing the module, without executing any code.

Not for the REPL.

Which means that calling f() before g() should be an undef ref error instead of the current behavior.

Changing to that behavior is certainly better, but it won't fix this issue since there's eval (which is also why REPL is special), unless we put a very tight restriction on when eval can be called and what it can do of course.

[yuyichao]

I'm reopening this because what a global binding refers to – imported or not – should be fully determined by parsing the module, without executing any code.

Actually this is not even true in a normal module when no metaprogramming is used. include is basically a special form of eval and it is impossible to statically evaluate it without executing any user code.

[vtjnash]

Ah, yep. Stefan is right. The issue title might need to be edited, but I nominate that for 1.0. Should be an easy fix to lowering.

[StefanKarpinski]

Just to be clear the fix to this problem is that it should not be possible to change what a global binding refers to:

• If g() is defined before f() is called, then string is a new global in the current module and calling f() should be an undefined variable error.
• If f() is called before g() is defined, then string refers to Base.string and the definition of g() should be an immediate error.

There's a few other ways this could be resolved, but I think this one is fairly straightforward and eliminates any confusion about what a binding refers to by the time a module is fully loaded.

@yuyichao, I understand how dynamic languages work, thanks.

[yuyichao]

Actually the case I'm talking about is

julia> const a = 1
1

julia> f() = a
f (generic function with 1 method)

julia> f()
1

julia> a = 2
WARNING: redefining constant a
2

julia> f()
1

which I've been assuming to be included in this issue due to the mentioning of #265. If the issue is only about not changing global binding, which is indeed the only case the first post is about then I certainly agree it should be fixed.

[fcard]

Yes, this issue was only about imported bindings getting confused with global variables, sorry.

What I meant by "breaking the language" was this:

julia> const x = 1
1

julia> module M
       import Main.x

       f(a::String) = repeat(a, 2)
       f(a::Int) = a + a

       g() = (global x = "abc"; f(x))
       end
M

julia> @code_typed M.g()
CodeInfo(:(begin 
        M.x = "abc"
        return $(Expr(:invoke, MethodInstance for f(::Int64), :(M.f), :(M.x)))
    end))=>Int64

Where the lowered code has an invocation to the wrong method.

[JeffBezanson]

So IIUC, you want an error instead of a warning? That might be fine, but I don't see why it's such a big deal. Warnings in julia are serious; they're basically errors that let you keep going for convenience.

I think we probably should make the change @StefanKarpinski describes, but it's orthogonal --- one issue is what causes the binding to be resolved (executing the statement global x = ... vs. just defining a function that contains such a statement), and the other issue is what to do when some code tries to change where the binding points (error or warning).

[fcard]

The warning is good enuf to me, the proposed changes are enough of a good ending.

[StefanKarpinski]

I'm pretty sure this no longer needs to be open.