internal: Avoid temporary Vec in MBE
#11475
Conversation
crates/mbe/src/expander/matcher.rs
Outdated
| } | ||
|
|
||
| fn collect_vars(buf: &mut Vec<SmolStr>, pattern: &MetaTemplate) { | ||
| fn collect_vars<F: FnMut(SmolStr)>(collector_fun: &mut F, pattern: &MetaTemplate) { |
There was a problem hiding this comment.
no need to pass the closure by mut ref here if I see that right
bors d+
There was a problem hiding this comment.
rustc seems to disagree or maybe I miss something. I need a mut ref because the closure's call need a &mut self
There was a problem hiding this comment.
mut collector_fun: F should work
Calling the closure requires &mut F, if you pass ownership the name needs to be mutable for rust to be able to borrow mutably when calling it
|
✌️ bellau can now approve this pull request. To approve and merge a pull request, simply reply with |
crates/mbe/src/expander/matcher.rs
Outdated
| } | ||
|
|
||
| fn collect_vars(buf: &mut Vec<SmolStr>, pattern: &MetaTemplate) { | ||
| fn collect_vars<F: FnMut(SmolStr)>(collector_fun: &mut F, pattern: &MetaTemplate) { |
There was a problem hiding this comment.
is it better to use this notation ?
fn collect_vars(collector_fun: &mut impl FnMut(SmolStr), pattern: &MetaTemplate)
There was a problem hiding this comment.
either works, we don't really have any style guidelines on this I think. I personally would probably prefer the impl notation here
bellau
force-pushed
the
impr-remove-unecessary-temp-buffer
branch
from
b790cf7 to
ff4024e
Compare
|
bors r+ |
lnicola
changed the title
Vec in MBE
2 participants
It's a micro optimization. I don't know if it's really necessary (less alloc is always better).