Get rid of specialization #210
Comments
|
The codegen in pyo3-derive-backend is a problem once the above functions are removed. For example: pyo3/pyo3-derive-backend/src/py_class.rs Lines 329 to 336 in 5100676 pyo3/pyo3-derive-backend/src/py_impl.rs Lines 33 to 40 in 264f642 I am not familiar enough with it to have a solution. |
|
I've also already ran against that and haven't found a good solution either :/ I think the most viable way forward would be not to have a default implementation of |
|
#332 fixes the |
|
Is this still an issue given #332 being merged? I'm interested in helping push this to stable, and would appreciate advice for where to start. EDIT: It looks like the first two instances in |
|
@bspeice #210 (comment) is fixed by #332, while some specialization are still there. The main question is how we can rewrite the following so we don't have a conflicting implementation without specialization anymore: pub trait FooImpl {
fn foo() -> Option<ffi::Foo> {
None
}
}
impl<T> FooImpl for T {}
impl<'p, T> FooImpl for T
where
T: Bar<'p>,
{
fn foo() -> Option<ffi::PySequenceMethods> {
return bar();
}
}(You can see a lot of those in the error message by removing |
|
As a contributor to Mercurial where we started using Rust a few months ago, I can say that we very much look forward to using PyO3, and the progress towards a build on stable is really encouraging. |
|
It is technically fixable. We mainly have two options: InventoryInstead of implementing traits, the protocol implementations are submit to an inventory and added collected at class initialization time. The disadvantage here is that this essentially a well established linker hack, which can cause some weird interactions with the c api (e.g. #341). Inventory is already used for pymethods blocks (so that we can have 0 to n pymethods blocks). Manual annotationsYou could force explicit annotations on pyclass blocks about which protocols will be implemented. The pyclass block would add default implementations for all protocol you didn't specify. If we wanted to completely get rid of inventory, we'd also need to forbid multiple pymethod block (or add an awkward syntax for numbering them) and annotate whether a struct has one or zero pymethod blocks. |
|
I feel like #2 is not a particularly hard burden, especially with the outlook that it will get removed in some future. For me a few more manual steps are certainly worth it for going to stable. |
|
Is there anything I can do to help? I'm not sure how much time I can set aside, but compiling on stable would be a huge deal to the Mercurial project. |
|
@Alphare There are mainly two categories of traits that still use specialization: The first are already protocol implementation traits, which are used to implement the dunder methods. If I haven't made a grave mistake, extending #552 to all The other category are the conversion traits, specifically A way to get a (very verbose) todo list is to remove the |
|
I'm having a play with that technique to see if I can get it working for an Add method as an initial example. If someone else beats me to it I would not complain at all! Once Pyo3 is stable I expect serious uptick in usage. |
|
rg "default .*fn" seems to have ~60 hits now not 18, we're further away then when the issue was raised :-( . #552 doesn't compile - did it ever or was that the work in progress bit? Most of the specialised functions are static taking no args - all of @dtolnay 's excellent specialisation examples dependend upon selecting the specialisation based upon the type of the argument. So that's the bad news out of the way. Anyone got any good news / suggestions? |
It's because of #614
It's also because of #614
Yeah, we cannot use it.
We have a gitter channel. And what I can say is...
I think the later one should be resolved by changing the object layout of pyclass. |
I'm pretty sure that the approach using inventory still works, but so far no one volunteered to work in that. |
|
I’m happy doing the leg work. If you can guide me a bit as to what needs to
be done then will do my best. I really think python-rust integration is as
important as wasm-rust integration but alas doesn’t get as much focus. All
pointers gratefully received!!
…On Fri, 15 Nov 2019 at 14:50, konstin ***@***.***> wrote:
And, for the former one, I still have not decided what kind of approach we
should use.
@konstin <https://github.com/konstin>'s 'unified slots' approach is good
but still needs default.
I'm pretty sure that the approach using inventory still works, but so far
no one volunteered to work in that.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#210?email_source=notifications&email_token=AAGEJCCN6EKZ7PQEU77HLZLQT2ZMFA5CNFSM4FRR7SY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEFUWVY#issuecomment-554388311>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCGWK2N37KNPYULLJI3QT2ZMFANCNFSM4FRR7SYQ>
.
|
|
I have some alternative solutions that I've been hacking away at here: https://github.com/wickerwaka/pyo3/tree/removing-specialization I still have a about 30% of the tests stubbed out, but it is building and running with stable rust. The solution I settled on is having the proc_macros provide default implementations for the various protocol traits that the user has not implemented. For any missing implementations in Generally it has been working pretty well. I am currently trying to work around issues with Sel, Del and SetDel implementations for the sequence and mapping protocols, since the current implementations rely on specialization to make a compile time decision over which methods to use. Again, this is still very hacky and is just a proof of concept and learning experience for me, it is not in a mergable state. |
|
Nice hacking! One thing I really like about your approach: being more dependent on proc macros (compile time) rather than being more dependent on inventory (runtime) seems like a simpler solution. |
|
@davidhewitt |
|
I'm not familiar with the problem so just passing information along, would this be of any use? https://github.com/dtolnay/case-studies/blob/master/autoref-specialization/README.md Found it here: rust-lang/rust#31844 (comment) |
alas, no
|
FWIW I don't see the presence or absence of arguments as being relevant to the technique. You can always turn a no-arg function into a method by dispatching it on PhantomData::<T>. For example instead of: trait Trait {
fn f();
}
impl<T: Display> Trait for T {
default fn f() {...}
}
impl Trait for String {
fn f() {...}
}you would dispatch it as: impl<T: Display> DisplayKind for &PhantomData<T> {}
impl StringKind for PhantomData<String> {} |
|
Just an experimental note: trait LenProvider: Sized {
fn len() -> Option<Box<dyn Fn(PyObject) -> usize>> {
None
}
}
struct DummyProvider;
impl LenProvider for DummyProvider {}
struct LenProviderImpl<T: LenImpl>(PhantomData<T>);
impl<T: 'static + LenImpl> LenProvider for LenProviderImpl<T> {
fn len() -> Option<Box<dyn Fn(PyObject) -> usize>> {
fn wrap<T: LenImpl>(obj: PyObject) -> usize {
let obj = T::from_obj(obj);
obj.__len__().into().unwrap()
}
Some(Box::new(wrap::<T>))
}
}And then we can select which implementation to use in proc-macro code. |
|
I’m probably naive here, but why can’t we have a trait with all the dunder
methods on it with default implementations of unimplemented! and the we
just override methods we want to impl? Is it because you might be able to
write blanket impls more easily? Or is it that all the impl would have to
be in one place? I’m just trying to understand why the easy route is ruled
out.
…On Mon, 9 Dec 2019 at 06:50, Yuji Kanagawa ***@***.***> wrote:
Just an experimental note:
I'm considering the use of 'provider pattern' which uses switch
implementation by generics, instead of trait specialization:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8131e41b3c816bb317391c252f831ab0
In this pattern, we prepare 2 kinds of structs, which corresponds to
*default* implementation and *real* protocol implementation.
trait LenProvider: Sized {
fn len() -> Option<Box<dyn Fn(PyObject) -> usize>> {
None
}
}
struct DummyProvider;impl LenProvider for DummyProvider {}
struct LenProviderImpl<T: LenImpl>(PhantomData<T>);impl<T: 'static + LenImpl> LenProvider for LenProviderImpl<T> {
fn len() -> Option<Box<dyn Fn(PyObject) -> usize>> {
fn wrap<T: LenImpl>(obj: PyObject) -> usize {
let obj = T::from_obj(obj);
obj.__len__().into().unwrap()
}
Some(Box::new(wrap::<T>))
}
}
And then we can select which implementation to use in proc-macro code.
If a user implement __len__, we pass LenProviderImpl<T> to the
initialization function.
If he doesn't, we use DummyProvider.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#210?email_source=notifications&email_token=AAGEJCC2RRTOBX246MLWF3DQXXTEDA5CNFSM4FRR7SY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGIBIMQ#issuecomment-563090482>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCABJBZ7V5E6RMH3PBDQXXTEDANCNFSM4FRR7SYQ>
.
|
|
@gilescope |
Those protocols aren't just normal methods, but use different "tp slots" (s. https://docs.python.org/3/c-api/typeobj.html), and each protocol trait is for one of those slots. The python interpreter also will behave differently for methods that aren't there than for those that are implemented as error (and especially |
|
One thing to consider is moving to |
|
+1 on considering |
|
For now, I think we can remove specialization by using inventory for what is called Anyway, I think we should be more optimistic about this. As @konstin showed, there's certainly a way to remove specialization. |
|
I would love it if you have the chance to make that PR! That's easily the biggest use of specialization and then it should be relatively easy for us to get to stable 👍 Don't worry about my WIP branches breaking with changes you make - I will rebase them after 😂 |
|
STATUS: Now I'm experimenting with a new idea to re-implement object protocols without specialization. |
|
@kngwyu have you got the provider pattern branch anywhere? I'd be curious to read the code later and see if I could offer any ideas |
No. |
|
Alas, no worries. Many thanks for your continued work on this! (And of course take your time - we have waited a long time already, can wait a few more months 😄 ) |
|
🎉 |
The last step for #5 is getting rid of specialization. I've remove most uses in 7c0379b and subsequent commits, except for the followingrust-lang/rust#64564 changed the rules so there are again many1710974:default fn.Once those are removed, we pyo3 can be stable on rust 1.30! I'm happy to take pull requests tackling one or more of thosedefault fnusesThe text was updated successfully, but these errors were encountered: