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

Store plain DeviceT instead of Managed<_> in EthernetInterface #73

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

phil-opp
Copy link
Contributor

@phil-opp phil-opp commented Nov 8, 2017

This avoids unnecessary boxing of the device and simplifies the code. However, it does no longer allow EthernetInterfaces with a borrowed device.

@whitequark
Copy link
Contributor

whitequark commented Nov 8, 2017

I think the idea was to make it possible to store a trait object, but Device is no longer object-safe anyway, right? It doesn't seem important to me to allow an EthernetInterface with a borrowed Device, and even if it is, we can restore this functionality.

@phil-opp
Copy link
Contributor Author

phil-opp commented Nov 8, 2017

In smoltcp-rs/rust-managed#3 (comment), you assumed that it used to store a trait object. Now it is a generic parameter, so we can store DeviceT directly instead of Managed<DeviceT>.

@phil-opp
Copy link
Contributor Author

phil-opp commented Nov 8, 2017

I'm not sure about the object safety of Device, but I will take a look.

@whitequark whitequark merged commit 4ae84ab into smoltcp-rs:master Nov 8, 2017
@phil-opp
Copy link
Contributor Author

phil-opp commented Nov 8, 2017

I'm not sure about the object safety of Device, but I will take a look.

The problem seems to be that we need to specify the associated types in order to construct a Box:

error[E0191]: the value of the associated type `TxToken` (from the trait `phy::Device`) must be specified
  --> src/iface/ethernet.rs:35:31
   |
35 |     device: ::std::boxed::Box<Device<'a>>,
   |                               ^^^^^^^^^^ missing associated type `TxToken` value

So we would need generic parameters again…

@phil-opp phil-opp deleted the patch-a branch November 8, 2017 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants