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

Feature gate import shadowing #116

Merged
merged 2 commits into from
Aug 12, 2014
Merged

Conversation

Kimundi
Copy link
Member

@Kimundi Kimundi commented Jun 12, 2014

@pnkfelix
Copy link
Member

(Update circa 15 July 2014: This comment was responding to an earlier version of the RFC which was less clear and thus seemed broader in scope. I'm leaving the comment in place so that one can understand the thread of discussion.)


So, just to be clear: This would stop me from choosing certain names in a pattern binding (such as a fn parameter name) just because the same name happens to be pulled in from a use declaration (perhaps via a glob), is that right?


One important case to make sure continues to work (and can work, even with this proposal in place; we just need to make sure it does work): if a macro introduces an item, it should not be restricted in the names it chooses for its hygienically injected bindings. For example, consider the following:

#![feature(macro_rules)]

macro_rules! three_maker {
    ( $name:ident ) => {
        pub fn $name() -> int {
            let x = 3;
            x
        }
    }
}

fn main() {
    mod foo {
        pub fn x() -> int { 4 }
        three_maker!(y)
    }

    println!("foo::x(): {}", foo::x());
    println!("foo::y(): {}", foo::y());
}

What I want to avoid is that the proposed rule be over-zealously interpreted as meaning that the let x = ... within three_maker becomes illegal due to three_maker! being expanded within mod foo, where (a different) x is bound to a fn item there.

The reason you need to make sure this case continues to work, i.e. the reason it is important that such psuedo-shadowing is allowed, is that if you disallowed such pseudo-shadowing, then macros are no longer proper abstractions, since any use of them becomes dependent upon all of their internally bound names being available.


(Overall, as a Schemer, I prefer flexible shadowing rules rather than trying to restrict programmers from shadowing identifiers. But I think I do understand where people come from when they ask for things like this.)

@Kimundi
Copy link
Member Author

Kimundi commented Jun 18, 2014

@pnkfelix I'm not sure what you mean with your first point about pattern bindings.

@pnkfelix
Copy link
Member

@Kimundi I just said "pattern binding" to mean binding in general, i.e. via the declarations of a fn signature, or a let item, or a match clause.

Its one thing if this RFC Is restricted to outlawing certain combinations of use, while allowing arbitrary shadowing in the cases mentioned above. But as I understand it, the RFC in its current state is imposing limitations on all kinds of bindings, not just use.

@Kimundi
Copy link
Member Author

Kimundi commented Jun 21, 2014

Ah, I see you think I'm proposing forbidding shadowing between any declaration in the same module.

Thats not what I meant: I only propose forbidding shadowing of items (including view items) in the same flat scope.

This would be forbidden:

use foo::Bar;
struct Bar;

But this would be allowed:

use foo::Bar;
fn x() {
     struct Bar;
}

Thats what I mean with flat scope: There is the top level of the module, and then anonymous modules like function bodies that inherit all declarations from their parent, while also being able to shadow them themself, and currently extern crate, use and items shadow each other inside each of them.


So, bindings in let or match wouldn't have a problem because they are in a new scope, and thus are allowed to shadow. And you should also still be able to write let x = ...; let x = ...; because this proposal does not make a statements about shadowing of values in a function body.

Same situation with the macro: The let x; is in a new scope, and thus wouldn't collide with the function x. (Also, I think hygiene would prevent this anyway)

@pnkfelix
Copy link
Member

Ah! That is indeed different than what I thought you were proposing.

Okay. Maybe revise RFC to make that distinction explicit.

@glaebhoerl
Copy link
Contributor

+100, thank you for writing the proposal I was going to write.

I'll try to recap how I understand this proposal, and how I think it should be, and I hope that these things coincide.

Definitions

Scope

A scope is basically the space between braces, with inner braces starting a new scope. So:

scope 1
{
    scope 2
    {
        scope 3
    }
    scope 2
}
scope 1
{
    scope 4
}
scope 1

Namespace

A separate namespace exists for each type of thing for which you can use the same identifier without conflict. Rust has separate namespaces for type-level things (types, traits), lifetimes, value-level things (variables, statics, functions, enum variants), modules, and macros.

Item declaration

Item declarations are basically those things which it is legal to declare outside of function bodies, so not let or other local bindings. Examples include extern crate, use, struct, fn, static, mod, etc.

The rules

  • Item declarations in the same scope and namespace may not shadow each other.
  • Item declarations may shadow declarations from outer scopes (in the same namespace).

Motivation

Item declarations in a given scope are logically unordered, so it makes no sense for them to shadow each other.

Allowing shadowing of declarations from outer scopes improves modularity, because it means that if additional declarations are introduced to an outer scope, the meaning of inner scopes is unaffected.

@Kimundi
Copy link
Member Author

Kimundi commented Jun 25, 2014

Thanks for writing that up, your "rules" section is pretty much what I'm trying to propose here :)

EDIT:
As clarification, in this context "items declarations" refers to both definitions and view items intentionally.

@glaebhoerl
Copy link
Contributor

Ah, the term "view item" was unfamiliar to me. But yes, by "item declarations" I meant both view items and definitions.

@ben0x539
Copy link

This proposal seems reasonable to agree on. I'd be surprised if it even negatively affects existing code and I think it has the potential to make item visibilities less surprising, and if it turns out untenable we can relent at any time post-1.0..

@pnkfelix
Copy link
Member

@Kimundi can you revise the RFC to try to incorporate some of the feedback from the discussion here?

@Kimundi
Copy link
Member Author

Kimundi commented Jun 25, 2014

@pnkfelix Yeah, but only around starting the weekend.

@Kimundi
Copy link
Member Author

Kimundi commented Jun 30, 2014

@pnkfelix I updated the RFC with more information, I hope its now clearer what it is about.

@pcwalton
Copy link
Contributor

+1, I'm all about making resolve simpler.

@glaebhoerl
Copy link
Contributor

W.r.t. unresolved questions:


Overlapping imports

Right now its legal to write this:

fn main() {
        use Bar = std::result::Result;
        use Bar = std::option::Option;
        let x: Bar<uint> = None;
}

where the latter use shadows the former. This would have to be forbidden as well,
however the current semantic seems like a accident anyway.

I think forbidding this should be explicitly part of the RFC as well.


Prelude

  • It is unclear how the libstd preludes fits into this.

    On the one hand, it basically acts like a hidden use std::prelude::*; import
    which ignores the globs feature, so it could simply also ignore the
    import_shadowing feature as well, and the rule becomes that the prelude is a magic
    compiler feature that injects imports into every module but doesn't prevent the user
    from taking the same names.

    On the other hand, it is also thinkable to simply forbid shadowing of prelude items as well,
    as defining things with the same name as std exports is not recommended anyway, and this would nicely enforce that. It would however mean that the prelude can not change without breaking backwards compatibility, which might be too restricting.

FWIW, what Haskell does here is that:

  • The Prelude is implicitly imported by default.

  • In all other respects it's non-special, i.e. name clashes with things imported from the Prelude are illegal, the same as with any other module.

  • But you can also import the Prelude explicitly: import Prelude ..., in which case the implicit import from the first point doesn't happen, and the only the explicit import is used.

  • If you want to declare things with the same name as Prelude things, then you do an explicit Prelude import and either hide those things, import only the things you need, or just import it qualified:

    -- no Rust equivalent
    import Prelude hiding (Int)
    data Int = ...
    
    -- corresponds to `use std::prelude::{Show, Option};`
    import Prelude (Show, Maybe)
    data Int = ...
    
    -- corresponds to `use std::prelude;`
    import qualified Prelude
    data Int = ...
    

Rust doesn't have an equivalent for hiding, but even so, given that this is an uncommon use case, requiring one of the latter two to be done seems quite reasonable.


Prelude and backwards compatibility

However, the above doesn't solve the problem that adding names to the prelude could break user code, and would therefore be backwards incompatible. I can think of several different ways this could potentially be solved. In roughly increasing order of generality:

  1. As floated in the RFC, treat the implicit prelude imports specially, and make them shadowable, unlike other imports.
  2. Add a variant on use which allows the imported names to be shadowed, e.g. shadowable use foo::bar; (only with better syntax). Declare that the implicit prelude import is a shadowable import. (This is similar to the prelude use idea floated in the RFC.)
  3. Make all private glob imports shadowable. (But emit a warning.)
  4. Make all private imports shadowable. (But emit a warning.)
  5. Make all imports (view items) shadowable by non-view items, as currently. (But emit a warning.)

My recommendation: Do none of these for now.

There's no urgent need. Make shadowing things imported from the prelude, whether implicitly or explicitly, illegal, period, as with any other module. Revisit the question if and when, after 1.0, we actually want to add things to the prelude. We can then debate the available options and potentially adopt one of them at that point.

@nikomatsakis
Copy link
Contributor

Somewhat related, I've specified a formalization (and prototype) of the name resolution rules I would eventually like to see Rust use: https://github.com/nikomatsakis/rust-name-resolution-algorithm It would be nice to make changes to resolve to make it forwards compatible with these rules. (That said, I do plan to amend those rules to take macros into account, and that has not been done.)

@nikomatsakis
Copy link
Contributor

Is there any good reason to have a feature gate at all? I think I would prefer to simply remove it.

@nikomatsakis
Copy link
Contributor

In meeting, we clarified that feature gate is just a means to do a more graceful deprecation, which makes sense.

@brson brson merged commit e5f3fb1 into rust-lang:master Aug 12, 2014
@brson
Copy link
Contributor

brson commented Aug 12, 2014

Discussed at https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-08-12.md.

Accepted as RFC 50.

Tracking bug: rust-lang/rust#16464

The feature gating is just going to be for the purposes of deprecation, with the intent to remove the feature completely in the future.

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
@Centril Centril added the A-resolve Proposals relating to name resolution. label Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Proposals relating to name resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants