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

Tracking issue for RFC #2166: impl-only-use #48216

Closed
3 tasks done
aturon opened this issue Feb 14, 2018 · 22 comments · Fixed by #56303
Closed
3 tasks done

Tracking issue for RFC #2166: impl-only-use #48216

aturon opened this issue Feb 14, 2018 · 22 comments · Fixed by #56303
Assignees
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Feb 14, 2018

This is a tracking issue for the RFC "impl-only-use " (rust-lang/rfcs#2166).

Steps:

@aturon aturon added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Feb 14, 2018
@petrochenkov
Copy link
Contributor

I'm going to implement it this weekend.

@nikomatsakis
Copy link
Contributor

I was thinking about this RFC recently. I wonder if we should enable users of existing preludes to take advantage of this feature. For example, for back-compat reasons, the Rayon prelude cannot change to export _, but I wonder if on the other side we could support some notation like use rayon::prelude::* as _ -- or is that too weird?

(Note that -- with the feature as currently implemented at least -- preludes can have pub use Trait as _; and that will get picked up by a glob import.)

kennytm added a commit to kennytm/rust that referenced this issue Mar 15, 2018
@petrochenkov
Copy link
Contributor

One deficiency of the current implementation is that _ in use Trait as _ is not hygienic.
Normal trait imports generated by macros produce the next error that doesn't happen with _.

#![feature(decl_macro)]
#![allow(unused)]

struct S;

mod m {
    pub trait Tr {
        fn f(&self) {}
    }
    impl Tr for ::S {}
}

pub macro m() {
    use m::Tr;
}

m!();

fn main() {
    S.f(); // ERROR no method named `f` found for type `S` in the current scope
}

This is going to be fixed by #48842 or equivalent.

@nikomatsakis
Copy link
Contributor

@petrochenkov

You are saying that if we alter the macro in your example to this:

pub macro m() {
    use m::Tr as _;
}

then the code will compile, right? (And that is the problem?)

In general, it seems sort of unclear whether one would expect this to work, but I guess consistency is good.

@nikomatsakis
Copy link
Contributor

This is going to be fixed by #48842 or equivalent.

Why would that fix the problem?

@petrochenkov
Copy link
Contributor

@nikomatsakis

You are saying that if we alter the macro in your example to this ... then the code will compile, right?

Yes.

Why would that fix the problem?

Due to attaching hygiene context to _ (#48842 (comment)).

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 16, 2018

Actually, everything is more interesting - this bug is hidden by another bug!
Currently there's no way to use a trait in macro so that its methods are in scope after expansion of that macro.
Nothing of this works:

pub macro outer_use1($Rename: ident) {
    use m::Tr as $Rename;
}
pub macro outer_use2($Tr: ident) {
    use m::$Tr;
}
pub macro outer_use3($Tr: path) {
    pub use $Tr;
}

outer_use1!(Rename);
outer_use2!(Tr);
outer_use3!(m::Tr);

 // still a "no method named `f`" error after expanding all the macros above
S.f();

bors added a commit that referenced this issue Mar 17, 2018
syntax: Make `_` a reserved identifier

Why:
- Lexically `_` is an identifier.
- Internally it makes implementation of `use Trait as _;` (#48216) and some other things cleaner.
- We prevent the externally observable effect of `_` being accepted by macros expecting `ident` by treating `_` specially in the `ident` matcher:
```rust
macro_rules! m {
    ($i: ident) => { let $i = 10; }
}

m!(_); // Still an error
```
@joshtriplett
Copy link
Member

I just ran into an interesting corner case: if I write use somecrate as _;, that produces an unused_imports warning:

warning: unused import: `somecrate as _`

That seems wrong; use somecrate as _ should pull in somecrate even if the module doesn't use any of its symbols, such as for the purposes of linking, allocators, etc.

@eddyb
Copy link
Member

eddyb commented Aug 18, 2018

That bug is not limited to as _: if you write use std::io as _io; on stable, it warns.

@joshtriplett
Copy link
Member

Working on fixing this.

@joshtriplett
Copy link
Member

joshtriplett commented Aug 18, 2018

#53479 should fix this for extern crate foo as _; or extern crate foo as _bar;.

@Nemo157
Copy link
Member

Nemo157 commented Aug 19, 2018

For searchability: this is the unstable feature underscore_imports

@eddyb
Copy link
Member

eddyb commented Aug 19, 2018

Nominating for discussion of Rust 2018-only stabilization at the next @rust-lang/lang meeting.

@nikomatsakis
Copy link
Contributor

@eddyb I would suggest preparing a "stabilization report" that contains summary of what behavior is to be stabilized, what corner cases were uncovered, what changed with respect to the RFC, etc. Given that you wrote "Rust 2018-only" that seems to imply to me that there have been some corner cases or other things discovered? Or some interaction with the module system changes?

@nikomatsakis
Copy link
Contributor

It seems like the behavior is the same when you use macro_rules!, presumably because of hygiene. e.g., this works either with or without _:

#![feature(decl_macro)]
#![feature(underscore_imports)]
#![allow(unused)]

struct S;

mod m {
    pub trait Tr {
        fn f(&self) {}
    }
    impl Tr for ::S {}
}

macro_rules! m {
  () => {
    use m::Tr as _;
  }
}

m!();

fn main() {
    S.f(); // ERROR no method named `f` found for type `S` in the current scope
}

decl macros are not stable, so I guess the hygiene interactions there don't matter too much.

@eddyb
Copy link
Member

eddyb commented Aug 23, 2018

@nikomatsakis Sadly I am not as informed as @petrochenkov on this feature.
But #48216 (comment) appears to be fixed on nightly.

I don't think we need to stabilize yet, but allowing use crate_name as _; in the new edition seems necessary in some situations (e.g. when you want to link against a crate but not use items from it).

However, if we make use foo as _foo; not warn when foo is a crate, then that's fine too.

@Centril
Copy link
Contributor

Centril commented Aug 23, 2018

If possible, I'd like for this feature to be included in Rust 2015 as well.
Are there any blockers / problems with doing that?
The hygiene problem doesn't seem to actually matter per #48216 (comment).

@petrochenkov
Copy link
Contributor

I'm not aware of any issues preventing stabilization of this on Rust 2015.
as _ and all the other module/import stuff reformed in 2018 are pretty much orthogonal.

The hygiene issues appear only if we have both call-site and def-site hygiene, so they affect only nightly.

@Centril
Copy link
Contributor

Centril commented Sep 15, 2018

@petrochenkov Since you are most informed about this, could you prepare a stabilization report for this for both Rust 2015 and Rust 2018?

@Centril
Copy link
Contributor

Centril commented Nov 11, 2018

@petrochenkov any progress?

@petrochenkov petrochenkov self-assigned this Nov 11, 2018
@petrochenkov
Copy link
Contributor

It's in the queue, but with lower priority than uniform paths.

@petrochenkov
Copy link
Contributor

Stabilization report + stabilization PR - #56303.

bors added a commit that referenced this issue Dec 17, 2018
Stabilize `underscore_imports`

Closes #48216
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants