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

Handle ?Sized better in ZeroFrom derive #4657

Merged
merged 6 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
- `yoke`
- Remove `StableDeref` bound from `Yoke<Y, Option<C>>` methods (https://github.com/unicode-org/icu4x/pull/4457)
- Added `CartableOptionPointer` and function to convert from `Yoke<Y, Option<C>>` (https://github.com/unicode-org/icu4x/pull/4449)\
- `zerofrom`
- Support `?Sized` type parameters which must be `Sized` to implement `ZeroFrom` (https://github.com/unicode-org/icu4x/pull/4657)
- `zerotrie`
- Add `as_borrowed_slice` and `AsRef` impl (https://github.com/unicode-org/icu4x/pull/4381)
- Add `ZeroTrieSimpleAsciiCursor` for manual iteration (https://github.com/unicode-org/icu4x/pull/4383)
Expand Down
18 changes: 10 additions & 8 deletions utils/zerofrom/derive/examples/zf_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#![allow(unused)]

use std::borrow::Cow;
use std::{borrow::Cow, marker::PhantomData};
use zerofrom::ZeroFrom;
use zerovec::{maps::ZeroMapKV, ule::AsULE, VarZeroVec, ZeroMap, ZeroVec};

Expand Down Expand Up @@ -87,27 +87,29 @@ pub enum CloningZF3<'data> {
}

#[derive(ZeroFrom)]
#[zerofrom(may_borrow(T))] // instead of treating T as a copy type, we want to allow zerofromming T too
pub struct GenericsThatAreAlsoZf<T> {
#[zerofrom(may_borrow(T, Q))] // instead of treating T as a copy type, we want to allow zerofromming T too
pub struct GenericsThatAreAlsoZf<T, Q: ?Sized, Ignored: ?Sized> {
x: T,
y: Option<T>,
ignored: PhantomData<Ignored>,
q: Q,
}

pub fn assert_zf_generics_may_borrow<'a, 'b>(
x: &'b GenericsThatAreAlsoZf<&'a str>,
) -> GenericsThatAreAlsoZf<&'b str> {
GenericsThatAreAlsoZf::<&'b str>::zero_from(x)
x: &'b GenericsThatAreAlsoZf<&'a str, usize, str>,
) -> GenericsThatAreAlsoZf<&'b str, usize, str> {
GenericsThatAreAlsoZf::<&'b str, usize, str>::zero_from(x)
}

#[derive(ZeroFrom)]
pub struct UsesGenericsThatAreAlsoZf<'a> {
x: GenericsThatAreAlsoZf<&'a str>,
x: GenericsThatAreAlsoZf<&'a str, usize, str>,
}

// Ensure it works with invariant types too
#[derive(ZeroFrom)]
pub struct UsesGenericsThatAreAlsoZfWithMap<'a> {
x: GenericsThatAreAlsoZf<ZeroMap<'a, str, str>>,
x: GenericsThatAreAlsoZf<ZeroMap<'a, str, str>, usize, str>,
}

fn main() {}
20 changes: 17 additions & 3 deletions utils/zerofrom/derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use syn::fold::{self, Fold};
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{
parse_macro_input, parse_quote, DeriveInput, Ident, Lifetime, MetaList, Token, Type, TypePath,
WherePredicate,
parse_macro_input, parse_quote, DeriveInput, Ident, Lifetime, MetaList, Token,
TraitBoundModifier, Type, TypeParamBound, TypePath, WherePredicate,
};
use synstructure::Structure;
mod visitor;
Expand Down Expand Up @@ -121,12 +121,26 @@ fn zf_derive_impl(input: &DeriveInput) -> TokenStream2 {
// a newly introduced mirror type parameter that we are borrowing from, similar to C in the original trait.
// For convenience, we are calling these "C types"
let generics_env: HashMap<Ident, Option<Ident>> = tybounds
.iter()
.iter_mut()
.map(|param| {
// First one doesn't work yet https://github.com/rust-lang/rust/issues/114393
let maybe_new_param = if has_attr(&param.attrs, "may_borrow")
|| may_borrow_attrs.contains(&param.ident)
{
// Remove `?Sized`` bound because we need a param to be Sized in order to take a ZeroFrom of it.
// This only applies to fields marked as `may_borrow`.
let mut bounds = core::mem::take(&mut param.bounds);
while let Some(bound_pair) = bounds.pop() {
let bound = bound_pair.into_value();
if let TypeParamBound::Trait(ref trait_bound) = bound {
if trait_bound.path.get_ident() == Some(&quote::format_ident!("Sized"))
Copy link
Member

Choose a reason for hiding this comment

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

you should just be able to == str here

Copy link
Member

Choose a reason for hiding this comment

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

.get_ident().map(|x| x == "Sized").unwrap_or(false)

&& matches!(trait_bound.modifier, TraitBoundModifier::Maybe(_))
{
continue;
}
}
param.bounds.push(bound);
}
Some(Ident::new(
&format!("{}ZFParamC", param.ident),
param.ident.span(),
Expand Down
8 changes: 8 additions & 0 deletions utils/zerofrom/src/zero_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use core::marker::PhantomData;

#[cfg(feature = "alloc")]
use alloc::borrow::{Cow, ToOwned};
#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -126,3 +128,9 @@ impl<'zf, T> ZeroFrom<'zf, [T]> for &'zf [T] {
other
}
}

impl<'zf, T: ?Sized + 'zf> ZeroFrom<'zf, PhantomData<T>> for PhantomData<T> {
fn zero_from(other: &'zf PhantomData<T>) -> Self {
*other
}
}
Loading