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.

Sign up for GitHub

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

Jump to bottom

Make line::StateMachine covariant wrt R #324

Merged
merged 2 commits into from
Sep 2, 2018
Merged

Make line::StateMachine covariant wrt R #324

merged 2 commits into from
Sep 2, 2018

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Aug 31, 2018

This is a breaking change for readers with Offset != usize. Not keen on doing this, but I don't know a better way to get variance...

@philipc philipc requested a review from fitzgen August 31, 2018 08:03
@coveralls
Copy link

coveralls commented Aug 31, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.02%) to 84.787% when pulling 55adfde on philipc:statemachine-variance into 45f4f34 on gimli-rs:master.

@fitzgen
Copy link
Member

fitzgen commented Aug 31, 2018

The motivation is to allow StateMachine<MyReader<'a>> be passed to something that takes StateMachine<MyReader<'b>> where 'a: 'b, correct?

Can we add a simple test case that will fail to compile if that is not the case?

Otherwise LGTM, unless I am misunderstanding something.

@philipc
Copy link
Collaborator Author

philipc commented Sep 1, 2018

Correct. I've added a test case.

@philipc philipc merged commit 41e68ab into gimli-rs:master Sep 2, 2018
@philipc philipc deleted the statemachine-variance branch September 2, 2018 05:41
@fitzgen
Copy link
Member

fitzgen commented Sep 4, 2018

Correct. I've added a test case.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fitzgen fitzgen Awaiting requested review from fitzgen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@philipc @coveralls @fitzgen