NLL: On "cannot move out of type" error, print original before rewrite by pnkfelix · Pull Request #52907 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation3 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

NLL: On "cannot move out of type" error, print original source before rewrite.

Arguably this change is sometimes injecting noise into the output (namely in the cases where the suggested rewrite is inline with the suggestion and we end up highlighting the original source code). I would not be opposed to something more aggressive/dynamic, like revising the suggestion code to automatically print the original source when necessary (e.g. when the error does not have a span that includes the span of the suggestion).
Also, as another note on this change: The doc comment for Diagnostic::span_suggestion says:

/// The message /// /// * should not end in any punctuation (a : is added automatically) /// * should not be a question /// * should not contain any parts like "the following", "as shown"
but the : is not added when the emitted line appears out-of-line relative to the suggestion. I find that to be an unfortunate UI experience.

As a drive-by fix, also changed code to combine multiple suggestions for a pattern into a single multipart suggestion (which vastly improves user experience IMO).

Includes the updates to expected NLL diagnostics.

Fix #52877

… rewrite.

Arguably this change is sometimes injecting noise into the output (namely in the cases where the suggested rewrite is inline with the suggestion and we end up highlighting the original source code). I would not be opposed to something more aggressive/dynamic, like revising the suggestion code to automatically print the original source when necessary (e.g. when the error does not have a span that includes the span of the suggestion).
Also, as another note on this change: The doc comment for Diagnostic::span_suggestion says: /// The message /// /// * should not end in any punctuation (a : is added automatically) /// * should not be a question /// * should not contain any parts like "the following", "as shown"

but the : is not added when the emitted line appears out-of-line relative to the suggestion. I find that to be an unfortunate UI experience.

As a drive-by fix, also changed code to combine multiple suggestions for a pattern into a single multipart suggestion (which vastly improves user experience IMO).

Includes the updates to expected NLL diagnostics.

r? @petrochenkov

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

📌 Commit a5a3f76 has been approved by petrochenkov

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jul 31, 2018

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request

Aug 1, 2018

…e-should-precede-suggestions, r=petrochenkov

NLL: On "cannot move out of type" error, print original before rewrite

NLL: On "cannot move out of type" error, print original source before rewrite.

Arguably this change is sometimes injecting noise into the output (namely in the cases where the suggested rewrite is inline with the suggestion and we end up highlighting the original source code). I would not be opposed to something more aggressive/dynamic, like revising the suggestion code to automatically print the original source when necessary (e.g. when the error does not have a span that includes the span of the suggestion).
Also, as another note on this change: The doc comment for Diagnostic::span_suggestion says:

    /// The message
    ///
    /// * should not end in any punctuation (a `:` is added automatically)
    /// * should not be a question
    /// * should not contain any parts like "the following", "as shown"

but the : is not added when the emitted line appears out-of-line relative to the suggestion. I find that to be an unfortunate UI experience.

As a drive-by fix, also changed code to combine multiple suggestions for a pattern into a single multipart suggestion (which vastly improves user experience IMO).

Includes the updates to expected NLL diagnostics.

Fix rust-lang#52877

bors added a commit that referenced this pull request

Aug 1, 2018

Rollup of 31 pull requests

Successful merges:

#52332 (dead-code lint: say "constructed" for structs)
#52340 (Document From trait implementations for OsStr, OsString, CString, and CStr)
#52628 (Cleanup some rustdoc code)
#52732 (Remove unstable and deprecated APIs)
#52745 (Update clippy to latest master)
#52756 (rustc: Disallow machine applicability in foreign macros)
#52771 (Clarify thread::park semantics)
#52810 ([NLL] Don't make "fake" match variables mutable)
#52821 (pretty print for std::collections::vecdeque)
#52822 (Fix From)
#52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper)
#52831 (remove references to AUTHORS.txt file)
#52835 (Fix Alias intra doc ICE)
#52842 (update comment)
#52846 (Add timeout to use of curl in bootstrap.py.)
#52851 (Make the tool_lints actually usable)
#52853 (Improve bootstrap help on stages)
#52859 (Use Vec::extend in SmallVec::extend when applicable)
#52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.)
#52867 (releases.md: fix 2 typos)
#52870 (Implement Unpin for FutureObj and LocalFutureObj)
#52876 (run-pass/const-endianness: negate before to_le())
#52878 (Fix wrong issue number in the test name)
#52883 (Include lifetime in mutability suggestion in NLL messages)
#52904 (NLL: sort diagnostics by span)
#52905 (Fix a typo in unsize.rs)
#52907 (NLL: On "cannot move out of type" error, print original before rewrite)
#52908 (Use SetLenOnDrop in Vec::truncate())
#52914 (Only run the sparc-abi test on sparc)
#52918 (Backport 1.27.2 release notes)
#52929 (Update compatibility note for 1.28.0 to be correct)

Failed merges:

#52758 (Cleanup for librustc::session)
#52799 (Use BitVector for global sets of AttrId)

r? @ghost

pietroalbini added a commit to pietroalbini/rust that referenced this pull request

Aug 1, 2018

…e-should-precede-suggestions, r=petrochenkov

NLL: On "cannot move out of type" error, print original before rewrite

NLL: On "cannot move out of type" error, print original source before rewrite.

Arguably this change is sometimes injecting noise into the output (namely in the cases where the suggested rewrite is inline with the suggestion and we end up highlighting the original source code). I would not be opposed to something more aggressive/dynamic, like revising the suggestion code to automatically print the original source when necessary (e.g. when the error does not have a span that includes the span of the suggestion).
Also, as another note on this change: The doc comment for Diagnostic::span_suggestion says:

    /// The message
    ///
    /// * should not end in any punctuation (a `:` is added automatically)
    /// * should not be a question
    /// * should not contain any parts like "the following", "as shown"

but the : is not added when the emitted line appears out-of-line relative to the suggestion. I find that to be an unfortunate UI experience.

As a drive-by fix, also changed code to combine multiple suggestions for a pattern into a single multipart suggestion (which vastly improves user experience IMO).

Includes the updates to expected NLL diagnostics.

Fix rust-lang#52877

bors added a commit that referenced this pull request

Aug 1, 2018

Rollup of 30 pull requests

Successful merges:

#52340 (Document From trait implementations for OsStr, OsString, CString, and CStr)
#52628 (Cleanup some rustdoc code)
#52732 (Remove unstable and deprecated APIs)
#52745 (Update clippy to latest master)
#52771 (Clarify thread::park semantics)
#52778 (Improve readability of serialize.rs)
#52810 ([NLL] Don't make "fake" match variables mutable)
#52821 (pretty print for std::collections::vecdeque)
#52822 (Fix From)
#52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper)
#52825 (Make sure #47772 does not regress)
#52831 (remove references to AUTHORS.txt file)
#52842 (update comment)
#52846 (Add timeout to use of curl in bootstrap.py.)
#52851 (Make the tool_lints actually usable)
#52853 (Improve bootstrap help on stages)
#52859 (Use Vec::extend in SmallVec::extend when applicable)
#52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.)
#52867 (releases.md: fix 2 typos)
#52870 (Implement Unpin for FutureObj and LocalFutureObj)
#52876 (run-pass/const-endianness: negate before to_le())
#52878 (Fix wrong issue number in the test name)
#52883 (Include lifetime in mutability suggestion in NLL messages)
#52888 (Use suggestions for shell format arguments)
#52904 (NLL: sort diagnostics by span)
#52905 (Fix a typo in unsize.rs)
#52907 (NLL: On "cannot move out of type" error, print original before rewrite)
#52914 (Only run the sparc-abi test on sparc)
#52918 (Backport 1.27.2 release notes)
#52929 (Update compatibility note for 1.28.0 to be correct)

Failed merges:

r? @ghost

Labels

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.