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

Expand ZeroFrom to cover cases where attrs can borrow #3770

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Aug 3, 2023

This adds zerofrom(may_borrow), which allows deriving ZeroFrom on generic types which have parameters we care about borrowing.

I'm not happy about the terminology in ZF, we have a couple things talking about the same thing:

  • the "C type", which is the type fed into zero_from, and the type parameter of ZeroFrom
  • 'zf_inner, which is the lifetime of the "C type"
  • lifetime_ty, which is the C type version of the field type. usually the type with 'zf_inner replacing its main lifetime

I'd like to come up with a nice name for these things we can make consistent. I try to document this a bit better here though.

I'm doing this in an attempt to start using generics in datetime symbols code more; I have #3767 open but I want to try and clean up the rest.

If this approach works I plan to do something similar in Yoke.

may_borrow can't be applied directly to type parameters due to rust-lang/rust#114393. This code is future-proof for whenever that upstream bug is fixed.

Part of #3771

Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems about right. It's been a little while since I've looked at this derive code so it's possible there's things I missed.

@Manishearth Manishearth merged commit 8ba2513 into unicode-org:main Aug 10, 2023
25 checks passed
@Manishearth Manishearth deleted the zf-generics-may-borrow branch August 10, 2023 01:08
@Manishearth Manishearth mentioned this pull request Sep 21, 2023
13 tasks
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 this pull request may close these issues.

2 participants