-
Notifications
You must be signed in to change notification settings - Fork 48
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
When a trait has a nontrivial parameter, materialize it in the caller.
(Technically, this does let you call trait methods with nontrivial arguments,
but it's not in a final/decent place.)
This is an alternate version of unknown commit which tried to continue to defer
materialization. However, as discussed in[]
currently impossible.
In particular, we cannot define a trait like this:
```rs
impl<T: Ctor<Output=A>> MyTrait<T> for S {...}
impl<T: Ctor<Output=B>> MyTrait<T> for S {...}
```
... because Rust does not understand that these impls are disjoint:
rust-lang/rust#20400
#### What's next?
(Apologies if this is a bit unorganized, I've spent too much time in the trenches.)
So this CL is just a first step: we *must* monomorphize the implementation.
Rather than accepting any `T: Ctor`, accept an `RvalueReference` (a concrete type).
After this CL, I think we have a slightly more open field than I thought. In particular,
we should be able to regain the `Ctor` API, except using only *one* parameterized impl.
So, instead of the broken impls above, we can have this impl:
```rs
impl<'a> MyTrait<RvalueReference<'a, A>> for S {...}
impl<'a> MyTrait<RvalueReference<'a, B>> for S {...}
impl<U, CtorType> MyTrait<CtorType> for S
where
&C : for<'a> MyTrait<RvalueReference<'a, U>>,
CtorType: Ctor<Output=U>
{...}
```
Because this is only _one_ parameterized impl, there's no conflicts. It is
implemented in terms of the concrete non-parameterized impls as generated by
this change.
However, I'm not yet 100% certain this will work, and it is actually not a small task
to do, even on top of this CL. For example, there's a bunch of refactoring to let one
generate a second blanket impl using knowledge about the trait function etc. from the
concrete impl.
##### RvalueReference might need to get replaced.
If we can use the `Ctor` approach described above... we can't use `RvalueReference`,
actually, because Rust will then recurse infinitely. The `RvalueReference` type used
for the `for<'a> MyTrait<RvalueReference<...>>` bound must be in the _same_ crate so
that Rust knows that `RvalueReference` doesn't itself impl `Ctor`. And unfortunately,
no, negative impls aren't good enough here, yet, apparently. At least, it didn't
resolve it when I tried it!
You can test this in a local two-crate setup.
Crate 1:
```rs
pub trait Ctor {
type Output;
}
pub struct RvalueReference<'a, T>(&'a T);
```
Crate 2:
```rs
use lib1::*;
pub struct A1;
pub struct A2;
pub struct B;
impl <'a> From<RvalueReference<'a, A1>> for B {
fn from(_: RvalueReference<'a, A1>) -> Self { todo!(); }
}
impl <'a> From<RvalueReference<'a, A2>> for B {
fn from(_: RvalueReference<'a, A2>) -> Self { todo!(); }
}
impl <T: Ctor> From<T> for B
where B : for<'a> From<RvalueReference<'a, T::Output>> {
fn from(_: T) -> Self { todo!(); }
}
```
If you build crate 2, it will fail with the following error:
```
error[E0275]: overflow evaluating the requirement `for<'a> B: From<lib1::RvalueReference<'a, _>>`
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`lib2`)
note: required because of the requirements on the impl of `for<'a> From<lib1::RvalueReference<'a, _>>` for `B`
--> src/lib.rs:15:16
|
15 | impl <T: Ctor> From<T> for B
| ^^^^^^^ ^
= note: 126 redundant requirements hidden
= note: required because of the requirements on the impl of `for<'a> From<lib1::RvalueReference<'a, _>>` for `B`
For more information about this error, try `rustc --explain E0275`.
error: could not compile `lib2` due to previous error
```
But it will work fine if you move `RvalueReference` to another crate!
##### If all else fails, we'll force the caller to materialize the Ctor
If even the approach outlined above doesn't work, well, we'll just have to force callers
to materialize the `Ctor`: call `Trait::method(mov(emplace!(foo())))` instead of
`Trait::method(foo())`.
That's what I described in[]
but I'm hoping we can work our way out after all!
Either way, both approaches build on this change. Investigating the followups may take some
time, so I'd rather not leave this change sitting around generating merge conflicts,
if possible. :X
PiperOrigin-RevId: 464613254- Loading branch information
1 parent
7ed8c6f
commit 2b1e46d
Showing
4 changed files
with
200 additions
and
44 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters