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

Implement UFCS (Tracking RFC 132) #16293

Closed
huonw opened this issue Aug 6, 2014 · 20 comments
Closed

Implement UFCS (Tracking RFC 132) #16293

huonw opened this issue Aug 6, 2014 · 20 comments
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Aug 6, 2014

https://github.com/rust-lang/rfcs/blob/master/text/0132-ufcs.md

@huonw huonw changed the title UFCS (Tracking RFC 46) Implement UFCS (Tracking RFC 46) Aug 6, 2014
@pnkfelix
Copy link
Member

pnkfelix commented Aug 7, 2014

This is (expected to be a) backwards-compatible change, so while we do desire it, it is not a strict necessity for 1.0

Assigning P-high, not 1.0 milestone.

@nrc
Copy link
Member

nrc commented Oct 12, 2014

Question: in a trait or impl, should it be possible to call foo(x) where foo is a method in scope? We don't allow calling foo() (i.e., with an implicit self), but we do allow calling bar where bar is a function in scope. It seems to me like we should allow it since it is weird to need qualification of an item in an inner scope but not an outer one. OTOH, it means we would get worse error messages where a programmer has omitted an explicit self, since it would just be a "not enough args" error, rather than something more specific.

nrc added a commit to nrc/rust that referenced this issue Oct 15, 2014
bors added a commit that referenced this issue Oct 15, 2014
With the 'receiver' as an argument and static dispatch. Part of UFCS implementation (#16293).

r?
@pnkfelix pnkfelix changed the title Implement UFCS (Tracking RFC 46) Implement UFCS (Tracking RFC 132) Nov 13, 2014
@alexchandel
Copy link

Issue #8888 is still referenced by the Float trait, which contains unused_self parameters.

bors added a commit that referenced this issue Jan 15, 2015
Working towards #16293, this adds support for `<T as Trait>::method` in expressions.
@eddyb
Copy link
Member

eddyb commented Jan 23, 2015

I've found a few consequences of the RFC that I haven't seen mentioned before, while trying to design a full implementation:
Resolve wouldn't need to track impls at all. This means UFCS reverts the need to force impls to be in the same module with their type.

struct Foo<T>;
mod bar {
    impl super::Foo<()> { // impl allowed outside of Foo's module.
        fn bar(&self) -> Option<()> { None }
    }
}

impl Foo<bool> {
    // same method name, allowed only because Self cannot overlap
    // with any other impl with the same method (as with trait impls).
    fn bar(&self) -> Option<bool> { Some(true) }
}

// anonymous impls would be checked by coherence as if they were
// implementing traits from outside the current crate.
impl<T> [Foo<T>] {
    fn extra(&self) -> usize { 0 }
}
impl<T> Vec<Foo<T>> {
    fn extra(&self) -> usize { self.capacity() - self.len() }
}

All of these combined will end up removing some extension traits, and make type aliases much more powerful, without special-casing them. This will be possible, given we complete defaulted type params:

// std::collections (the collections crate wouldn't have a clue about RNGs):
type HashMap<K, V, H = RandomizedSipHash> = collections_crate::HashMap<K, V, H>;

On the other hand, i32::foo would only mean <i32>::foo, so you couldn't use modules like std::i32 at all. For this, I propose a relatively simple solution:

// in libcore:
#[lang="i32"]
enum i32 {} // unihabited enum introduces a type in scope and nothing else

// coherence would allow this impl as the type is "rooted" in the crate
// defining the lang item.
impl i32 {
    fn foo(self) -> ... {...}
}

That would also make the rustdoc special handling for primitives a bit simpler.

Then again, that last one isn't necessary. I just checked and the only items in std::i32 are a few constants which would be better off in the Int trait, once we get associated constants.
I would still like having all those primitives in the prelude, but I guess the situation isn't as dire.

EDIT: std::str has 3 free functions (and a bunch of types/traits), maybe it's a better example of a primitive type with an "associated" module.

@eddyb
Copy link
Member

eddyb commented Feb 15, 2015

Nevermind the label fiddling above, I thought I had a case of backward incompatibility, but it's nothing more than bugs.

bors added a commit that referenced this issue Feb 24, 2015
Adds `<module::Type>::method` support and makes `module::Type::method` a shorthand for it.
This is most of #16293, except that chaining multiple associated types is not yet supported.
It also fixes #22563 as `impl`s are no longer treated as modules in resolve.

Unfortunately, this is still a *[breaking-change]*:
* If you used a global path to a primitive type, i.e. `::bool`, `::i32` etc. - that was a bug I had to fix.
Solution: remove the leading `::`.
* If you passed explicit `impl`-side type parameters to an inherent method, e.g.:
```rust
struct Foo<T>(T);
impl<A, B> Foo<(A, B)> {
    fn pair(a: A, b: B) -> Foo<(A, B)> { Foo((a, b)) }
}
Foo::<A, B>::pair(a, b)
// Now that is sugar for:
<Foo<A, B>>::pair(a, b)
// Which isn't valid because `Foo` has only one type parameter.
// Solution: replace with:
Foo::<(A, B)>::pair(a, b)
// And, if possible, remove the explicit type param entirely:
Foo::pair(a, b)
```
* If you used the `QPath`-related `AstBuilder` methods @hugwijst added in #21943.
The methods still exist, but `QPath` was replaced by `QSelf`, with the actual path stored separately.
Solution: unpack the pair returned by `cx.qpath` to get the two arguments for `cx.expr_qpath`.
@eternaleye
Copy link
Contributor

Quite a while back, #12566 (self argument destructuring) was closed because it'd be part of UFCS - but the closure referenced #11938 which was closed as a dup of this issue, and it seems to have gotten lost in the shuffle. Any updates?

@nrc nrc removed their assignment Apr 8, 2015
@bombless
Copy link
Contributor

bombless commented Apr 8, 2015

Seems that self-argument destructing is not implemented on UFCS.

struct A(uint);

impl A {
     fn b(self: &A(ref mut u), i: uint) { //~ error: expected identifier, found keyword `ref`
//~^ error: expected one of `(`, `+`, `,`, `::`, or `<`, found `mut`
        *u = i;
    }
}

(https://github.com/rust-lang/rust/blob/0fa17008e76c0d741ef6a3f391bad6d79eacaff4/src/test/run-pass/self-pat.rs)

@nrc
Copy link
Member

nrc commented Apr 23, 2015

Hmm, what still needs to be done here? Anything other than self argument destructuring (see the above two comments).

@ihrwein
Copy link
Contributor

ihrwein commented May 30, 2015

I stumbled into destructoring self today. It would be nice to see it working.

I hit an other problem, it works fine.

@eternaleye
Copy link
Contributor

@ihrwein, It still does not work in my testing:

struct Foo(i32, i64);

impl Foo {
    fn classic(&self) {
        println!("{}, {}", self.0, self.1);
    }

    fn destructuring(&Foo(u, v): &Self) {
        println!("{}, {}", u, v);
    }
}

fn main() {
    let x = Foo(3, 14);
    x.classic(); // Works, but doesn't destructure
    Foo::destructuring(&x); // Works, but x is not an invocant
    x.destructuring(); // Explodes
}
<anon>:17:7: 17:22 error: no method named `destructuring` found for type `Foo` in the current scope
<anon>:17     x.destructuring(); // Explodes
                ^~~~~~~~~~~~~~~
<anon>:17:7: 17:22 note: found defined static methods, maybe a `self` is missing?
<anon>:8:5: 10:6 note: candidate #1 is defined in an impl for the type `Foo`
<anon>:8     fn destructuring(&Foo(u, v): &Self) {
<anon>:9         println!("{}, {}", u, v);
<anon>:10     }
error: aborting due to previous error
playpen: application terminated with error code 101

i.e., it supports destructuring Self in the most literal interpretation of the phrase, but it most certainly does not support destructuring the invocant.

@ihrwein
Copy link
Contributor

ihrwein commented Jul 13, 2015

@eternaleye try this way:

struct Foo(i32, i64);

impl Foo {
    fn classic(&self) {
        println!("{}, {}", self.0, self.1);
    }

    fn destructuring(self) {
        let Foo(u, v) = self;
        println!("{}, {}", u, v);
    }
}

fn main() {
    let x = Foo(3, 14);
    x.classic(); // Works, but doesn't destructure
    Foo::destructuring(x);
}

@eternaleye
Copy link
Contributor

@ihrwein

Those are completely different. My Foo::destructuring() takes self by read-only reference, while yours takes it by value - and of course it can be destructured in the body; by then it's a variable like any other. That's not the point.

The point is that UFCS, as far as I can see, was specified as making it such that defining a first parameter of type [&[mut ]]Self as a trait function would permit method-call syntax. It's not destructuring in the argument list that's special either - for anything other than Self it works flawlessly, and even for Self it works when either invoked as a static function or in any parameter but the first.

The problem is that the method-call syntax is being forbidden when it should be allowed - i.e., exactly UFCS.

One use case where this is especially nice, for its pure concision: Implementing state machines where each state is a newtype.

@alexcrichton alexcrichton added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 11, 2015
@alexcrichton
Copy link
Member

I believe this has all long since been implemented, so closing.

@eternaleye
Copy link
Contributor

@alexcrichton: No, my example is still broken (destructuring the invocant): http://is.gd/3i9oCg

@eddyb
Copy link
Member

eddyb commented Feb 18, 2016

@eternaleye The accepted RFC supports nothing of the sort.

@eternaleye
Copy link
Contributor

@eddyb: From the opening paragraph:

(In fact, the distinction between static methods and other methods is completely erased, as per the method lookup of RFC PR #48.)

Thus, defining a function that takes a first argument of type [&[mut ]]Self inside an impl block should permit method call syntax, no? But even without destructuring, it fails: http://is.gd/tZKLR9

@eddyb
Copy link
Member

eddyb commented Feb 18, 2016

@eternaleye It refers to self-ish methods behaving like static methods when accessed via a path.
If you read the full RFC, you'll see that it only talks about paths.

Also, if you look at RFC PR 48, there's some interesting discussion about special-casin the first argument, but nothing other than self, &self, &mut self and ~self (now self: Box<Self>) is in the accepted RFC 48.

EDIT: I had missed "Receiver reconciliation" which was never fully implemented AFAICT.

@eternaleye
Copy link
Contributor

Mm, I suppose I misunderstood then. I'll see about writing up an RFC myself.

EDIT: Er, on reading RFC 48 in the repo, what I'm seeing does not match what you say.

Under this proposal we would keep these shorthands but also permit any function in a trait to be used as a method, so long as the type of the first parameter is either Self or something derefable Self:

and

fn foo(self: &Self, ...) // equivalent to `fn foo(&self, ...)

and

It would not be required that the first parameter be named self, though it seems like it would be useful to permit it. It's also possible we can simply make self not be a keyword (that would be my personal preference, if we can achieve it).

(emphasis mine)

@nikomatsakis
Copy link
Contributor

@eternaleye it's true that we initially intended to not make the name significant, but I think that ship has sailed (and it's worth amending the RFC). For one thing, it would be backwards incompatible to make a change here, as it would introduce potential new ambiguities into method dispatch (though I don't know whether this would actually affect any crates in practice).

But also, I've shifted my opinion as to what I think is best. I think it is actively useful to be able to exclude functions from being used as methods, because it allows you to avoid potential conflicts between traits (any number of traits can have associated fns with the same name, but if method notation forces those names to be in conflict if a single type implements all the traits).

I also think there is just a certain amount of clarity in a rule like "associated fns whose first argument uses the self keyword can be used as methods"; it tells you what the trait author had in mind, and encourages a more uniform style on users of the trait.

It also means that we can impose a rule that says: "if you use the keyword self, the type must be derefable to Self", rather than us deducing whether the type is derefable to Self. Figuring out if something holds is generally much harder than requiring that it does hold, particularly around generic code.

@eternaleye
Copy link
Contributor

Regarding conflict, ISTR an older syntax (from before <T as U>::foo()) that may have just been an idea, but was x.Trait::foo()

And personally, my interest in this is for cleanliness and concision in implementing things like state machines - in particular, being able to destructure the invocant lends itself very nicely to simple, concise implementations of state machines that can be used in a method-chaining notation.

I can certainly see where you're coming from regarding deciding whether the type is derefable to Self, though.

Given the above, perhaps I'll do up an RFC for permitting @-binding of the invocant, ugly as that would be 😞

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. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants