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

Allow bench_refs() and bench_local_refs() to capture input lifetimes #54

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

neocturne
Copy link

This allows benchmarks using bench_refs() and bench_local_refs() to capture the input lifetimes, as was already suggested by TODO comments in the code.

By naming the input lifetime, the output type O is allowed to capture this lifetime. The I: 'i condition for &'i mut I must be made explicit now with named 'i, or the code won't compile.

First commit adds tests that don't compile, second commit makes them compile.

Copy link
Owner

@nvzqz nvzqz left a comment

Choose a reason for hiding this comment

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

Thanks for this change!

Some suggestions regarding testing:

  • I like the internal tests with Capturing and I think it would make sense to have it capture &'a mut () to restrict it based on the parameter.
  • Let's include an example benchmark using this feature.

Prepare tests that currently fail to compile, showing that bench outputs
can't capture the input lifetime.
…etimes

By naming the input lifetime, the output type `O` is allowed to capture
this lifetime. The `I: 'i` condition for `&'i mut I` must be made
explicit now with named `'i`, or the code won't compile.

This makes all Capturing tests added in the previous commit compile.
@neocturne
Copy link
Author

Any suggestions for a good example? In my project I'm using it for benchmarking a parser that creates a token stream/tree which references the input, but that seems too complex for an example (and it uses a separate parser crate). I guess I could do something simple like collecting the result of str::matches() in a Vec that gets returned?

Add a simple example that shows that bench_refs/bench_local_refs input
lifetimes may be captured now.
@neocturne
Copy link
Author

I have updated the PR with the suggested changes. I'm not really happy with the example, but I've added a comment to explain what it is supposed to demonstrate.

@neocturne
Copy link
Author

Any further comments?

@neocturne
Copy link
Author

@nvzqz Is there anything else you'd like me to change in this PR?

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