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

rules for shadowed bindings? #1357

Closed
dcampbell24 opened this issue Oct 10, 2012 · 28 comments
Closed

rules for shadowed bindings? #1357

dcampbell24 opened this issue Oct 10, 2012 · 28 comments
Labels
breaking This change will break code needs decision A decision on this change is needed

Comments

@dcampbell24
Copy link
Contributor

im is not a reserved word.

julia> im = 7
7
julia> 3im
21

@dcampbell24
Copy link
Contributor Author

Maybe it just needs to be a const?

@JeffBezanson
Copy link
Member

Different namespace. Base.im is const.
On Oct 10, 2012 4:13 PM, "David Campbell" [email protected] wrote:

Maybe it just needs to be a const?


Reply to this email directly or view it on GitHubhttps://github.com//issues/1357#issuecomment-9317761.

@dcampbell24
Copy link
Contributor Author

t.jl

println(im)
module Normal
import Base.*

im = 3
println(3im)

end

$ julia im.jl 
0 + 1im
9

t2.jl

module Normal
im
end

$ julia t2.jl 
im not defined
 in load_now at util.jl:231
 in load_now at util.jl:245
 in require at util.jl:174
 in process_options at client.jl:178
 in _start at client.jl:232
at /home/david/tmp/jtest/t.jl:3
 in load_now at util.jl:231
 in load_now at util.jl:245
 in require at util.jl:174
 in process_options at client.jl:178
 in _start at client.jl:232
 in load_now at util.jl:256
 in require at util.jl:174
 in process_options at client.jl:178
 in _start at client.jl:232

julia> Base.im = 6
type Module has no field im

I am confused. There is a non-const and const version of im both imported from Base but you can only use the const version of im by typing Base.im and trying to assign to Base.im reports that is does not exist?

@dcampbell24
Copy link
Contributor Author

julia> import Base.im

julia> im = 5
Warning: imported binding for im overwritten in module Main
5

This behavior is very confusing. Depending on how you import the value, you may or may not get a warning and you are still allowed to effectively overwrite a value that is supposed to be a constant.

@JeffBezanson
Copy link
Member

You are not overwriting or changing the constant. You are creating a new module-local binding for im. It is a separate variable in a different namespace. Being able to do this is the whole point of namespaces. Base.im still exists with its usual value.

If the sequence import Base.*; im = 5 gave an error, then you'd have to carefully arrange not to import any names you plan on defining in the current module. That would be very annoying. You'd constantly be adding statements to exclude more variables.

I decided only to give the warning for code that directly contradicts itself, such as the explicit import import Base.im; im=5. I don't give a warning for import Base.*; im=5 since you might not realize that Base.im exists. But I could just as easily give a warning in both cases if we think that's better.

@dcampbell24
Copy link
Contributor Author

On one line "im" refers to the constant Base.im and on the next line to the yet to exist Main.im. This is confusing regardless of the story told to explain why it happens. When a variable is imported without any prefix, typing that variable name without any prefix, should refer to the variable that was imported, not a new binding in the current namespace. I hardly think allowing users to silently shadow variables is the whole point of name spaces.

It may be very annoying, but that is a sign of a larger problem. Using the import Module.* syntax destroys namespace separation and should be used sparingly. If just doing import Base.* causes namespace hell, then there is too much exported in Base.

I don't think the ability to silently redefine functions and function methods is any better, but maybe this discussion should be taken somewhere else.

@JeffBezanson
Copy link
Member

So you think imported bindings should take precedence over bindings in the current module? I.e. import Base.*; im=5; println(im) should either be an error, or print the value of Base.im? To me this generally violates expectations around scoping, where more local things tend to take precedence.

Most people expect to have simple things like +, -, im, etc. available by default without a lot of import statements.

It's true that you might want to be careful with import Module._, but it does not destroy anything. Other modules remain fully untouched, and nothing in another module can take control over how your identifiers resolve. For example import Foo._; x = 5; println(x)will always print 5 no matter what goes on in Foo. Or, if you want Foo'sx, then don't shadow it. I fail to see how that destroys namespace separation. That seems to be a perfect example of namespace separation to me.

@StefanKarpinski
Copy link
Member

I have to agree that I find the current behavior kind of confusing. I think the issue for me is that writing import Base.* feels like it should cause im to become a const in the current module like it was declared in Base. Therefore assigning to it should be an error. In fact, I think this is good place for this conversation because the original subject of this issue is such a stinky code smell that I can't believe I hadn't gotten a whiff of it before (ok, I got way too into that metaphor). You should not be able to do this:

julia> im = "foo"
"foo"

julia> im
"foo"

That's really bizarre. The im binding should be constant not only in the Base module but also in any module it is imported into. If you want to override im and still write import Base.* then you have to follow it with global im (or whatever we decide is a less confusing syntax for that).

@dcampbell24
Copy link
Contributor Author

You are right about scoping. I wasn't thinking of importing as just creating new bindings in the local module for values in another module, but as pulling the bindings of the other module into the current one, that is creating a shorthand for using the bindings in the imported module. I didn't realize that whether the bindings in the imported module were const or not didn't matter at all or that you can use a module without importing it. This is why I felt the bindings created by the import should work the same way as the bindings in the imported module. I am troubled by the instability of the meaning of im and other bindings, but am not sure what the best solution is. I appreciate your patience and feedback.

@JeffBezanson
Copy link
Member

Yes, the const-ness is unrelated. If you are allowed to make a new variable shadowing an imported one, it has its own const status (since it is a new variable). If Base.im's const-ness affects the const-ness of other variables in other modules also called im, that would seem to me to be inappropriate leakage of the kind modules are supposed to prevent.

The point about the warning I can understand; maybe we should warn about all shadowed bindings.

It helps to consider a name other than im. start is a good example, since it's a common word that also happens to be in Base, and people might not know about it. Somebody might want to use it as a variable name. We aren't going to give an error for typing start=0 at the prompt, especially since it currently works fine. Without disturbing all the other very important uses of start. That is what modules are all about.

@JeffBezanson
Copy link
Member

I'm surprised nobody has used this example yet, which is the only real bad case I can think of:

julia> module Foo
       import Base.*
       println(im)
       im=1
       println(im)
       end
0 + 1im
1

The problem here is that the binding for im changes mid-module. That case, where you have already used the name in one way and then changed, is a candidate for being an error.

But, the assumption is you have some awareness of what variables you're introducing (assigning to) in your module. After all, it can be determined by looking locally at just your module. That is a good property to have, and the rest is slightly less important.

@StefanKarpinski
Copy link
Member

I get the reasoning behind the current behavior (you're introducing a new binding, not changing the imported one) but I still find it kind of unsettling, but I'll have to sleep on it a bit.

@pao
Copy link
Member

pao commented Oct 11, 2012

Part of the mismatch here, it seems, is that technical users might expect certain bindings (im, pi, e) to act as if they were literals. Having them change is more like redefining the value of 1 than that of start. That those are really constant bindings in Base to a literal is, in this context, an implementation detail.

Of course, not all users fall into domains where that makes sense. I've used i and j as loop variables in MATLAB code tons of times.

@StefanKarpinski
Copy link
Member

I think part of the issue is that intuitively when I write import Base.* I expect bindings like im to exist in some actual sense instead of materializing or not later depending what happens elsewhere.

@dcampbell24
Copy link
Contributor Author

I think that import should be doing something more along the lines of what I thought it was doing. I find this odd:

julia> module Foo
       a = 3
       b = 45
       export b
       end

julia> Foo.a
3
julia> Foo.b = 7
type Module has no field b

Why doesn't a need to be exported for me to use it in Main, and why can't I assign a new value to b even though it is exported? Okay, so I can import b into Main and create a new binding, but that typically is not how one wants global variables to behave. If things worked as I thought, then if a one wanted to share a value, but not have it change, he would make it a const. If a variable is supposed to be totally private, it would just need to not be exported.

@StefanKarpinski
Copy link
Member

The motivation behind not allowing assignment to bindings from outside a module was to have some level of "closure" and thereby leave more room for potential static analysis of modules. I.e. once a module is defined, its bindings are essentially read-only. There are some exceptions, of course, since you can expose functions that mutate the state:

julia> module Foo
         import Base.*
         export x, inc_x
         x = 1
         function inc_x()
           global x
           x += 1
         end
       end

julia> import Foo.*

julia> x
1

julia> inc_x()
2

julia> x
2

julia> Foo.x
2

I'm not sure how I feel about the fact that Foo.x and Main.x actually refer to the same thing here and changing one changes the other. That really seems like spooky action at a distance and may be related to why this whole thing feels confusing.

Back to the bit about not allowing assignment to module bindings, the point of that choice is that as long as a module binding isn't captured in any closure, you can be sure that it won't change in the future, which can be helpful to the compiler. But personally, I'd rather make global bindings default to being type-const [#964] but allow assigning new values. That would be more helpful to the compiler and feel less restrictive.

@JeffBezanson
Copy link
Member

You should not be worried about what "exists in some actual sense", but about how concrete sequences of statements behave. And it is not surprising for x to equal 5 after writing x=5.

Everybody seems to agree that it is nice to be able to access all variables if you prefix them Foo.a, regardless of whether they are exported. Anything else would be too restrictive. Disallowing Foo.a=2 was a conservative choice to start with; it can eventually be allowed without breaking anything.

The exception you give is not an exception at all; the assignment takes place in the same module. And if a variable's value is changing and you import that variable, you will see the changes. I have no idea what's spooky about that. What other behavior would make sense? If imported x remained constant but Foo.x gave the changing value we would have a mysteriously copied variable. Isn't it more confusing for two things you thought were the same to behave differently?

@StefanKarpinski
Copy link
Member

Telling people what to worry about doesn't really help. Having x equal to 5 after writing x=5 is surprising if you expect x to be a constant, which is the entire point of this discussion.

I think the root of the "spooky action" issue is a difference in mental models for what import does. The current model is that imported bindings are actually shared between modules: after writing import Foo.x one has that Foo.x and Main.x are the same binding. I would expect writing import Foo.x to be like writing x = Foo.x – i.e. the effect is that Main.x and Foo.x are different bindings with the same value (initially). Of course, import doesn't have to actually perform all those assignments – if a binding is never used, then there's no need to do anything with it.

There's also this oddity which maybe deserves an issue of its own. After doing the above, I tried the following to see if I could affect the binding Foo.x via the exported x:

julia> function inc_x2()
         global x
         x += 1
       end

julia> inc_x2()
in inc_x2: x not defined
 in inc_x2 at none:3

I could not, but this seems like not what ought to happen in any case.

@JeffBezanson
Copy link
Member

What gives me trouble here is the premise that somebody has written im=5 yet also wants im to be a constant that can't be changed. Why would you want both things simultaneously?

Maybe we could say the intended meaning of x=5 is "make x 5 unless there is some other x around", but that leaves you in this uncertain state where it's hard to know whether the assignment will work.

The behavior of global x is to do everything that an assignment would do, except actually assigning a value. Your new example is equivalent to import Base.*; global im; println(im), so you get an undefined var error. Very likely, we'd prefer never to need global x declarations at the top level. Fixing the behavior of method definitions (there's an issue open about it) will give us this.

If you have a mutable global variable, you definitely don't want to copy it by default. The var's behavior is dictated by its owner module, so if the owner says it changes over time then that's what you get. Without knowing the detailed semantics of the variable, you don't know what it means to "snapshot" it.

@dcampbell24
Copy link
Contributor Author

Wanting im to be a constant and having written im=5 is an error, not a desire to be illogical. Importing arbitrary bindings may make it difficult to know whether something is bound or not, but if the assignment does not work, you get an error. It is easier to reason about getting an error than having a value not be what you thought it was and work anyway, but cause your results to be wrong.

@JeffBezanson
Copy link
Member

There is not one example here of having a value not be what you thought it was. I understand the argument for giving an error, but the current design does not lead to a variable getting an inscrutable value. After writing im=5, there seem to be only two reasonable outcomes: an error, or im equals 5.

If we give an error, then we mandate the following workflow: import what you need, set up some definitions, then see what errors you get and fix up all your imports and names to allow you to use the names you want. That third step is just unnecessary.

@JeffBezanson
Copy link
Member

Oh, there's also a fourth step: when somebody else's module changes, see what new errors you get and do the busy work of avoiding clashes in your names. That kind of thing is not supposed to happen.

@dcampbell24
Copy link
Contributor Author

Someone could try using im expecting it to be equal to Base.im and have it be 5. Just because code exists within a module someone is adding code to does not mean he knows what all of the assignments are in the module.

I don't see what the big issue is with fixing name clashes in an import. Why is it expected that there should be name clashes and this is a good way to program? When someone writes code that does not make sense, it is normal to get errors and to have to fix them. Yes, this may mean changing the name of a variable or not importing a variable you are not using. You'd have to rename it anyway if you actually want to use these two different variables at the same time and if you aren't using the variable, why do you need to import it?

@JeffBezanson
Copy link
Member

Adding code to a module you're not familiar with is always going to be a problem. I could still do the following

module Indiana
import Base.* except pi    # we don't have "except" yet but we should
pi = 3.2
...

Exact same problem. You could add code to this module expecting pi to have its usual value, but it doesn't.

@dcampbell24
Copy link
Contributor Author

Assuming people follow reasonably standard conventions for the location of import statements, I think import statements are more likely to be noticed than assignment statements.

@JeffBezanson
Copy link
Member

Our latest thinking is that constructs involving the same word (import) should behave the same, but that the current behavior of import Foo.* is useful, and should therefore be renamed. Perhaps to something like use Foo or using Foo, which means you are using its bindings in a read-only way, where they can be shadowed, and method definitions will not add to its functions by default. Then import would add things in a stricter way, where shadowing them would be an error. You should also not be able to switch between shadowing and not (in either case).

Saying we are "expecting name clashes" is putting the wrong spin on it. This kind of issue is unavoidable. You can have a global x, then also say x=0 at the start of a function and it is understood to be a local x. That is what you want almost 100% of the time, so nobody really has a problem with it. Yes, some people prefer var x=0, and I acknowledge their arguments, but we're not going to do that.

Sure I'm all for errors for code that doesn't make sense, but it's a problem if you can't know whether your code makes sense without knowing the full list of bound names in the current context. Telling people to individually import everything is absurdly pedantic. Sure we can make Base smaller, but that only goes so far. Most often that will just mean copypasting 5-10 lines of import * boilerplate instead of one.

Example from python:

In [7]: from numpy.random import random

In [8]: random
Out[8]: <function random_sample>

In [9]: random = 1

In [10]: random
Out[10]: 1

Example from c++:

#include <iostream>
namespace foo {
    int x = 0;
}

using namespace foo;

int x = 2;

int main()
{
    std::cout<<::x<<std::endl;
    return 0;
}

C++ allows the second assignment to x, it just forces you to specify which you mean when using it. Picking a disambiguating rule seems to be a reasonable alternative to requiring annotation of each use, especially for a language like julia.

@StefanKarpinski
Copy link
Member

Examples of the proposed usage:

module Foo
using Base
using Other
import Bar.baz
import Bam.qux

...

This would cause baz and qux to be shared bindings with Bar and Bam, regardless of what happens in Foo. Globals that are assigned to in Foo belong to Foo whereas other globals are looked for in Other first then Base. This arrangement makes the choice of name export for what is visible via using a bit suspect, but that can be tweaked.

@JeffBezanson
Copy link
Member

Branch merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

4 participants