RFC: warn and don't import when two usings provide the same name

[JeffBezanson]

fixes #4345

Does the following:

julia> module A
       export foo
       foo() = 1
       end

julia> module B
       export foo
       foo() = 2
       end

julia> using A

julia> using B

julia> foo()
Warning: both B and A export foo; uses of it in module Main must be qualified
ERROR: UndefVarError: foo not defined

importall is a bit of a different story. It's hard to provide the same behavior there, since it's an imperative: the names are imported right when the importall statement runs. Fortunately it's used far less often than using, and we already give a warning for conflicts in that case.

Will update news and docs if this is to be merged.

[StefanKarpinski]

Awesome.

[timholy]

I'm a fan.

[Keno]

What happens with:

using A
foo()
using B
foo()

?

[JeffBezanson]

You get Warning: using B.foo in module Main conflicts with an existing identifier. after using B, and foo() continues to call A.foo.

[toivoh]

+1

[StefanKarpinski]

I'm all for just merging this.

[kmsquire]

+1

On Thursday, April 23, 2015, Stefan Karpinski [email protected]
wrote:

I'm all for just merging this.

—
Reply to this email directly or view it on GitHub
#10951 (comment).

[ivarne]

Great!

Remember the NEWS.md and docs.

[dcjones]

While I think this is a good idea, it breaks Gadfly since it, as well as Base, export something called Stat. Base.Stat is deprecated though. Would it be reasonable to remove it now?

[JeffBezanson]

Yes I think we can remove that. We'll probably remove all the 0.3 deprecations pretty soon.

[IainNZ]

Interesting examples of breakages:
http://pkg.julialang.org/?pkg=Digits&ver=nightly

Warning: both Digits and Base export contains; uses of it in module Main must be qualified
ERROR: LoadError: test error in expression: contains(n,3456) == true
UndefVarError: contains not defined
 in anonymous at test.jl:87
 in do_test at test.jl:47
 in include at ./boot.jl:250
 in include_from_node1 at loading.jl:129
 in process_options at ./client.jl:308
 in _start at ./client.jl:407
while loading /home/vagrant/testpkg/v0.4/Digits/test/runtests.jl, in expression starting on line 36

http://pkg.julialang.org/?pkg=Calculus&ver=nightly

>>> test log
Warning: both Calculus and Base export gradient; uses of it in module Main must be qualified
ERROR: LoadError: LoadError: UndefVarError: gradient not defined
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in anonymous at no file:19
 in include at ./boot.jl:250
 in include_from_node1 at loading.jl:129
 in process_options at ./client.jl:308
 in _start at ./client.jl:407

[IainNZ]

More worryingly:

>>> 'using HttpServer' log
Warning: both Docile and Base export @doc; uses of it in module HttpServer must be qualified
WARNING: Dict-based `@docstring` config is deprecated. Use keywords instead.
ERROR: LoadError: LoadError: UndefVarError: @doc not defined
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in reload_path at ./loading.jl:153
 in _require at ./loading.jl:68
 in require at ./loading.jl:51
 in include at ./boot.jl:250
 in include_from_node1 at loading.jl:129
 in process_options at ./client.jl:308
 in _start at ./client.jl:407

[pao]

Looking at the examples posted from PkgEvaluator, some quotation marks here might aid readability?

"Warning: both `%s` and `%s` export `%s`; ..."

The ones for the modules are less needed, but setting off the method name seems like it would make this a tad easier to read.

[IainNZ]

Yeah that'd be a nice little change

[pao]

So apparently that option to "create a pull request" in the GitHub editor doesn't actually work. Nice. ec99e37

[JeffBezanson]

Oh wow I had no idea.

[StefanKarpinski]

So what? It just makes the change instead of creating a PR? That's broken and dangerous.

[JeffBezanson]

Yeah that's an incredibly bad bug.

[pao]

I intend to reproduce it + file a report with them when I get home, if no one beats me to it.

[pao]

And failed to reproduce. Blargh. Sorry, either I misclicked or it's specific to the Firefox (or profile, or cookies, or...) at the office.

[vtjnash]

So apparently that option to "create a pull request" in the GitHub editor doesn't actually work. Nice. ec99e37

if you have direct rights to a repo, it'll let you edit any file directly in the browser. it can be very useful for doc and README changes, but generally not so good for code edits

[pao]

I'm aware; I've fixed doc typos that way before.

GitHub appears to have recently added the ability to make a branch & pull request instead, which is what I tried to do precisely because it was a code change. It's a radio button option that appears in the edit window below the commit message box. It worked fine the second time I tried it, so I'm not sure what happened the first time.

[simonster]

This is a bit annoying:

julia> using Base.Libdl

julia> dlopen
Warning: both `Libdl` and `Base` export `dlopen`; uses of it in module `Main` must be qualified
ERROR: UndefVarError: dlopen not defined

(The dlopen function exported by Base is deprecated.)

[tkelman]

from JuliaPackaging/BinDeps.jl#143 (comment)

Should the warning maybe not happen when all imports refer to the same object? I.e. When they are === to each other?

[JeffBezanson]

Unless I messed up, I believe I already implemented that. However Base.dlopen !== Libdl.dlopen. They're separate functions. One prints a warning, the other does dlopen. We'll have to find a solution to this. Maybe instead of a function deprecation, we can add the warning to UndefVarError.