-
Notifications
You must be signed in to change notification settings - Fork 13k
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
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:
initial
is declared, and thencur_items
is 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?