-
Notifications
You must be signed in to change notification settings - Fork 23
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
Incompatibility with functions create more &mut self
.
#13
Comments
Rust should be able to understand non-overlapping borrows. I need to test but I believe However The latter is a tricky problem to solve if we keep the metrics registry in |
Hmm yeah, I could try to play around with it a bit to isolate it, but it seemed to have issue with just taking a mutable reference to other fields. It's a little unfortunate, I have a already implemented a fairly complex system, and am trying to add some metering to specific critical sections, so I'd prefer not to rewrite/wrap everything in order just to appease the borrow checker. :( A question: Why does |
No it's not doing anything with drop or custom structs, it's really just wrapping the calls with callbacks (e.g before/after the metric) and needs to borrow the metric for the duration of the call to do so. I explored several designs before and this was at the time the most convincing one to keep the generated code reasonably vanilla Rust, not store mutable state in a static global, and be compatible with blocking and async code. Documentation should contain an example of the pseudo-code that metered generates (using the
If you can reduce a test case to demonstrate this, that should be something we can fix in metered. |
I may have been mistaken with the non-overlapping borrows. I think I forgot to comment out a lambda that was causing the issue. 🤦♂ So I've been looking through the source and I feel like the issue with mut receivers can be fixed relatively easily:
expands to:
It just looks like the only issue is that the calls to If the
then:
expands to:
And you don't have to worry about conflicting borrows across the result block. 🎉 It would be a breaking change for anyone using I also noticed that at |
I think your solution was the original code and aliasing the metric a workaround at some point, but I don't remember the details. I would be open to test your change on my projects / test cases and if it works then merge it. |
Ok, I've opened a PR so you can give it a try: #14. I would be curious to see a case where it doesn't work and, if so, how it can worked around. |
So far the approach suggested uses |
Just to confirm, If I have a function that ever takes a
&mut self.foo
or calls a method with a receiverself.bar(&mut self)
, then everything just breaks?It complains about reused borrows. Seems really limiting that I can only use the proc macros to measure things in functions that take
self
but then never call out to anything taking&mut self
, etc..:( the only other alternative being to use the
measure!
macro around specific blocks, and then creating the Metric container manually I guess? But that still doesn't work if my section under test calls another function with a mutableself
?The text was updated successfully, but these errors were encountered: