Make MatcherPos::stack a SmallVec.#55525
Conversation
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@nnethercote I'm a bit confused, where is that diff ? is that in the PR somewhere/ |
|
@nikomatsakis: this PR contains the diff that triggers the compile error. |
|
So if you apply this PR's diff to your own tree and compile, you'll get the error that I suspect is bogus. |
|
I see |
|
(Trying this now locally) |
|
Hmm, so, I don't think that the borrow checker is wrong but the error reporting is ungreat here (and this isn't my favorite part of the language). I guess This forces us to be more conservative with This is combined with the fact that the enum MatcherPosHandle<'a> {
Ref(&'a mut MatcherPos<'a>),
Box(Box<MatcherPos<'a>>),
}Here, the single parameter So what winds up happening is that .. somehow .. the lifetiime of the borrow winds up extended. Anyway, sorry this explanation isn't that coherent =) I'll dig a bit more. |
|
OK, pushed a fix I believe. |
This comment has been minimized.
This comment has been minimized.
fb1c9eb to
f88f3e3
Compare
|
(Warning: force pushed over my commit to resolve tidy failure) |
|
Goodness! Thank you for figuring that out, I never would have. |
|
@nikomatsakis: is it reasonable to push this as-is? Or should I squash the two patches together? |
This avoids some allocations.
f88f3e3 to
68e76dc
Compare
|
@nikomatsakis: I merged the patches and put you as the author. |
|
@bors r+ |
|
📌 Commit 68e76dc has been approved by |
…ercote
Make `MatcherPos::stack` a `SmallVec`.
This avoids some allocations.
This seems like a trivial change, but the compiler rejects it:
```
Compiling syntax v0.0.0 (/home/njn/moz/rust1/src/libsyntax)
error[E0597]: `initial` does not live long enough=========> ] 89/110: syntax
--> libsyntax/ext/tt/macro_parser.rs:647:57
|
647 | let mut cur_items = smallvec![MatcherPosHandle::Ref(&mut initial)];
| ^^^^^^^^^^^^ borrowed value does not live long enough
...
762 | }
| -
| |
| `initial` dropped here while still borrowed
| borrow later used here, when `initial` is dropped
error: aborting due to previous error
```
This is either a compiler bug, or there's some subtle thing I don't understand. The lifetimes sure seem straightforward: `initial` is declared, and then `cur_items` is declared immediately afterward, and it uses a reference to `initial`. The error message makes it sound like the compiler is dropping the variables in the wrong order.
r? @nikomatsakis, any idea what the problem is?
|
☀️ Test successful - status-appveyor, status-travis |
This avoids some allocations.
This seems like a trivial change, but the compiler rejects it:
This is either a compiler bug, or there's some subtle thing I don't understand. The lifetimes sure seem straightforward:
initialis declared, and thencur_itemsis declared immediately afterward, and it uses a reference toinitial. The error message makes it sound like the compiler is dropping the variables in the wrong order.r? @nikomatsakis, any idea what the problem is?