Avoid a bogus THIR span for let x = offset_of!(..)#152284
Avoid a bogus THIR span for let x = offset_of!(..)#152284rust-bors[bot] merged 2 commits intorust-lang:mainfrom
let x = offset_of!(..)#152284Conversation
rustbot
added
S-waiting-on-review
|
r? @nnethercote rustbot has assigned @nnethercote. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Zalathar
force-pushed
the
bogus-thir-let
branch
from
b661bbc to
cc3fdc6
Compare
|
I could also imagine making |
|
@bors r+ rollup |
rust-bors
bot
added
S-waiting-on-bors
This comment has been minimized.
This comment has been minimized.
Avoid a bogus THIR span for `let x = offset_of!(..)` The code that creates spans for THIR `let` statements was doing span endpoint manipulation without checking for inclusion/context, resulting in bogus spans observed in #151693. The incorrect spans are easiest to observe with `-Zunpretty=thir-tree`, but they also cause strange user-facing diagnostics for irrefutable let-else.
rust-bors
bot
added
S-waiting-on-review
|
💔 Test for ff65aad failed: CI. Failed job:
|
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
Failure looks Docker-related, so probably bogus. @bors retry |
rust-bors
bot
added
S-waiting-on-bors
| init.span.find_ancestor_inside_same_ctxt(local.span) => | ||
| { | ||
| local.span.with_hi(init_span.hi()) | ||
| } |
There was a problem hiding this comment.
Why not just init.span.to(local.span)?
There was a problem hiding this comment.
Partly because I didn't remember that until existed, but also I think it's doing something subtly different?
This code caps local.span.hi to init_span.hi, but Span::until would cap it to init_span.lo.
Oh I think I misread your suggestion; let me think about it again.
There was a problem hiding this comment.
As I understand it, local.span is the span of the entire let-statement, and init.span is the span of the RHS:
let x = offset_of!(Foo, field) else { return; }
^^^^^^^^^^^^^^^^^^^^^^ init.span
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ local.span
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (what the old+new code is trying to get)
I don't think using Span::until with local.span can ever get the desired result.
There was a problem hiding this comment.
(Honestly local is kind of a bad name for the let statement; I assume it was overlooked by the rename from StmtKind::Local to StmtKind::Let in #122487.)
Rollup merge of #152284 - Zalathar:bogus-thir-let, r=nnethercote Avoid a bogus THIR span for `let x = offset_of!(..)` The code that creates spans for THIR `let` statements was doing span endpoint manipulation without checking for inclusion/context, resulting in bogus spans observed in #151693. The incorrect spans are easiest to observe with `-Zunpretty=thir-tree`, but they also cause strange user-facing diagnostics for irrefutable let-else.
5 participants
The code that creates spans for THIR
letstatements was doing span endpoint manipulation without checking for inclusion/context, resulting in bogus spans observed in #151693.The incorrect spans are easiest to observe with
-Zunpretty=thir-tree, but they also cause strange user-facing diagnostics for irrefutable let-else.