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

{To,From}{Bit,Byte}StreamWith: allow lifetime-constrained Contexts #16

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

fengalin
Copy link
Contributor

@fengalin fengalin commented Nov 30, 2023

Warning: this proposal introduces a breaking change. The change is as trivial as adding <'_>, but all implementers of {To,From}{Bit,Byte}StreamWith would be affected. IMO this would warrant a major version bump.

See the examples for a complete use case.

In current implementation, {To,From}{Bit,Byte}StreamWith impose that the Context be static. In some cases, we want to pass a Context which owns a reference to another struct.

This MR adds a lifetime to these traits to allow using a Context constrained by a lifetime. Ex.:

impl FromBitStreamWith<'a> for FrameHeader {
    type Context = StreamInfo<'a>;
    [...]
}

Implementations for which the Context is 'static bound can use:

impl FromBitStreamWith<'_> for FrameHeader {
    type Context = StreamInfo;
    [...]
}

@fengalin
Copy link
Contributor Author

I just realized this should also be applied to To{Bit,Byte}StreamWith. I'll push an update tomorrow.

In current implementation, `From{Bit,Byte}StreamWith` impose that the `Context`
be static. In some cases, we want to pass a `Context` which owns a reference to
another struct.

This commit adds a lifetime to these traits to allow using a `Context`
constrained by a lifetime. Ex.:

```rust
impl FromBitStreamWith<'a> for FrameHeader {
    type Context = StreamInfo<'a>;
    [...]
}
```

Implementations for which the `Context` is `'static` bound can use:

```rust
impl FromBitStreamWith<'_> for FrameHeader {
    type Context = StreamInfo;
    [...]
}
```
In current implementation, `To{Bit,Byte}StreamWith` impose that the `Context`
be static. In some cases, we want to pass a `Context` which owns a reference to
another struct.

This commit adds a lifetime to these traits to allow using a `Context`
constrained by a lifetime. Ex.:

```rust
impl ToBitStreamWith<'a> for FrameHeader {
    type Context = StreamInfo<'a>;
    [...]
}
```

Implementations for which the `Context` is `'static` bound can use:

```rust
impl ToBitStreamWith<'_> for FrameHeader {
    type Context = StreamInfo;
    [...]
}
```
@fengalin fengalin force-pushed the from_reader-lt-constrained-context branch from bfa3308 to 3ffca65 Compare December 1, 2023 09:36
@fengalin fengalin changed the title From{Bit,Byte}StreamWith: allow lifetime-constrained Contexts {To,From}{Bit,Byte}StreamWith: allow lifetime-constrained Contexts Dec 1, 2023
@fengalin
Copy link
Contributor Author

fengalin commented Dec 1, 2023

It's ready now.

@tuffy tuffy merged commit 21ba869 into tuffy:master Dec 1, 2023
@fengalin fengalin deleted the from_reader-lt-constrained-context branch December 1, 2023 19:06
@fengalin
Copy link
Contributor Author

fengalin commented Dec 7, 2023

Thank you for your reactivity @tuffy!

Since this requires a major version bump, I understand it might take some times before the next release. Do you have an idea when that could occur though? Just asking so I can schedule some downstream modifications accordingly.

@fengalin
Copy link
Contributor Author

fengalin commented Dec 9, 2023

Thanks for the release @tuffy!

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