syntax/hir: give loop labels a span #33351
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
This is a libsyntax [breaking-change], right? In that case, cc #31645 and cc @Manishearth, not sure how big the impact would be. |
|
I suppose so, yes. There might also be a reason for these not to have a span, I don't know :) |
|
Nah, lots of things don't have a span just because it's not necessary. But yeah, to whoever reviews this: please just write r=whatever without mentioning bors, don't actually approve it. This will be merged when we have sufficient other libsyntax breaking changes. |
|
So I could help getting this merged by breaking libsyntax in other ways? 😜 |
|
lol. Make a change large enough that it's going to need rebases every day and I'm fine with batching up now, otherwise we really should wait. |
|
@Manishearth isn't Niko's diagnostics change pretty breaking? At least it will break clippy (cf PR there). |
|
[breaking-batch] applies to libsyntax public APIs and semantics only, since that breaks syntax extensions and the serde/aster/quasi stuff -- this leads to a disturbance in the Force, as if millions of lockfiles suddenly cried out in terror. On the other hand, clippy doesn't break dependencies since people don't use it as a mandatory dependency (or shouldn't, at any rate), so it's just a matter of ignoring clippy failures in travis for a day. Problematic, but less so. Besides, clippy depends on so many random-and-totally-unstable bits of the compiler that we'd be freezing too much. OTOH it's pretty okay if libsyntax's API is somewhat frozen, since it's mostly not going to change anyway. |
|
☔ The latest upstream changes (presumably #33443) made this pull request unmergeable. Please resolve the merge conflicts. |
birkenfeld
force-pushed
the
loop-label-spans
branch
from
8d0c655
to
436e2cd
Compare
| visitor.visit_expr(subexpression); | ||
| visitor.visit_block(block); | ||
| walk_opt_ident(visitor, expression.span, opt_ident) | ||
| for sp_ident in opt_sp_ident { |
There was a problem hiding this comment.
(I'd personally prefer if let Some(sp_ident) to for here, but I won't block this PR on that nit.)
There was a problem hiding this comment.
That's how I'd write it too, I kept it consistent with existing code (line 826).
|
@Manishearth r=me |
|
☔ The latest upstream changes (presumably #33532) made this pull request unmergeable. Please resolve the merge conflicts. |
birkenfeld
force-pushed
the
loop-label-spans
branch
from
436e2cd
to
b90d0e7
Compare
|
☔ The latest upstream changes (presumably #33654) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@Manishearth please let me know when a batch is about to land. I don't want to rebase for no reason. |
|
Could you rebase this? The batch will happen soon |
This makes the "shadowing labels" warning *not* print the entire loop as a span, but only the lifetime. Also makes rust-lang#31719 go away, but does not fix its root cause (the span of the expanded loop is still wonky, but not used anymore).
birkenfeld
force-pushed
the
loop-label-spans
branch
from
b90d0e7
to
2e812e1
Compare
…elix This makes the \"shadowing labels\" warning *not* print the entire loop as a span, but only the lifetime. Also makes rust-lang#31719 go away, but does not fix its root cause (the span of the expanded loop is still wonky, but not used anymore).
…elix This makes the \"shadowing labels\" warning *not* print the entire loop as a span, but only the lifetime. Also makes rust-lang#31719 go away, but does not fix its root cause (the span of the expanded loop is still wonky, but not used anymore).
This makes the "shadowing labels" warning not print the entire loop as a span, but only the lifetime.
Also makes #31719 go away, but does not fix its root cause (the span of the expanded loop is still wonky, but not used anymore).