Fix wrong test replacemen in `ref mut` suggestion #51249

csmoe · 2018-05-31T14:35:06Z

rust-highfive · 2018-05-31T14:35:17Z

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

killercup · 2018-06-01T01:13:43Z

src/librustc_borrowck/borrowck/mod.rs

@@ -1213,7 +1213,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                            db.span_label(
                                let_span,
                                format!("consider changing this to `{}`",
-                                         snippet.replace("ref ", "ref mut "))
+                                         snippet.replacen("ref ", "ref mut ", 1))


This is (a) not requiring the ref to be at the start of the span (this might be problematic) and (b) only matching the first space after it (this is okay if we want to preserve weird formatting). This might be fine for all inputs we consider now, but matching "^ref " seems safer in general.

csmoe · 2018-06-01T07:49:53Z

the already-n-lint introduced here #44614 (review)

TimNN · 2018-06-12T17:35:57Z

Friendly triage ping @pnkfelix / @rust-lang/compiler, it looks like this PR needs your review.

oli-obk · 2018-06-12T17:46:53Z

src/test/ui/suggestions/issue-51244.nll.stderr

+  --> $DIR/issue-51244.rs:13:5
+   |
+LL |     let ref my_ref @ _ = 0;
+   |         -------------- help: consider changing this to be a mutable reference: `&mut ef my_ref @ _`


This suggestion is wrong

Blocked by #51612

@pietroalbini block occurs here.

estebank · 2018-06-12T18:12:40Z

src/librustc_borrowck/borrowck/mod.rs

+                                snippet.replacen("ref ", "ref mut ", 1)
+                            } else {
+                                snippet
+                            };
                            db.span_label(


Could we change this to be a span_suggestion? That way editors will know they can perform the replacement and the following code:

let replace_str = if snippet.starts_with("ref ") { snippet.replacen("ref ", "ref mut ", 1) } else { snippet }; db.span_label( let_span, "use a mutable reference instead", replace_str, );

output would look like this:

error[E0594]: cannot assign to immutable borrowed content `*my_ref` --> $DIR/issue-51244.rs:13:5 | LL | let ref my_ref @ _ = 0; | -------------- use a mutable reference instead: `ref mut my_ref @ _` LL | *my_ref = 0; | ^^^^^^^^^^^ cannot borrow as mutable

This will likely require you to run ./x.py test src/test/ui --bless to update the output in other test files too.

If you do not wish to make this change now, please file a ticket to follow up on all suggestions on this file, so that the team doesn't forget to address this later.

estebank · 2018-06-15T16:05:36Z

Use ./x.py test src/test/ui --stage 1 --bless to update the stderr files we compare against, as other tests are affected by the change.

csmoe · 2018-06-15T23:01:16Z

@estebank I did bless but didn't commit the stderr-updates expect the certain test for this issue(ref mut), since the other suggestions is really strange, for example(snippet from the ci error above):

[00:52:06] 4 LL |     let Wrap(x) = &Wrap(3);
[00:52:06] -    |              - consider changing this to `x`
[00:52:06] +    |              - help: use a mutable reference instead: `x`
[00:52:06] 6 LL |     *x += 1; //~ ERROR cannot assign to immutable
[00:52:06] 7    |     ^^^^^^^ cannot borrow as mutable

In the origin suggestion: changing to x, changes nothing since the span is already x.

[00:52:06] 18   --> $DIR/enum.rs:29:9
[00:52:06] 19    |
[00:52:06] 19    |
[00:52:06] 20 LL |     while let Some(x) = &Some(3) {
[00:52:06] -    |                    - consider changing this to `x`
[00:52:06] +    |                    - help: use a mutable reference instead: `x`
[00:52:06] 22 LL |         *x += 1; //~ ERROR cannot assign to immutable
[00:52:06] 23    |         ^^^^^^^ cannot borrow as mutable

And this time, the span(() is totally wrong.

estebank · 2018-06-15T23:05:26Z

You're right, although I would say that that is probably a preexisting problem (the current suggestion is already wrong). It needs to be fixed, but it doesn't seem to be a regression caused by your code.

rust-highfive · 2018-06-22T14:56:56Z

The job x86_64-gnu-llvm-3.9 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.

[00:50:18] ....................................................................................................
[00:50:23] ....................................................................................................
[00:50:29] ....................................................................................................
[00:50:35] ....................................................................................................
[00:50:41] ........i....................................................F...........................i..........
[00:50:53] ....................................................................................................
[00:50:59] ....................................................................................................
[00:51:06] ...........................i........................................................................
[00:51:07] ............
[00:51:07] ............
[00:51:07] failures:
[00:51:07] 
[00:51:07] ---- [ui] ui/nll/issue-51244.rs stdout ----
[00:51:07] diff of stderr:
[00:51:07] 
[00:51:07] - error[E0594]: cannot assign to data in a `&` reference
[00:51:07] + error[E0594]: cannot assign to `*my_ref` which is behind a `&` reference
[00:51:07] 2   --> $DIR/issue-51244.rs:15:5
[00:51:07] 3    |
[00:51:07] - LL |     let ref my_ref @ _ = 0;
[00:51:07] -    |         -------------- help: consider changing this to be a mutable reference: `&mut ef my_ref @ _`
[00:51:07] 6 LL |     *my_ref = 0; //~ ERROR cannot assign to data in a `&` reference [E0594]
[00:51:07] +    |     ^^^^^^^^^^^ cannot assign
[00:51:07] 8 
[00:51:07] 9 error: aborting due to previous error
[00:51:07] 10 
[00:51:07] 10 
[00:51:07] 
[00:51:07] 
[00:51:07] The actual stderr differed from the expected stderr.
[00:51:07] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/nll/issue-51244/issue-51244.stderr
[00:51:07] To update references, rerun the tests and pass the `--bless` flag
[00:51:07] To only update this specific test, also pass `--test-args nll/issue-51244.rs`
[00:51:07] error: 1 errors occurred comparing output.
[00:51:07] status: exit code: 101
[00:51:07] status: exit code: 101
[00:51:07] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/nll/issue-51244.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/nll/issue-51244/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/nll/issue-51244/auxiliary" "-A" "unused"
[00:51:07] ------------------------------------------
[00:51:07] 
[00:51:07] ------------------------------------------
[00:51:07] stderr:
[00:51:07] stderr:
[00:51:07] ------------------------------------------
[00:51:07] {"message":"cannot assign to `*my_ref` which is behind a `&` reference","code":{"code":"E0594","explanation":null},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/nll/issue-51244.rs","byte_start":529,"byte_end":540,"line_start":15,"line_end":15,"column_start":5,"column_end":16,"is_primary":true,"text":[{"text":"    *my_ref = 0; //~ ERROR cannot assign to data in a `&` reference [E0594]","highlight_start":5,"highlight_end":16}],"label":"cannot assign","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error[E0594]: cannot assign to `*my_ref` which is behind a `&` reference\n  --> /checkout/src/test/ui/nll/issue-51244.rs:15:5\n   |\nLL |     *my_ref = 0; //~ ERROR cannot assign to data in a `&` reference [E0594]\n   |     ^^^^^^^^^^^ cannot assign\n\n"}
[00:51:07] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:51:07] {"message":"For more information about this error, try `rustc --explain E0594`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0594`.\n"}
[00:51:07] ------------------------------------------
[00:51:07] 
[00:51:07] thread '[ui] ui/nll/issue-51244.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3139:9
[00:51:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:51:07] 
[00:51:07] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:51:07] 
[00:51:07] 
[00:51:07] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:51:07] 
[00:51:07] 
[00:51:07] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:51:07] Build completed unsuccessfully in 0:02:18
[00:51:07] Build completed unsuccessfully in 0:02:18
[00:51:07] Makefile:58: recipe for target 'check' failed
[00:51:07] make: *** [check] Error 1
2429396 ./obj
2429364 ./obj/build
1825588 ./obj/build/x86_64-unknown-linux-gnu
729072 ./src
---
143560 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu
143556 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release
138680 ./obj/build/bootstrap/debug/incremental
124108 ./obj/build/bootstrap/debug/incremental/bootstrap-1r3bppl29tbrj
124104 ./obj/build/bootstrap/debug/incremental/bootstrap-1r3bppl29tbrj/s-f283ofp042-upibm1-368y0icpkxgcv
107604 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release
103608 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends
102856 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu
102852 ./obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release
---
travis_time:end:00443d51:start=1529679413430751523,finish=1529679413439523066,duration=8771543
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:10b10baa
$ head -30 ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
head: cannot open ‘./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers’ for reading: No such file or directory
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:31beefc2
$ 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)

stokhos · 2018-06-30T21:58:52Z

ping from triage @csmoe will you have time to fix the code?

pietroalbini · 2018-07-09T10:48:31Z

@csmoe this is blocked on what?

NLL: Suggest `ref mut` and `&mut self` Fixes rust-lang#51244. Supersedes rust-lang#51249, I think. Under the old lexical lifetimes, the compiler provided helpful suggestions about adding `mut` when you tried to mutate a variable bound as `&self` or (explicit) `ref`. NLL doesn't have those suggestions yet. This pull request adds them. I didn't bother making the help text exactly the same as without NLL, but I can if that's important. (Originally this was supposed to be part of rust-lang#51612, but I got bogged down trying to fit everything in one PR.)

NLL: Suggest `ref mut` and `&mut self` Fixes #51244. Supersedes #51249, I think. Under the old lexical lifetimes, the compiler provided helpful suggestions about adding `mut` when you tried to mutate a variable bound as `&self` or (explicit) `ref`. NLL doesn't have those suggestions yet. This pull request adds them. I didn't bother making the help text exactly the same as without NLL, but I can if that's important. (Originally this was supposed to be part of #51612, but I got bogged down trying to fit everything in one PR.)

bors · 2018-07-13T19:51:00Z

☔ The latest upstream changes (presumably #52242) made this pull request unmergeable. Please resolve the merge conflicts.

csmoe · 2018-07-13T22:36:35Z

Fixed #52242

rust-highfive assigned pnkfelix May 31, 2018

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

csmoe force-pushed the ref_mut branch from ef4e36e to ca536cf Compare May 31, 2018 14:38

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch 2 times, most recently from 35ba512 to fb30eb6 Compare May 31, 2018 15:01

This comment has been minimized.

Sign in to view

killercup reviewed Jun 1, 2018

View reviewed changes

csmoe force-pushed the ref_mut branch 2 times, most recently from ce7540a to cc50347 Compare June 1, 2018 04:20

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from cc50347 to fd19175 Compare June 1, 2018 07:56

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from 08ee7eb to 3c43aa6 Compare June 1, 2018 12:01

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from b38735c to 1cfae6c Compare June 1, 2018 13:14

This comment has been minimized.

Sign in to view

killercup mentioned this pull request Jun 1, 2018

Automatically mark all but the last comment as resolved rust-lang/rust-log-analyzer#19

Open

csmoe force-pushed the ref_mut branch from 1cfae6c to b0dd84b Compare June 3, 2018 03:59

This comment has been minimized.

Sign in to view

oli-obk requested changes Jun 12, 2018

View reviewed changes

oli-obk assigned oli-obk and unassigned pnkfelix Jun 12, 2018

estebank reviewed Jun 12, 2018

View reviewed changes

csmoe added 2 commits June 13, 2018 22:42

fix wrong replacing

3833e60

update test

9d42c80

oli-obk removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2018

csmoe force-pushed the ref_mut branch from 4904bfc to 1cb895c Compare June 14, 2018 00:01

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from 1cb895c to b0bc50f Compare June 15, 2018 06:48

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from b0bc50f to 79b8e54 Compare June 17, 2018 09:05

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from 79b8e54 to 3259a45 Compare June 22, 2018 09:43

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from 3259a45 to 81c6c22 Compare June 22, 2018 11:19

This comment has been minimized.

Sign in to view

csmoe force-pushed the ref_mut branch from 81c6c22 to 2c386a1 Compare June 22, 2018 12:44

This comment has been minimized.

Sign in to view

span_suggestion

43cc08b

csmoe force-pushed the ref_mut branch from 2c386a1 to 43cc08b Compare June 22, 2018 14:03

csmoe added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jul 1, 2018

pietroalbini removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 9, 2018

csmoe mentioned this pull request Jul 12, 2018

NLL: Suggest ref mut and &mut self #52242

Merged

csmoe closed this Jul 13, 2018

csmoe deleted the ref_mut branch July 26, 2018 13:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix wrong test replacemen in `ref mut` suggestion #51249

Fix wrong test replacemen in `ref mut` suggestion #51249

csmoe commented May 31, 2018

rust-highfive commented May 31, 2018

This comment has been minimized.

This comment has been minimized.

killercup Jun 1, 2018 •

edited

Loading

This comment has been minimized.

csmoe commented Jun 1, 2018 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

TimNN commented Jun 12, 2018

oli-obk Jun 12, 2018

csmoe Jun 22, 2018

csmoe Jul 9, 2018

estebank Jun 12, 2018

This comment has been minimized.

This comment has been minimized.

estebank commented Jun 15, 2018 •

edited

Loading

csmoe commented Jun 15, 2018 •

edited

Loading

estebank commented Jun 15, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-highfive commented Jun 22, 2018

stokhos commented Jun 30, 2018

pietroalbini commented Jul 9, 2018

bors commented Jul 13, 2018

csmoe commented Jul 13, 2018

Fix wrong test replacemen in ref mut suggestion #51249

Fix wrong test replacemen in ref mut suggestion #51249

Conversation

csmoe commented May 31, 2018

rust-highfive commented May 31, 2018

This comment has been minimized.

This comment has been minimized.

killercup Jun 1, 2018 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

csmoe commented Jun 1, 2018 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

TimNN commented Jun 12, 2018

oli-obk Jun 12, 2018

Choose a reason for hiding this comment

csmoe Jun 22, 2018

Choose a reason for hiding this comment

csmoe Jul 9, 2018

Choose a reason for hiding this comment

estebank Jun 12, 2018

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

estebank commented Jun 15, 2018 • edited Loading

csmoe commented Jun 15, 2018 • edited Loading

estebank commented Jun 15, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-highfive commented Jun 22, 2018

stokhos commented Jun 30, 2018

pietroalbini commented Jul 9, 2018

bors commented Jul 13, 2018

csmoe commented Jul 13, 2018

Fix wrong test replacemen in `ref mut` suggestion #51249

Fix wrong test replacemen in `ref mut` suggestion #51249

killercup Jun 1, 2018 •

edited

Loading

csmoe commented Jun 1, 2018 •

edited

Loading

estebank commented Jun 15, 2018 •

edited

Loading

csmoe commented Jun 15, 2018 •

edited

Loading