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

Change struct representation to transparent #13

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Conversation

spastorino
Copy link
Owner

dynosaur/tests/pass/basic-mut.stdout Outdated Show resolved Hide resolved
Comment on lines 51 to 54
pub fn boxed(value: impl MyTrait)
-> Box<DynMyTrait<'dynosaur_struct>> {
Self::new(Box::new(value))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to above

dynosaur/tests/pass/basic-mut.stdout Outdated Show resolved Hide resolved
dynosaur/tests/pass/basic-mut.stdout Outdated Show resolved Hide resolved
}
pub fn new(value: Box<impl MyTrait>)
-> Box<DynMyTrait<'dynosaur_struct>> {
let value: Box<dyn ErasedMyTrait> = value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see how this could have compiled since it's implicitly Box<dyn ErasedMyTrait + 'static>. It would require that the parameter was Box<impl MyTrait + 'static>.

@spastorino spastorino force-pushed the change-struct-repr branch 2 times, most recently from 59f90cb to d43be76 Compare August 21, 2024 20:00
Copy link
Collaborator

@tmandry tmandry left a comment

Choose a reason for hiding this comment

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

Approved but if you address my last comment in this PR, ideally I'd like to take another look.

dynosaur/tests/pass/basic-mut.stdout Outdated Show resolved Hide resolved
@tmandry tmandry merged commit d5c5995 into main Aug 26, 2024
1 check passed
@tmandry tmandry deleted the change-struct-repr branch August 26, 2024 17:04
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