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

Implement Display for LocatedSpanEx #40

Merged
merged 1 commit into from Oct 16, 2019

Conversation

ebkalderon
Copy link
Contributor

Changed

  • Replace explicit ToString implementation for LocatedSpanEx with Display.

This should make nom_locate more ergonomic to use with Criterion. Note that the type annotation T is constrained here by ToString instead of Display so we can support Display for LocatedSpanEx even for types that do not implement Display, but do implement ToString.

Fixes #39.

The type annotation `T` is constrained here by `ToString` instead of
`Display` so we can support `Display` for `LocatedSpanEx` even for types
that do not implement `Display`, but do implement `ToString`.
@progval
Copy link
Collaborator

progval commented Oct 15, 2019

This adds two extra copies when using ToString. Do you know if they get optimized out?

@ebkalderon
Copy link
Contributor Author

@progval Hard to say. There are exactly two occurrences of memcpy in both versions, according to Compiler Explorer, with most of the assembly differences being the addition of the generated std::fmt symbol boilerplate. No additional calls appear to have been introduced, AFAICT. Perhaps you might be able to spot a meaningful difference?

@progval
Copy link
Collaborator

progval commented Oct 16, 2019

Perfect, thanks!

@progval progval merged commit c61a971 into fflorent:master Oct 16, 2019
@ebkalderon ebkalderon deleted the impl-display-for-locatedspanex branch October 18, 2019 06:00
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.

Implement Display for LocatedSpanEx
2 participants