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

tezos-encoding + derive: keep track of lifetime in NomReader #46

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

rafoo
Copy link

@rafoo rafoo commented Nov 20, 2023

The NomReader trait is implicitly parameterized by the lifetime of the input byte slice. This commit makes the quantification on this explicit. This is needed to implement NomReader for structures which keep a reference to some slice of the input, for example to implement lazy deserialization.

@emturner emturner self-requested a review November 21, 2023 09:50
tezos-encoding/src/lib.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
tezos-encoding/src/nom.rs Outdated Show resolved Hide resolved
deserialized: Option<T>,
}

impl<'a, T> NomReader<'a> for LazyCell<'a, T>
Copy link
Collaborator

Choose a reason for hiding this comment

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

ideally would have a impl From<T> for LazyCell`?

and a BinWriter for LazyCell which internally would 'fail' the value if not present.

@emturner
Copy link
Collaborator

Thanks for the PR! Overall a big fan of adding something like this.

Pretty happy with the first commit - most comments focused on the second (perhaps could be split out into a second PR if needed)

@rafoo
Copy link
Author

rafoo commented Nov 21, 2023

Thanks for the review. I agree to split this in two PRs.

@rafoo rafoo force-pushed the rafoo@lifetime_in_nomreader branch 2 times, most recently from 6359e45 to cc63f5f Compare December 5, 2023 15:26
@rafoo rafoo requested a review from emturner December 5, 2023 15:26
@rafoo
Copy link
Author

rafoo commented Dec 8, 2023

I've removed the second commit. All remaining comments are now outdated if I am not mistaken.

The `NomReader` trait is implicitly parameterized by the lifetime of
the input byte slice. This commit makes the quantification on this
explicit. This is needed to implement `NomReader` for structures which
keep a reference to some slice of the input, for example to implement
lazy deserialization.
@rafoo rafoo force-pushed the rafoo@lifetime_in_nomreader branch from cc63f5f to 6cb8c5e Compare December 8, 2023 14:52
@emturner emturner merged commit f34d805 into trilitech:master Dec 8, 2023
7 checks passed
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