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

More precise spans for temps and their drops #55781

Merged
merged 5 commits into from Nov 15, 2018

Conversation

Projects
None yet
5 participants
@pnkfelix
Member

pnkfelix commented Nov 8, 2018

This PR has two main enhancements:

  1. when possible during code generation for a statement (like expr();), pass along the span of a statement, and then attribute the drops of temporaries from that statement to the statement's end-point (which will be the semicolon if it is a statement that is terminating by a semicolon).
  2. when evaluating a block expression into a MIR temp, use the span of the block's tail expression (rather than the span of whole block including its statements and curly-braces) for the span of the temp.

Each of these individually increases the precision of our diagnostic output; together they combine to make a much clearer picture about the control flow through the spans.

Fix #54382

pnkfelix added some commits Nov 8, 2018

Regression test for issue #54382
(I opted to rely on compare-mode=nll rather than opt into
`#![feature(nll)]`, mostly to make it easy to observe the interesting
differences between the AST-borrwock diagnostic and the NLL one.)
@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Nov 8, 2018

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 8, 2018

This example, by the way, seems to be a particular variant of the kind of code that inspired me to make PR #54782 (cc #54556), which tried to lead the developer to consider adding a semicolon to the end of the inner tail expression.

I am not 100% sure why that suggestion is not being issued here; my best guess is that the is_block_tail field is not being set up in this particular case. Maybe we can add that in a later PR... but its more important right now to bring NLL into parity with AST-borrowck's diagnostics here.

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 8, 2018

(as you can see in my follow up commits, I couldn't resist looking into why the suggestion from PR #54782 was not being applied to this case.)

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 9, 2018

@davidtwco

This comment has been minimized.

Member

davidtwco commented Nov 9, 2018

@bors r+

@bors

This comment has been minimized.

Contributor

bors commented Nov 9, 2018

📌 Commit dd63982 has been approved by davidtwco

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Nov 12, 2018

Rollup merge of rust-lang#55781 - pnkfelix:issue-54382-more-precise-s…
…pans-for-temps-and-their-drops, r=davidtwco

More precise spans for temps and their drops

This PR has two main enhancements:

 1. when possible during code generation for a statement (like `expr();`), pass along the span of a statement, and then attribute the drops of temporaries from that statement to the statement's end-point (which will be the semicolon if it is a statement that is terminating by a semicolon).
 2. when evaluating a block expression into a MIR temp, use the span of the block's tail expression (rather than the span of whole block including its statements and curly-braces) for the span of the temp.

Each of these individually increases the precision of our diagnostic output; together they combine to make a much clearer picture about the control flow through the spans.

Fix rust-lang#54382

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Nov 15, 2018

Rollup merge of rust-lang#55781 - pnkfelix:issue-54382-more-precise-s…
…pans-for-temps-and-their-drops, r=davidtwco

More precise spans for temps and their drops

This PR has two main enhancements:

 1. when possible during code generation for a statement (like `expr();`), pass along the span of a statement, and then attribute the drops of temporaries from that statement to the statement's end-point (which will be the semicolon if it is a statement that is terminating by a semicolon).
 2. when evaluating a block expression into a MIR temp, use the span of the block's tail expression (rather than the span of whole block including its statements and curly-braces) for the span of the temp.

Each of these individually increases the precision of our diagnostic output; together they combine to make a much clearer picture about the control flow through the spans.

Fix rust-lang#54382

bors added a commit that referenced this pull request Nov 15, 2018

Auto merge of #55974 - pietroalbini:rollup, r=pietroalbini
Rollup of 17 pull requests

Successful merges:

 - #55182 (Redox: Update to new changes)
 - #55211 (Add BufWriter::buffer method)
 - #55507 (Add link to std::mem::size_of to size_of intrinsic documentation)
 - #55530 (Speed up String::from_utf16)
 - #55556 (Use `Mmap` to open the rmeta file.)
 - #55622 (NetBSD: link libstd with librt in addition to libpthread)
 - #55750 (Make `NodeId` and `HirLocalId` `newtype_index`)
 - #55778 (Wrap some query results in `Lrc`.)
 - #55781 (More precise spans for temps and their drops)
 - #55785 (Add mem::forget_unsized() for forgetting unsized values)
 - #55852 (Rewrite `...` as `..=` as a `MachineApplicable` 2018 idiom lint)
 - #55865 (Unix RwLock: avoid racy access to write_locked)
 - #55901 (fix various typos in doc comments)
 - #55926 (Change sidebar selector to fix compatibility with docs.rs)
 - #55930 (A handful of hir tweaks)
 - #55932 (core/char: Speed up `to_digit()` for `radix <= 10`)
 - #55956 (add tests for some fixed ICEs)

Failed merges:

r? @ghost

@bors bors merged commit dd63982 into rust-lang:master Nov 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment