use structured suggestion for "missing mut" label #54157

euclio · 2018-09-12T18:19:23Z

Fixes #54133 for both NLL and non-NLL.

I'm not super happy with the existing wording here, since it's now a suggestion. I wonder if the message would work better as something like "help: make binding mutable: mut foo"?

Also, are the HELP and SUGGESTION comments necessary?

estebank

Also, are the HELP and SUGGESTION comments necessary?

They are. Test files' annotation with ERROR is always mandatory, and if there are any NOTE/HELP annotation, all of them need to be added. These comments are a way of making sure that the important messages being tested are looked at, while the stderr files let us see and catch any visual regressions that wouldn't be noticed if we only looked at the raw text.

r=me with the nits resolved

estebank · 2018-09-12T18:28:42Z

src/librustc_borrowck/borrowck/mod.rs

                        let_span,
-                        format!("consider changing this to `mut {}`", snippet),
+                        "consider changing this to",
+                        format!("mut {}", snippet),
                    );


I'm not super happy with the existing wording here, since it's now a suggestion.

What about "make this binding mutable":

LL | let x: isize; | - help: make this binding mutable: `mut x` LL | x = 1; | ----- first assignment to `x`

estebank · 2018-09-12T18:29:56Z

src/librustc_borrowck/borrowck/mod.rs

@@ -1299,9 +1299,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                        snippet
                    );
                } else {
-                    db.span_label(
+                    db.span_suggestion(


Use span_suggestion_with_applicability and Applicability::MachineApplicable, so that rustfix and VScode can apply this automagically.

estebank · 2018-09-12T18:30:04Z

src/librustc_mir/borrow_check/error_reporting.rs

                        decl.source_info.span,
-                        format!("consider changing this to `mut {}`", name),
+                        "consider changing this to",
+                        format!("mut {}", name),
                    );


Use span_suggestion_with_applicability and Applicability::MachineApplicable, so that rustfix and VScode can apply this automagically.

Fixes rust-lang#54133.

euclio · 2018-09-12T21:17:26Z

Addressed nits.

estebank · 2018-09-12T22:49:13Z

@bors r+

bors · 2018-09-12T22:49:14Z

📌 Commit d871b8a has been approved by estebank

bors · 2018-09-15T05:18:45Z

⌛ Testing commit d871b8a with merge b7f9284df0619182adfbb9d855232c730d64db17...

bors · 2018-09-15T06:36:18Z

💔 Test failed - status-travis

rust-highfive · 2018-09-15T06:36:19Z

The job dist-mipsel-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:003a82ef:start=1536993300124009086,finish=1536993300128852153,duration=4843067
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:205d1120
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:049c7555
travis_time:start:049c7555
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0322e954
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

kennytm · 2018-09-15T07:42:44Z

@bors retry travis-ci/travis-ci#9696

bors · 2018-09-16T09:47:12Z

⌛ Testing commit d871b8a with merge 32dc5a0...

@estebank

use structured suggestion for "missing mut" label Fixes #54133 for both NLL and non-NLL. r? @estebank I'm not super happy with the existing wording here, since it's now a suggestion. I wonder if the message would work better as something like "help: make binding mutable: `mut foo`"? Also, are the `HELP` and `SUGGESTION` comments necessary?

bors · 2018-09-16T12:13:31Z

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 32dc5a0 to master...

rust-highfive assigned estebank Sep 12, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 12, 2018

estebank approved these changes Sep 12, 2018

View reviewed changes

estebank added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 12, 2018

use structured suggestion for "missing mut" label

d871b8a

Fixes rust-lang#54133.

euclio force-pushed the structured-suggestion branch from c173cdb to d871b8a Compare September 12, 2018 21:17

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 12, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 15, 2018

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 Sep 15, 2018

bors merged commit d871b8a into rust-lang:master Sep 16, 2018

This was referenced Sep 16, 2018

[nll] borrows that must be valid for a free lifetime should explain why #54229

Merged

Remove usages of span_suggestion without Applicability #54241

Merged

euclio deleted the structured-suggestion branch February 7, 2019 18:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use structured suggestion for "missing mut" label #54157

use structured suggestion for "missing mut" label #54157

euclio commented Sep 12, 2018 •

edited

Loading

estebank left a comment

estebank Sep 12, 2018

estebank Sep 12, 2018

estebank Sep 12, 2018

euclio commented Sep 12, 2018

estebank commented Sep 12, 2018

bors commented Sep 12, 2018

bors commented Sep 15, 2018

bors commented Sep 15, 2018

rust-highfive commented Sep 15, 2018

kennytm commented Sep 15, 2018

bors commented Sep 16, 2018

bors commented Sep 16, 2018

use structured suggestion for "missing mut" label #54157

use structured suggestion for "missing mut" label #54157

Conversation

euclio commented Sep 12, 2018 • edited Loading

estebank left a comment

Choose a reason for hiding this comment

estebank Sep 12, 2018

Choose a reason for hiding this comment

estebank Sep 12, 2018

Choose a reason for hiding this comment

estebank Sep 12, 2018

Choose a reason for hiding this comment

euclio commented Sep 12, 2018

estebank commented Sep 12, 2018

bors commented Sep 12, 2018

bors commented Sep 15, 2018

bors commented Sep 15, 2018

rust-highfive commented Sep 15, 2018

kennytm commented Sep 15, 2018

bors commented Sep 16, 2018

bors commented Sep 16, 2018

euclio commented Sep 12, 2018 •

edited

Loading