# TaskLog: Improvement of IntoOwned TaskLog: Improvement of IntoOwned ================================= [![DongYoon Kang's photo](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966587995/acf07fe0-3dd9-4294-a57c-7b8659e6fd5d.jpeg)](https://hashnode.com/@kdy1) [DongYoon Kang](https://hashnode.com/@kdy1) ·[Oct 11, 2023](https://kdy1.dev/2023-10-12-extension-of-intoowned)· 10 min read Table of contents ----------------- * [ Problem ](https://kdy1.dev/#heading-problem) * [ Steps ](https://kdy1.dev/#heading-steps) * [ Add derive, ignoring errors ](https://kdy1.dev/#heading-add-derive-ignoring-errors) * [ New crate, move codes ](https://kdy1.dev/#heading-new-crate-move-codes) * [ Manual impl for various types ](https://kdy1.dev/#heading-manual-impl-for-various-types) * [ Triage to use derive for selectors ](https://kdy1.dev/#heading-triage-to-use-derive-for-selectors) * [ Manual implementation for Selector ](https://kdy1.dev/#heading-manual-implementation-for-selector) * [ Relaxed bounds of generic implementations ](https://kdy1.dev/#heading-relaxed-bounds-of-generic-implementations) * [ Generic impl for Box<\[T\]> ](https://kdy1.dev/#heading-generic-impl-for-boxlttgt) * [ Relax bounds derives ](https://kdy1.dev/#heading-relax-bounds-derives) * [ extern crate ](https://kdy1.dev/#heading-extern-crate) * [ Implement for tuples ](https://kdy1.dev/#heading-implement-for-tuples) * [ Use ::Owned for generics ](https://kdy1.dev/#heading-use-owned-for-generics) * [ Remove 'static bound from derive ](https://kdy1.dev/#heading-remove-static-bound-from-derive) * [ for<'aa> => 'any , in manual impl ](https://kdy1.dev/#heading-forltaagt-gt-any-in-manual-impl) * [ Debugging & Add one assertion ](https://kdy1.dev/#heading-debugging-amp-add-one-assertion) * [ More derive ](https://kdy1.dev/#heading-more-derive) * [ Wrapping up ](https://kdy1.dev/#heading-wrapping-up) * [ Notable approaches ](https://kdy1.dev/#heading-notable-approaches) * [ Implement at least one item manually before writing a derive macro ](https://kdy1.dev/#heading-implement-at-least-one-item-manually-before-writing-a-derive-macro) * [ Analyze requirements of generics ](https://kdy1.dev/#heading-analyze-requirements-of-generics) * [ Making types explicit to control rustc ](https://kdy1.dev/#heading-making-types-explicit-to-control-rustc) * [ Conclusion ](https://kdy1.dev/#heading-conclusion) * [ P.S. ](https://kdy1.dev/#heading-ps) I'm back with `lightingcss` again! But it's just an example I selected, and what I want to say is: > You need to repeat to learn how to think. Thinking is passing by, and there are so many that you shouldn't think you can use the same way next time just because you've used it once. * * * Let's see the PR for today. * PR: [https://github.com/parcel-bundler/lightningcss/pull/599](https://github.com/parcel-bundler/lightningcss/pull/599) This is super complex PR, of course. That's why it deserves the example of these blog posts. If a task is very complex, it's quite easy to analyze my approaches after finishing the work. * * * Problem ======= While trying to use the result of the previous work, I found that I forgot to add `derive`s for `IntoOwned`. Oops. But well, I'm very used to doing things like this. This should not be a big deal. So I just opened `lightningcss` in vscode and added #[cfg_attr(feature = "into_owned", derive(lightningcss_derive::IntoOwned))] to various types, including `StyleRule`. However due to the design decisions in previous work, `Selector`, which is used as a field of a variant of the `StyleRule`, cannot implement `IntoOwned` easily. Fundamentally, `Selector` is `parcel_selectors::Selector` and `StyleRule` is `lightningcss::StyleRule` where `lightningcss` depends on `parcel_selectors` . So... I thought I needed a separate crate for this, and asked the maintainer. This time, I didn't file an issue. Instead, I just opened a PR. * PR: [https://github.com/parcel-bundler/lightningcss/pull/599](https://github.com/parcel-bundler/lightningcss/pull/599#issue-1935464613) One of the suggested solutions by the maintainer was creating a new crate, which is I wanted to do. So I decided to go with the path. Steps ===== This was a long journey. It took much longer than initially expected, but I think it was worth it. Add `derive`, ignoring errors ----------------------------- * This step is done before creating a PR. I know what I want to do. I want to make `lightningcss::CssRule` , and related types implement `IntoOwned`. So I just added `derive` to them. As a result, I could get a useful error message. I have to implement them for `parcel_selector::Selector`and `parcel_selector::SelectorList` , but I couldn't because of the reason I described above. So I asked the maintainer and decided the way to go. New crate, move codes --------------------- I needed to create a new package, and naming a project is the hardest thing in the world of programming. But I don't like using lots of time for naming things, so I just selected `static-self`, which is quite intuitive. I created two new crates. The first one is the actual package, and the second one is a proc-macro, which makes implementing `IntoOwned` semi-automatic. After that, I just copy-pasted codes for `IntoOwned`. Both the derive macro and the runtime library. There were many red lines in IDE, but I don't care. I cleaned up the code so I could compile them. These cleanups were about adding relevant dependencies or adding a proper `mod into_owned;` statement. Manual impl for various types ----------------------------- `IntoOwned` is now a separate project, so it deserves the effort. I implemented `IntoOwned` for `Option` and `Box` as the maintainer recommended. > Btw, since it's now a trait, I think a bunch of the special cases (e.g. Option and Box) can also be implemented with generics instead of in the derive macro so it might get a lot simpler. impl<'any, T> IntoOwned<'any> for Option where T: for<'aa> IntoOwned<'aa>, { type Owned = Option<>::Owned>; fn into_owned(self) -> Self::Owned { self.map(|v| v.into_owned()) } } impl<'any, T> IntoOwned<'any> for Box where T: for<'aa> IntoOwned<'aa>, { type Owned = Box<>::Owned>; fn into_owned(self) -> Self::Owned { Box::new((*self).into_owned()) } } Those implementations are quite straightforward. I could remove special-case codes for `Option` and `Box` in the proc-macro. Triage to use derive for selectors ---------------------------------- As derive is cleaned up, I wanted to use it everywhere. #[cfg_attr(feature = "into_owned", derive(static_self::IntoOwned))] pub struct SelectorList<'i, Impl: SelectorImpl<'i>>( #[cfg_attr(feature = "serde", serde(borrow))] pub SmallVec<[Selector<'i, Impl>; 1]>, ); I added it to one of the problematic types. But if we do this, we have more problems. ![](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966589497/5844c07e-65ab-4525-8610-bc36aa077062.jpeg) `Impl` is a group for actual type parameters, and it's not used as a generic type parameter of a field. However, detecting this from a procedural macro is not a good idea. So, I decided to go with manual implementations for type with `Impl: SelectorImpl`. Manual implementation for `Selector` ------------------------------------ First, I tried #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>> static_self::IntoOwned<'any> for Selector<'i, Impl> where Impl: static_self::IntoOwned<'any>, >::Owned: SelectorImpl<'any>, { type Owned = Selector<'any, >::Owned>; fn into_owned(self) -> Self::Owned { Selector(self.0, self.1.into_owned()) } } to know what it should look like. Because of the bound, I needed one more implementation of `IntoOwned`. I did it for `Component`. I aliased `>::Owned` as `Sel`, for convenience. #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, Sel> static_self::IntoOwned<'any> for Selector<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = Sel>, Sel: SelectorImpl<'any>, { type Owned = Selector<'any, Sel>; fn into_owned(self) -> Self::Owned { Selector(self.0, self.1.into_owned()) } } Also, it became much more intuitive. This time, I also made some stupid mistakes. `Sel` is the output type, but I was confused and I tried something like #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, Sel> static_self::IntoOwned<'any> for Selector<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = Sel>, Sel: SelectorImpl<'any>, Sel::NamespaceUrl: for<'aa> IntoOwned<'aa>, // this one { type Owned = Selector<'any, Sel>; fn into_owned(self) -> Self::Owned { Selector(self.0, self.1.into_owned()) } } This bound is about requesting the `IntoOwned::Owned` to implement `IntoOwned`, which is pretty strange. #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, Sel> static_self::IntoOwned<'any> for Selector<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = Sel>, Sel: SelectorImpl<'any>, Impl::NamespaceUrl: for<'aa> static_self::IntoOwned<'aa, Owned = Sel::NamespaceUrl>, Impl::NamespacePrefix: for<'aa> static_self::IntoOwned<'aa, Owned = Sel::NamespacePrefix>, { type Owned = Selector<'any, Sel>; fn into_owned(self) -> Self::Owned { Selector(self.0, self.1.into_owned()) } } After some time, I found that these are the correct bounds. The old `NamespaceUrl` maps to the new `NamespaceUrl`, and so on. But some of the types had problems. I had to write bounds like #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, NewSel> static_self::IntoOwned<'any> for AttrSelectorWithOptionalNamespace<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = NewSel>, NewSel: SelectorImpl<'any>, Impl::LocalName: static_self::IntoOwned<'any, Owned = NewSel::LocalName>, NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>: static_self::IntoOwned<'any, Owned = NamespaceConstraint<(NewSel::NamespacePrefix, NewSel::NamespaceUrl)>>, ParsedAttrSelectorOperation: static_self::IntoOwned<'any, Owned = ParsedAttrSelectorOperation>, { type Owned = AttrSelectorWithOptionalNamespace<'any, NewSel>; fn into_owned(self) -> Self::Owned { AttrSelectorWithOptionalNamespace { namespace: self.namespace.into_owned(), local_name: self.local_name.into_owned(), local_name_lower: self.local_name_lower.into_owned(), operation: self.operation.into_owned(), never_matches: self.never_matches, } } } Relaxed bounds of generic implementations ----------------------------------------- I found that `for<'aa> IntoOwned<'aa>` is too much for bounds of type parameters, because we only need `Self<'any>`. So, I modified the bound of generic implementations first. impl<'any, T> IntoOwned<'any> for Vec where T: IntoOwned<'any>, { type Owned = Vec<>::Owned>; fn into_owned(self) -> Self::Owned { self.into_iter().map(|v| v.into_owned()).collect() } } So now if `T` implements `IntoOwned<'a>`, `Vec` implements `IntoOwned<'a>`. Previously, additional bounds were required because the bound was `T: for<'aa> x<'aa>` . Generic impl for `Box<[T]>` --------------------------- ![](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966590559/11f23f5d-4080-45fd-ac32-5440f6c07f9f.jpeg) I found that now the problem is `Box<[T]>`, because `[T]` is `?Sized` and our previous implementation has `*self`, which cannot be used with `T: ?Sized`. After trying to remove the `Sized` bound, I found that `Box where T: Sized` and `Box<[T]>` does not overlap. So I implemented `IntoOwned`for \`\[ impl<'any, T> IntoOwned<'any> for Box<[T]> where T: IntoOwned<'any>, { type Owned = Box<[>::Owned]>; fn into_owned(self) -> Self::Owned { self.into_vec().into_owned().into_boxed_slice() } } I wrote this, and it worked! Relax bounds derives -------------------- The procedural macro was doing for type_param in type_param_names { generics.make_where_clause().predicates.push_value(parse_quote! { #type_param: 'static + for<'aa> ::static_self::IntoOwned<'aa> }) } but as we saw above, this is too much. I modified this to for type_param in type_param_names { generics.make_where_clause().predicates.push_value(parse_quote! { #type_param: 'static + ::static_self::IntoOwned<'any> }) } and it worked like a charm! I thought the PR was done. `extern crate` -------------- `rustc` complained about missing `extern crate`, so I added one. #[cfg(feature = "into_owned")] extern crate static_self; After that, I found that, still, there are many problems. I did #[cfg(feature = "into_owned")] impl<'any> static_self::IntoOwned<'any> for DefaultAtRule { type Owned = Self; fn into_owned(self) -> Self { self } } and #[cfg(feature = "into_owned")] impl<'any> ::static_self::IntoOwned<'any> for Selectors { type Owned = Self; fn into_owned(self) -> Self::Owned { self } } but these were not enough. Implement for tuples -------------------- For first, there were bounds like #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, NewSel> static_self::IntoOwned<'any> for AttrSelectorWithOptionalNamespace<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = NewSel>, NewSel: SelectorImpl<'any>, Impl::LocalName: static_self::IntoOwned<'any, Owned = NewSel::LocalName>, NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>: static_self::IntoOwned<'any, Owned = NamespaceConstraint<(NewSel::NamespacePrefix, NewSel::NamespaceUrl)>>, ParsedAttrSelectorOperation: static_self::IntoOwned<'any, Owned = ParsedAttrSelectorOperation>, { type Owned = AttrSelectorWithOptionalNamespace<'any, NewSel>; fn into_owned(self) -> Self::Owned { AttrSelectorWithOptionalNamespace { namespace: self.namespace.into_owned(), local_name: self.local_name.into_owned(), local_name_lower: self.local_name_lower.into_owned(), operation: self.operation.into_owned(), never_matches: self.never_matches, } } } I was sure that `(Impl::NamespacePrefix, Impl::NamespaceUrl)` was causing at least one problem because I didn't implement `IntoOwned` for tuple types. So I implemented it, and it was easy. * Commit: [https://github.com/parcel-bundler/lightningcss/pull/599/commits/3edcb0e8cd95a322f3b65c228f1568c8bf5f263b](https://github.com/parcel-bundler/lightningcss/pull/599/commits/3edcb0e8cd95a322f3b65c228f1568c8bf5f263b) Use `::Owned` for generics -------------------------- As I had implemented `IntoOwned` for tuples, I modified the bounds to #[cfg(feature = "into_owned")] impl<'any, 'i, Impl: SelectorImpl<'i>, NewSel> static_self::IntoOwned<'any> for AttrSelectorWithOptionalNamespace<'i, Impl> where Impl: static_self::IntoOwned<'any, Owned = NewSel>, NewSel: SelectorImpl<'any>, Impl::LocalName: static_self::IntoOwned<'any, Owned = NewSel::LocalName>, Impl::NamespacePrefix: static_self::IntoOwned<'any, Owned = NewSel::NamespacePrefix>, Impl::NamespaceUrl: static_self::IntoOwned<'any, Owned = NewSel::NamespaceUrl>, { type Owned = AttrSelectorWithOptionalNamespace<'any, NewSel>; fn into_owned(self) -> Self::Owned { AttrSelectorWithOptionalNamespace { namespace: self.namespace.into_owned(), local_name: self.local_name.into_owned(), local_name_lower: self.local_name_lower.into_owned(), operation: self.operation.into_owned(), never_matches: self.never_matches, } } } But there were some more problems, and I used `cargo-expand` to see the generated code. The problem was that derived `IntoOwned` implementation was stupid. #[cfg_attr(feature = "into_owned", derive(static_self::IntoOwned))] pub enum NamespaceConstraint { Any, /// Empty string for no namespace Specific(NamespaceUrl), } became impl<'any, NamespaceUrl> ::static_self::IntoOwned<'any> for NamespaceConstraint where NamespaceUrl: 'static + ::static_self::IntoOwned<'any>, { type Owned = Self; #[inline] fn into_owned(self) -> Self { self } } meaning `IntoOwned` cannot change anything about the type. The output type should be `::Owned` , so I patched the procedural macro. Remove `'static` bound from derive ---------------------------------- ![](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966592299/2620a66c-8dd7-4aab-8981-0f92c3fb83f5.jpeg) I faced another error. `rustc` was complaining about `'static` lifetime, so I tried removing `'static` from the derive bound. for type_param in type_param_names { generics.make_where_clause().predicates.push_value(parse_quote! { #type_param: 'static + ::static_self::IntoOwned<'any> }) } is modified to for type_param in type_param_names { generics.make_where_clause().predicates.push_value(parse_quote! { #type_param: ::static_self::IntoOwned<'any> }) } and it was the answer. `for<'aa>` => `'any` , in manual impl ---------------------------------------- I concluded that `for<'aa> IntoOwned<'aa>` is way too much, so I modified all bounds in manual implementation to use `'any` instead. Then... another problem! This time, the error message is even cryptic. ![](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966593803/6853f9df-1727-4ebf-af04-46690a6cfd60.jpeg) I didn't think about the associated types of `SelectorImpl`. `rustc` insists > Associated types of the `SelectorImpl` implementation for `Selectors` do not implement `IntoOwned`, so `Selector<'_, Selectors>` does not implement `IntoOwned` in a very cryptic language. It's right, but how can I know the cause? Debugging & Add one assertion ----------------------------- There's a way to go. #[cfg(feature = "into_owned")] fn _assert_into_owned() { // Ensure that we provide into_owned use static_self::IntoOwned; fn _assert<'any, T: IntoOwned<'any>>() {} _assert::(); } With this assertion, `rustc` shows the real problem. The problem was that types like `CSSString<'i>` , which are associated types of `SelectorImpl`, didn't implement `IntoOwned`. Such assertion makes many types explicit and `rustc` kindly shows good error messages for explicit types. ![](https://cdn.hashnode.com/res/hashnode/image/upload/v1713966595524/9d96fc35-958f-4705-8f2e-d83218cf8f49.jpeg) Ah... I forgot to implement `IntoOwned` for types in `SelectorImpl`. More derive ----------- /// A pseudo class. #[derive(Clone, PartialEq, Eq, Hash)] #[cfg_attr( feature = "serde", derive(serde::Serialize, serde::Deserialize), serde(tag = "kind", rename_all = "kebab-case") )] #[cfg_attr(feature = "jsonschema", derive(schemars::JsonSchema))] #[cfg_attr(feature = "into_owned", derive(static_self::IntoOwned))] pub enum PseudoClass<'i> { // Lots of variants... } I just derived them. Wrapping up =========== Finally, it's done! Let's see what I should highlight here. Notable approaches ------------------ ### Implement at least one item manually before writing a derive macro I wanted to use `derive` for `Selector` and friends, too. But **I could conclude it's a waste of time before actually wasting lots of time** because I implemented it to `Selector` and found that it's impractical. ### Analyze requirements of generics * Note: In many cases, it's fine to postpone this. Generic bounds work if you have bounds that are too much. But it's not ideal, and it can cause problems, just like in this case. **If such an error happens, don't trust your generic implementation because "it does not error."** ### Making types explicit to control `rustc` Typically, the types generated from type inference is useless for human, and many build tools hide them from the user. **But sometimes, you want to validate the type. If so, be explicit to the compiler**. If you are using a good language, the compiler will show detailed diagnostics for your explicit types. Conclusion ========== I always think that it would have been better to do something and how. This is the key to changing the way you think. * * * P.S. ==== I'm sure I have a bad approach I wanted to write about, but I forgot what it was. * * * [TaskLog](https://kdy1.dev/tag/tasklog?source=tags_bottom_blogs) Share this[](https://twitter.com/share?url=https%3A%2F%2Fkdy1.dev%2F2023-10-12-extension-of-intoowned&text=%20%40kdy1)[](http://www.reddit.com/submit?title=TaskLog%3A%20Improvement%20of%20IntoOwned&selftext=true&text=%20https%3A%2F%2Fkdy1.dev%2F2023-10-12-extension-of-intoowned)