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

Multiple globs are basically unusable #553

Closed
SiegeLord opened this issue Jan 4, 2015 · 12 comments
Closed

Multiple globs are basically unusable #553

SiegeLord opened this issue Jan 4, 2015 · 12 comments

Comments

@SiegeLord
Copy link

Copy of rust-lang/rust#20461.

A recent change broke this code:

use a::*;
use b::*;

mod a { pub fn foo() {} }
mod b { pub fn foo() {} }

This gives an error:

test.rs:4:5: 4:10 error: a value named `foo` has already been imported in this module
test.rs:4 use b::*;

This makes globs a lot less useful than they were before. If globs are going to be only useful in isolation, why have them as a feature at all?

@nrc
Copy link
Member

nrc commented Jan 4, 2015

The motivation for the change is avoiding shadowing. That was discussed in RFC 116.

Globs are still very useful, just as long as there are no overlapping public names, in which case the user should import individual items and not use globs (since there is no explicit way to specify which name to use).

So, how can we make this better? I see a few solutions:

  • re-allow shadowing (I think this is a terrible idea)
  • allow some way to exclude imports from a glob (I believe this is the Haskell approach), e.g. (strawman syntax, obvs):
use a::*;
use b::* hiding foo;
  • Allow limited shadowing in some form, pulling an idea out of thin air which I think is terrible, names in ** override names in *, so you use the number of stars to give a priority to overriding. Shadowing between globs with the same number of stars is forbidden
  • allow renaming in globs, e.g., use a::* with foo as a_foo;
  • do nothing and rely on tool support for making it easier to work with large lists of imports (specific ideas welcome here in any case).

Any more ideas?

@sfackler
Copy link
Member

sfackler commented Jan 4, 2015

How about only complaining if you try to use a name that shows up in 2 glob
imports? If you use a name that comes unambiguously from one of the imports
everything will work fine.

On Sun, Jan 4, 2015, 12:17 PM Nick Cameron notifications@github.com wrote:

The motivation for the change is avoiding shadowing. That was discussed in RFC
116
https://github.com/rust-lang/rfcs/blob/master/text/0116-no-module-shadowing.md
.

Globs are still very useful, just as long as there are no overlapping
public names, in which case the user should import individual items and not
use globs (since there is no explicit way to specify which name to use).

So, how can we make this better? I see a few solutions:

  • re-allow shadowing (I think this is a terrible idea)
  • allow some way to exclude imports from a glob (I believe this is the
    Haskell approach), e.g. (strawman syntax, obvs):

use a::;
use b::
-foo;

  • Allow limited shadowing in some form, pulling an idea out of thin
    air which I think is terrible, names in ** override names in *, so you
    use the number of stars to give a priority to overriding. Shadowing between
    globs with the same number of stars is forbidden
  • allow renaming in globs, e.g., use a::* with foo as a_foo;
  • do nothing and rely on tool support for making it easier to work
    with large lists of imports (specific ideas welcome here in any case).

Any more ideas?


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

@nrc
Copy link
Member

nrc commented Jan 4, 2015

@sfackler I don't think that helps this situation, because both imports are globs. We did have the 'explicit overrides glob' rule for a while, @eddyb got rid of it (and I think that is correct wrt the RFC). I'm not sure about the pros and cons of that approach.

@nrc
Copy link
Member

nrc commented Jan 4, 2015

Another way we could admit more programs is only to error if two globs import the same name and that name is actually used. I.e., double import is OK, but using a double import is an error. That would certainly be more flexible. It would introduce some kind of weird errors when you start using a double-imported name though, especially around traits, which are often not explicitly named.

@nrc
Copy link
Member

nrc commented Jan 4, 2015

On a second reading, I think @sfackler is suggesting exactly what I was above, not making a distinction between single and glob imports. I guess that is a hazard of use meaning import not actually use.

@glaebhoerl
Copy link
Contributor

see also cc @nikomatsakis

@Kimundi
Copy link
Member

Kimundi commented Jan 5, 2015

I like @nikomatsakis WIP proposal, which basically merges all glob imported items, and only produces conflicting import error message for the concrete item (like @sfackler and @nrc propose). It also allows glob imports to be shadowed by other items including specific imports, which makes them more resilient against upstream changes, and allows to resolve conflicts by writing out a concrete import.

use a::*; // a exports foo::x, foo::y, foo::z, foo::v
use b::*; // b exports foo::x, bar::y, bar::z, bar::v

let _ = x; // Valid, because both globs import the same logical item

struct z;// Valid, and doesn't conflict with glob imported items because it shadows them
let _ = z; 

let _ = y; // error: ambiguous name, can only be reached through more than one glob import
           // help: use a::y; or use b::y; to resolve.

use a::v;
let _ = v; // Valid, picks a specific import by shadowing the glob ones with the concrete import above.

In other words, glob imports would never conflict with any other item directly, and only act as a fallback for the case that no concrete item or import in a module match.

@paulp
Copy link

paulp commented Jun 18, 2015

Please don't ignore previous work. In scala you can see exactly how each idea being proposed here has played out over roughly a decade and billions of lines of code.

Scala Issue Database
Relevant Scala Spec

I am not suggesting you imitate scala. But given the existence of this huge body of data, it would be professional negligence to design the same feature in a vacuum as if this has never come up before. If those sound like strong words, it's because the same is true of numerous other rust language features, and each looks to have been designed without analyzing what has and hasn't worked in scala.

It is said that rust is "stable" now, for some definition thereof. Rust's history reveals an awful lot of figuring out what doesn't work by shipping and seeing what happens. Probably sometimes it has to be like that, but not generally. I don't think it's a luxury you have anymore.

@steveklabnik
Copy link
Member

I don't have any particular opinions yet about this issue, and I do agree that we should pay close attention to previous work. Thanks for the links, @paulp .

I don't think it's a luxury you have anymore.

This is the idea with feature gates: we can actually add new stuff, and give it a try, and it won't affect stable users. So we do kind of have this option, in certain cases. New glob behavior might not be one of them, but in the general case, this option is very much still open.

@paulp
Copy link

paulp commented Jun 18, 2015

As I think I've seen acknowledged elsewhere, N gated features effectively partition the language into 2^N dialects. In a narrow sense you can preserve stability of individual dialects by raising the value of N, but there is previous work here as well, and from it we can see it's easy to win the battle and lose the war.

@bruno-medeiros
Copy link

use a::*; // a exports foo::x, foo::y, foo::z, foo::v
use b::*; // b exports foo::x, bar::y, bar::z, bar::v

let _ = x; // Valid, because both globs import the same logical item

struct z;// Valid, and doesn't conflict with glob imported items because it shadows them
let _ = z; 

let _ = y; // error: ambiguous name, can only be reached through more than one glob import
           // help: use a::y; or use b::y; to resolve.

+1 for the changes above. Also note that this problem is not limited to glob imports, but can happen even with just single imports - (see https://internals.rust-lang.org/t/annoying-limitations-with-use-of-same-symbols/4310?u=bruno_medeiros for a simplified version of a case I've hit in real code)

use a::v;
let _ = v; // Valid, picks a specific import by shadowing the glob ones with the concrete import above.

Hum.... I'm not so sure about this one. I think it might be better if glob imports were semantically the same as individually importing each specific element - making the above code still invalid. It might be confusing otherwise, or have unintended consequences. I think a use a::v; should only shadow a glob import if the use a::v; is in an inner scope than the glob import. But not if they are on same scope.

@petrochenkov
Copy link
Contributor

Resolved by #1560, implemented and stabilized.

The example compiles on stable 1.19 (and couple of previous releases).

#![allow(unused)]

use a::*; // OK
use b::*; // OK

mod a { pub fn foo() {} }
mod b { pub fn foo() {} }

fn main() {}

The error is reported only when the ambiguous foo is used

fn main() {
    foo(); //~ ERROR `foo` is ambiguous
}

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

No branches or pull requests

9 participants