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

Use default type parameter for LocatedSpan #51

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Use default type parameter for LocatedSpan #51

merged 2 commits into from
Feb 11, 2020

Conversation

zakcutner
Copy link
Contributor

Thanks for the help with my previous pull request! I have decided to create another one for something small that has been bugging me for a bit...

When the addition of extra information to LocatedSpan was implemented, a new LocatedSpanEx struct was added which includes an extra property with generic parameter X. Consequently, the original LocatedSpan has become a type alias where X is set to the empty type ().

pub type LocatedSpan<T> = LocatedSpanEx<T, ()>;

However, this is unnecessary as for some time default type parameters have been allowed — see the relevant page in the Rust book for more information on how these can be used. I have simplified the types by renaming LocatedSpanEx back to LocatedSpan and adding a default type of () for the generic X parameter.

pub struct LocatedSpan<T, X = ()> { ... }

It may be important to allow for backwards compatibility so I have recreated LocatedSpanEx as a type alias of LocatedSpan (which can be removed in a future release). I have marked it with a deprecation warning since all uses can now be replaced with LocatedSpan. The remainder of the codebase, including documentation and testcases, has also been updated to simply use LocatedSpan.

#[deprecated(note = "Use LocatedSpan instead which is exactly the same")]
pub type LocatedSpanEx<T, X> = LocatedSpan<T, X>;

Additionally, on an unrelated note, I have fixed a FIXME issue in the testcases whereby the following line was not working.

assert_eq!(BytesSpan::new(b"foobar").compare(b"foo"), CompareResult::Ok);

The issue is that b"foo" has type &[u8; 3] whereas b"foobar" has type &[u8] as it is implicitly casted due to the type signature of ByteSpan::new. The solution is to perform the same treatment to b"foo", which must be done with an explicit cast.

assert_eq!(BytesSpan::new(b"foobar").compare(b"foo" as &[u8]), CompareResult::Ok);

Please let me know if I can clarify anything or if there are any questions or issues with this 😄

@progval
Copy link
Collaborator

progval commented Feb 11, 2020

Excellent!

We're about to release v2.0.0, so you can even drop LocatedSpanEx and add this to the CHANGELOG.md

@zakcutner
Copy link
Contributor Author

@progval Thanks for responding so fast! These changes are all done now, I've amended the commit as there's probably no need to include the original in the history

@progval
Copy link
Collaborator

progval commented Feb 11, 2020

Thanks!

@progval progval merged commit 7d3054b into fflorent:master Feb 11, 2020
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