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

Get rid of specialization: Roadmap #697

Closed
3 tasks
kngwyu opened this issue Dec 22, 2019 · 2 comments · Fixed by #969
Closed
3 tasks

Get rid of specialization: Roadmap #697

kngwyu opened this issue Dec 22, 2019 · 2 comments · Fixed by #969
Assignees

Comments

@kngwyu
Copy link
Member

kngwyu commented Dec 22, 2019

Apparently this is a follow-up issue of #210 and here goes a small TODO list for getting rid of specialization.

Yeah, but... please understand opening this means we are ready to resolve this soon!
I'll be busy for a while.

  • ObjectProtocol
  • Conversion
  • Sequence
  1. ObjectProtocol
    This is the main use of specialization.
    To resolve this we can use @konstin 's inventory trick(Poc dunder without specialization #552) or my dirty trick with provider trait.

  2. Conversion
    Like this.
    It is good that New #[pyclass] system that is aware of its concrete layout #683 makes pyclass detection easier and I think we can resolve this with not so complex implementation.
    But what can be problematic is some conversion traits depend on ToPyObject trait, which I really don't like.
    Now we can distinguish Rust-native and Python-native objects if we have T: TypeInfo, but, for ToPyObject, we cannot do it.
    Just adding PyTypeInfo bound might work, but I really don't like this situation.
    In my new semantics proposed in New #[pyclass] system that is aware of its concrete layout #683, our conversion hierarchy is like this:

Rust-native Python-native
In Rust T: PyClass Cannot exist!
In Python PyClassShell<T> <T as PyTypeInfo>::ConcreteLayout
Rust reference to the object in Python heap &T or &PyClassShell<T> &T

So, yeah, we can convert T: PyClass to Python objects..., but we should not treat them and their pointer (in Python heap) same as Python native types!

Currently, this kind of too general conversion trait needs specialization and ... I'm not sure how to resolve this.
Honestly, I want to create a new trait for the 'In Python' layer in the above table, but it'll be a breaking change and careful design change.

  1. Sequence
    https://github.com/PyO3/pyo3/blob/v0.8.4/src/types/sequence.rs#L248
    It's a problem with speed and efficiency.
    No technical problem is other than that.
@kngwyu kngwyu self-assigned this Dec 22, 2019
@davidhewitt
Copy link
Member

davidhewitt commented Dec 23, 2019

I've been thinking hard about the conversion table you drew out. I think this problem is closely related to #679.

Using the suggested design from #679, I would like to propose the table should look like this:

  Rust-native Python-native
Stack / GIL-Bound T: PyClass PyAny<'gil>, PyDict<'gil> etc.
Python heap Py<T> Py<PyAny>, Py<PyDict> etc.
Borrowed &'gil T &'gil PyAny<'gil>, &'gil PyDict<'gil> etc.

Going from the stack to the python heap could be done with Py::new().

It should also be possible to borrow from either of the first two rows to obtain the third. It should never be possible to go from a borrow to either of the first two forms.

@kngwyu
Copy link
Member Author

kngwyu commented Dec 28, 2019

I refer to conversion to explain what kind of conversion trait needs specialization and why.
So how should we modify them not to use specialization and to fit the real conversion hierarchy is what should be discussed, but

I would like to propose the table should look like this:

this should be discussed in #679.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants