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

where clauses are only elaborated for supertraits, and not other things #20671

Open
aturon opened this issue Jan 7, 2015 · 31 comments
Open

where clauses are only elaborated for supertraits, and not other things #20671

aturon opened this issue Jan 7, 2015 · 31 comments
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Jan 7, 2015

The following example:

trait Foo<T> {
   fn foo(&self) -> &T;
}

trait Bar<A> where A: Foo<Self> {}

fn foobar<A, B: Bar<A>>(a: &A) -> &B {
   a.foo()
}

fails with "error: type &A does not implement any method in scope named foo".

This UFCS variant

trait Foo<T> {
    fn foo(&self) -> &T;
}

trait Bar<A> where A: Foo<Self> {}

fn foobar<A, B: Bar<A>>(a: &A) -> &B {
    Foo::foo(a)
}

fails with "error: the trait Foo<_> is not implemented for the type A".

@aturon
Copy link
Member Author

aturon commented Jan 7, 2015

cc @nikomatsakis @jroesch

Possibly related to #20469 but it is not fixed by where clauses. I'm seeing this on nightly, but was led there from a problem on master.

@aturon
Copy link
Member Author

aturon commented Jan 7, 2015

Note, this blocks the transition from BorrowFrom to Borrow. Not a blocker for alpha, though.

@japaric
Copy link
Member

japaric commented Jan 7, 2015

Sorry, but I don't see why this should work without an explicit A: Foo<B> bound in the foobar function. trait Bar<A> where A: Foo<Self> is the same as trait Bar<A: Foo<Self>>, so this seems consistent with how bounds have always worked. To show an example with bounds on structs:

#![crate_type = "lib"]

// these two are equivalent
struct Rev1<I> where I: DoubleEndedIterator {
    it: I,
}

struct Rev2<I: DoubleEndedIterator> {
    it: I,
}

// these two don't work because `Rev<I>` doesn't imply that `I: DoubleEndedIterator`
fn foo1<I>(mut rev: Rev1<I>) {
    rev.it.next_back();
}

fn foo2<I>(mut rev: Rev2<I>) {
    rev.it.next_back();
}

// even these won't work unless you explicitly add `I: DoubleEndedIterator` as a bound
fn bar1<I>(mut rev: Rev1<I>) {}
fn bar2<I>(mut rev: Rev2<I>) {}

I skimmed over the where clause RFC, and I didn't see anything about propagating bounds just for traits. Am I missing something?

@aturon
Copy link
Member Author

aturon commented Jan 7, 2015

@japaric My assumption that this would work was based partly on conversations with @nikomatsakis after the initial RFC. In particular, we've discussed that trait Foo: Bar should be equivalent to trait Foo where Self: Bar in most respects, and from that one would further expect other bounds to apply. But currently these are not equivalent:

trait Foo<T> {
    fn foo(&self) -> &T;
}

//trait Bar<A> where Self: Foo<A> {}  <--- this produces an error
trait Bar<A>: Foo<A> {}

fn foobar<A, B: Bar<A>>(b: &B) -> &A {
    b.foo()
}

That seems inconsistent and I don't know of any good reason for it. (And of course it's a pain to have to rewrite bounds everywhere when those bounds must already hold.)

I do take your point that in general we do not propagate bounds, but I think we want to head toward more propagation, as in http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/ -- and I thought we'd already started down this road.

@jroesch
Copy link
Member

jroesch commented Jan 7, 2015

@japaric It is also my understanding (from conversations with @nikomatsakis) that this should work. It seems related to the generalization of bounds checking.

One of the annoyances I started purging from the compiler is the ParamBounds struct which is still used to store the super traits bounds declared on a trait definition. This is one of (if not the last) place where we still have special treatment of bounds (modulo my open PR). It is my hunch that the bounds checking code introduces all super trait bounds but doesn't do anything with the ones declared in the where clause.

I think this is an important question to resolve generally because like @aturon it was my understanding that any super trait bound should be identical to bounding the Self type with the same bound. I believe Niko and I had talked about needing to make some more reaching changes to support this, and if I recall correctly it was around the computing of super trait bounds.

In my imagined scheme we should probably collect any Self type constraints and introduce them like we have been with super trait bounds. It seems this would be the simplest strategy, and avoids introducing a breaking change for anyone relying on the current behavior (esp. if this isn't resolved pre-1.0).

@nikomatsakis
Copy link
Contributor

I think this is a grey area :) At present we only "elaborate" supertraits (meaning Self : Foo). This case is not a "supertrait". However, limiting ourselves to elaborating supertraits is fairly arbitrary (and, in fact, easily lifted). I can do an experiment in this area if it will speed along the BorrowFrom to Borrow transition. I agree it's not exactly blocking alpha but I'd love to kill the old_orphan_check and old_impl_check "features" (maybe that's unrealistic).

@japaric
Copy link
Member

japaric commented Jan 7, 2015

@aturon

In particular, we've discussed that trait Foo: Bar should be equivalent to trait Foo where Self: Bar

So I was missing a piece of information.

That seems inconsistent and I don't know of any good reason for it.

I agree with this. But want to point out that this second example is a "supertrait", whereas the first one is not. So having to explicitly add the B: Foo<A> bound in the first case is consistent with today's rules.

I do take your point that in general we do not propagate bounds, but I think we want to head toward more propagation

I'm not opposed to having implied bounds, but I would like it to be implemented in a general way (such that my struct bounds example would also compile), instead of making it more ad hoc ("we only propagate bounds on traits with where-clause bounds that have a Self parameter on either side" or something like that)


@nikomatsakis

I think this is a grey area :) At present we only "elaborate" supertraits (meaning Self : Foo). This case is not a "supertrait".

I agree!

However, limiting ourselves to elaborating supertraits is fairly arbitrary (and, in fact, easily lifted)

I think that at this point in time supertraits are ingrained in rustaceans' minds as something natural even if they are arbitrary. In fact, I just realized that it's a (very) special case of implied bounds.


Thanks everyone for their explanations!

@kmcallister kmcallister added A-traits Area: Trait system A-typesystem Area: The type system labels Jan 11, 2015
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Mar 6, 2015
all where-clauses, but supertrait considers a more limited
set. Supertrait expansion is suitable for virtual tables and associated
type inclusion, elaboration for more general reasoning.

Fixes rust-lang#20671.
@brendanzab
Copy link
Member

Not sure if it is related, but this doesn't work either:

use std::ops::*;

trait Vector<S> where
    for<'a, 'b> &'a Self: Add<&'b Self>,
    for<'a> &'a Self: Mul<S>,
{
    fn test(&self) {}
}

fn test<S, V: Vector<S>>(v: V) { v.test() }
<anon>:10:36: 10:42 error: the trait `for<'b, 'a> core::ops::Add<&'b V>` is not implemented for the type `&'a V` [E0277]
<anon>:10 fn test<S, V: Vector<S>>(v: V) { v.test() }
                                             ^~~~~~
<anon>:10:36: 10:42 help: see the detailed explanation for E0277
<anon>:10:36: 10:42 error: the trait `for<'a> core::ops::Mul<S>` is not implemented for the type `&'a V` [E0277]
<anon>:10 fn test<S, V: Vector<S>>(v: V) { v.test() }
                                             ^~~~~~

http://is.gd/FgFAWo

This is needed for cgmath to move full to operators, and remove our ugly mul_s-style methods :(

brendanzab added a commit to rustgd/cgmath that referenced this issue Sep 30, 2015
…ations

We can't yet remove the operator methods, due to rust-lang/rust#20671

This also removes the implementations of `Zero` and `One` for vectors.
brendanzab added a commit to rustgd/cgmath that referenced this issue Sep 30, 2015
We can't yet remove the operator methods, due to rust-lang/rust#20671
brendanzab added a commit to rustgd/cgmath that referenced this issue Sep 30, 2015
We can't yet remove the operator methods, due to rust-lang/rust#20671
@arielb1
Copy link
Contributor

arielb1 commented Sep 30, 2015

@bjz

Same "issue"

@nikomatsakis nikomatsakis changed the title where clauses for super traits do not fully propagate where clauses are only elaborated for supertraits, and not other things Oct 20, 2015
@russellmcc
Copy link
Contributor

I ran into this issue as well, and was really surprised by it. One situation where it would be very useful is refining traits that are used in higher-kinded trait bounds like in this example: https://gist.github.com/anonymous/e3a8fe834bb383504bb4

If we don't propagate associated type bounds for where clause traits, I don't know how to actually put the bound I need in the linked example on the function: the bound itself is polymorphic over the lifetime, which seems to not be expressible in the grammar. So, it seems that without this propagation there are useful bounds that we can't express.

@nikomatsakis
Copy link
Contributor

@russellmcc what bound are you unable to express precisely?

@russellmcc
Copy link
Contributor

In that example, I want to say that T: for<'a> Iterable<'a, Item=u32>, and in addition to that, for all 'a, <T as Iterable<'a>>::Iter implements the ExactSizeIterable<Item=&'a u32> trait. Unless I'm missing something, that can't currently be expressed. If we weren't talking about higher-kind bounds, I could add another free type variable E: ExactSizeIterable<&'a u32>, and then bound T::Iter=E, but that doesn't work because in this situation E would have to be higher kind, and there's no way to bound equality for higher kinded types that I know of.

Making the trait bounds propagate for associated types would allow me to express the exact same bound in a convenient way, as in my previous example.

@cuviper
Copy link
Member

cuviper commented Dec 22, 2015

I ran into something like this trying to create an AddAny supertrait, for all the variations of x + y, x + &y, &x + y, and &x + &y. The bounds I put on &Self as the left side of an Add won't propagate. It's pretty surprising to me that T: AddAny doesn't imply all of its transitive constraints, instead of just those on Self.
https://users.rust-lang.org/t/traits-with-self-bounds/4033

@SkiFire13
Copy link
Contributor

Interestingly this issue doesn't affect trait aliases. I don't know if it's possible but couldn't the compiler use the same logic to elaborate the where clauses in normal traits? I would expect them to work the same.

#![feature(trait_alias)]

trait Foo<T: ?Sized> {
   fn foo(&self) -> &T;
}

trait Bar<A> where A: Foo<Self> {}

// This doens't compile
// fn foobar_original<A, B: Bar<A>>(a: &A) -> &B {
//   a.foo()
// }

trait BarAlias<A> = Bar<A> where A: Foo<Self>;
// This does compile
fn foobar_alias<A, B: BarAlias<A>>(a: &A) -> &B {
  a.foo()
}

trait BarAliasExt<A>: BarAlias<A> {}
// This also compiles
fn foobar_super<A, B: BarAliasExt<A>>(a: &A) -> &B {
  a.foo()
}

@programmerjake
Copy link
Member

One other case where this appears (#85243):

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e13f9ef0a5c88c03fca13d57418f4986

pub trait Trait1 {
    fn f();
}

pub trait Trait2 {
    type Type: Trait1;
}

pub trait Trait3 {
    type Type2: Trait2<Type = Self>;
    fn f2() {
        <Self as Trait1>::f();
    }
}

I expected to see this compile successfully since in Trait3, Self is guaranteed to implement Trait1 because Self = Trait2::Type and Trait2::Type: Trait1.

Instead, this happened:

error[E0277]: the trait bound `Self: Trait1` is not satisfied

@cuviper
Copy link
Member

cuviper commented Sep 10, 2021

Would it be feasible to lint and point folks to this issue? Or is that just as hard as elaborating would be?

@c410-f3r
Copy link
Contributor

c410-f3r commented Apr 1, 2022

Another related snippet (I guess). Also not sure if this would be fixed by implied_bounds.

trait Foo: Iterator
where
    Self::Item: Default,
{
}

fn bar<T>(_: T)
where
    T: Foo,
    // Why the necessity of this bound? `Foo` already implies `Item: Default`
    T::Item: Default
{
}

KnucklesAni added a commit to yace-project/yace that referenced this issue Feb 22, 2023
While isize displacement is enough for native compilation we may also want
to generate code for a foreign architecture and this is realitively simple
change.

The only complication is rust-lang/rust#20671
Because of that limitation we have a lot of code duplication in that module.

Also add smoke tests to verify that all supported operations work.
@DieracDelta
Copy link
Contributor

DieracDelta commented Mar 9, 2023

I'm running into this. Specifically something along the lines of:

pub trait A {
/// lots of associated types with trait bounds
}

pub trait B : A 
where
/// constrains A's associated types further
{
}

For any impl block that is generic over a type that implements B I have to restate the where clause on B (assuming the added bounds on the associated types of A are used). This doesn't scale well as the number of types on A increases.

Is there an idiomatic workaround? The best I could think of is to restate the associated types, but this still seems suboptimal.

@jplatte
Copy link
Contributor

jplatte commented Mar 9, 2023

The best I could think of is to restate the associated types, but this still seems suboptimal.

That is the only workaround I know of. I would be very surprised if there was another way.

@ntc2
Copy link

ntc2 commented Jan 18, 2024

As a relatively new Rust user I just ran into this and found it super confusing, and seemingly inconsistent; in fact, I assumed it was a bug and tried upgrading to nightly to see if it went away. I want to emphasize that encountering this in the wild didn't simply feel like another thing that Rust doesn't support (cf. GADTs or arithmetic on type-level constants), but rather it felt strongly like a bug.

As mentioned in the issue title, the distinction is that "bounds on associated type are not super traits", and "only super traits are propagated", but this is not a satisfactory answer from a (this!) user point of view. Indeed, I didn't even have a precise concept of "super trait" in mind before I found the explanation here; the associated-type bound just felt like another bound.

When I ran into this in the wild, I eventually reduced my "real" code to this minimal example (similar to other examples above):

trait HasA {
    type A;
    fn get_a(&self) -> Self::A;
}

trait Foo {
    fn foo(&self);
}

trait Helper
where
    Self: HasA,
    Self::A: Foo,
{
}

fn test1<T: Helper>(t: T) {}

I was totally baffled that the Self: HasA bound in the definition of Helper propagates but the Self::A: Foo bound does not. Syntactically, they seem to be of the same logical class, as two bounds in the where clause (even if only one of them is a "super trait").

Moreover, it's super confusing that the function test1 here doesn't type check, since usually asserting a bound (here Helper) on an argument lets you make more assumptions about the argument in the body of the function, but here it somehow makes the function fail to type check, even tho the argument isn't even used!!!

It took me an hour+ to reduce my "real" code to this simple illustration of the problem, where my actual usage is more analogous to the following test2 function, which also fails to type check, for the same reason as test1 above:

fn test2<T: Helper>(t: T) {
    t.get_a().foo();
}

p.s. as mentioned above, apparently this doesn't affect trait aliases, which hopefully means this is easy to fix? Unfortunately, I can't use nightly in my project, or I would just switch to a trait alias, since creating a trait alias was exactly my goal.

@ntc2
Copy link

ntc2 commented Jan 23, 2024

A friend showed me a workaround. It's confusing when described abstractly, so probably better to just read the code below. But in summary: it seems that bounds on types associated with the trait being defined do get propagated, whereas we saw above that bounds on types associated with traits that bound the trait being defined don't get propagated.

Compare M1 and M2 below: here M1 is a copy of the broken code from my previous comment, whereas M2 is a working variation using the workaround:

// Doesn't type check
mod M1 {
    trait HasA {
        type A;
        fn get_a(&self) -> Self::A;
    }

    trait Foo {
        fn foo(&self);
    }

    trait Helper
    where
        // This `HasA` bound on `Self` propagates.
        Self: HasA,
        // This `Foo` bound on `<Self as HasA>::A` doesn't propagate.
        Self::A: Foo,
    {
    }

    // Doesn't type check: the bound `T::A: Foo` is not satisfied!?
    fn test1<T: Helper>(t: T) {}

    // Doesn't type check: same as above.
    fn test2<T: Helper>(t: T) {
        t.get_a().foo();
    }
}

// Variation on `M1` that does type check
mod M2 {
    trait HasA {
        type A;
        fn get_a(&self) -> Self::A;
    }

    trait Foo {
        fn foo(&self);
    }

    // Trick: define a "dummy" associated type `Helper::_A` that is
    // constrained to be equal to the associated type `HasA::A` that we
    // actually want to bound. By bounding `Helper::_A` we then implicitly
    // bound `HasA::A`, since apparently the equality constraint *does*
    // propagate.
    trait Helper: HasA<A = <Self as Helper>::_A> {
        // This `Foo` bound on `<Self as Helper>::_A` does propagate,
        // even tho a `Foo` bound on `<Self as HasA>::A` doesn't :/
        type _A: Foo;
    }

    fn test1<T: Helper>(t: T) {}

    fn test2<T: Helper>(t: T) {
        t.get_a().foo();
    }
}

apoelstra added a commit to rust-bitcoin/rust-miniscript that referenced this issue Mar 5, 2024
164bde9 miniscript: add parsing benchmarks (Andrew Poelstra)
bbafc39 expression: add parsing benchmark (Andrew Poelstra)
30d2d11 miniscript: factor out a couple parts of Terminal::from_tree (Andrew Poelstra)
f22dc3a miniscript: use new TRUE/FALSE constants throughout the codebase (Andrew Poelstra)
be41d8b miniscript: add constants for 1 and 0 (Andrew Poelstra)
270e65d Replace macros with blanket impls (Andrew Poelstra)
5ec0f44 add `blanket_traits` module to replace very noisy trait definitions (Andrew Poelstra)

Pull request description:

  There are several macros we use to implement functions when our `Pk` types satisfy a large bundle of trait conditions. There is a trick rust-lang/rust#20671 (comment) that we can use instead.

  While I'm at it, do several other small cleanups.

  As a weekend project I rewrote the expression module and was able to get a 10-15% speedup on parsing miniscripts, while eliminating recursion and simplifying the algorithms. I am still working on cleaning up the code and improving the error handling. This is the first set of commits from that branch, which should be simple and uncontroversial.

ACKs for top commit:
  apoelstra:
    > ACK [164bde9](164bde9). Are you taking a performance refactor stab at rust-miniscript?
  sanket1729:
    ACK 164bde9. Are you taking a performance refactor stab at rust-miniscript?

Tree-SHA512: 42e11f5f45fa705e14334e79126a66c97577fc6e807f804beaa1532bf3693a6c41a8b714bc4d1436209a1e0d808dc6a3ccc7198f20ff467f40f12126d1ee02f3
@fmease fmease added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system A-typesystem Area: The type system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests