Stream: t-compiler/wg-nll

Topic: issue-51512-note-field-after-move


Santiago Pastorino (Jun 20 2018 at 22:06, on Zulip):

about https://github.com/rust-lang/rust/issues/51512

Santiago Pastorino (Jun 20 2018 at 22:07, on Zulip):

will probably tackle it tomorrow, but just read it and I have a quick question

Santiago Pastorino (Jun 20 2018 at 22:07, on Zulip):

@nikomatsakis you mentioned moi and from there we can get the place

Santiago Pastorino (Jun 20 2018 at 22:08, on Zulip):

what I don't follow is that we get the moi from mois, so potentially we would get a lot of places, which one is the one we should use?

Santiago Pastorino (Jun 20 2018 at 22:10, on Zulip):

we would like to show more than one note per place?

Santiago Pastorino (Jun 20 2018 at 22:23, on Zulip):

I think this last phrase was confusing

Santiago Pastorino (Jun 20 2018 at 22:23, on Zulip):

I meant, more than one note, one per place, do we want that?

nikomatsakis (Jun 21 2018 at 00:23, on Zulip):

what I don't follow is that we get the moi from mois, so potentially we would get a lot of places, which one is the one we should use?

probably all of them, or maybe just the first — basically, there could be many prior points that have moved the value

nikomatsakis (Jun 21 2018 at 00:24, on Zulip):

e.g.,

let v = vec![];
if foo { drop(v;) } else { drop(v); }
use(v);
nikomatsakis (Jun 21 2018 at 00:24, on Zulip):

here there are two drop(v) statements and either could have done it

nikomatsakis (Jun 21 2018 at 00:24, on Zulip):

it seems reasonable to just show a label and info for the first thing in mois though

nikomatsakis (Jun 21 2018 at 00:24, on Zulip):

any one would do

Santiago Pastorino (Jun 21 2018 at 00:34, on Zulip):

ok

Santiago Pastorino (Jun 21 2018 at 00:34, on Zulip):

will try with the first

Santiago Pastorino (Jun 21 2018 at 03:31, on Zulip):

@nikomatsakis to start discussing https://github.com/spastorino/rust/commit/e42b78abc10e7e04f2aad2eb22ed1b446cc6d7d6

Santiago Pastorino (Jun 21 2018 at 03:32, on Zulip):

I need to change the stderr modifications better

Santiago Pastorino (Jun 21 2018 at 03:34, on Zulip):

it's a bit late here but giving this a quick glance

Santiago Pastorino (Jun 21 2018 at 03:34, on Zulip):

this src/test/ui/borrowck/borrowck-box-insensitivity.nll.stderr seems correct now

Santiago Pastorino (Jun 21 2018 at 03:35, on Zulip):

unsure about this src/test/ui/borrowck/issue-41962.stderr looks weird to me

Santiago Pastorino (Jun 21 2018 at 03:37, on Zulip):

and this one src/test/ui/moves-based-on-type-match-bindings.nll.stderr seems correct now

Santiago Pastorino (Jun 21 2018 at 03:37, on Zulip):

@nikomatsakis ttyt

nikomatsakis (Jun 21 2018 at 09:16, on Zulip):

@pnkfelix take a look at my question here

nikomatsakis (Jun 21 2018 at 09:16, on Zulip):

actually i'll just paste it in zulip:

nikomatsakis (Jun 21 2018 at 09:17, on Zulip):

unrelated but I do wonder if we should break out of this loop after the first entry...

The following example on stable:

fn main() {
    let v: Vec<u32> = vec![];

    if true {
        drop(v);
    } else {
        drop(v);
    }

    v.len();
}

prints

error[E0382]: use of moved value: `v`
  --> src/main.rs:10:5
   |
5  |         drop(v);
   |              - value moved here
...
10 |     v.len();
   |     ^ value used here after move
   |
   = note: move occurs because `v` has type `std::vec::Vec<u32>`, which does not implement the `Copy` trait

with NLL it prints:

error[E0382]: borrow of moved value: `v`
  --> src/main.rs:12:5
   |
7  |         drop(v);
   |              - value moved here
8  |     } else {
9  |         drop(v);
   |              - value moved here
...
12 |     v.len();
   |     ^ value borrowed here after move
   |
   = note: move occurs because `v` has type `std::vec::Vec<u32>`, which does not implement the `Copy` trait

I'm not sure it's an improvement to print both.

Santiago Pastorino (Jun 21 2018 at 14:13, on Zulip):
                if moved_lp.has_downcast() {
                    "the value".to_string()
                } else {
                    format!("`{}`", self.loan_path_to_string(moved_lp))
                },
Santiago Pastorino (Jun 21 2018 at 14:13, on Zulip):

@nikomatsakis this ^^^ is what you're talking about?

nikomatsakis (Jun 21 2018 at 14:19, on Zulip):

sounds roughly right; I think we had some existing function for this too? I have to look around

nikomatsakis (Jun 21 2018 at 14:19, on Zulip):

it might cover a few more cases

Santiago Pastorino (Jun 21 2018 at 15:09, on Zulip):

@nikomatsakis ast code ends calling this https://github.com/rust-lang/rust/blob/master/src/librustc_borrowck/borrowck/mod.rs#L1351

nikomatsakis (Jun 21 2018 at 15:27, on Zulip):

right, librustc_borrowck is the old borrow checker

Santiago Pastorino (Jun 21 2018 at 15:32, on Zulip):

yes, but I meant, should we do something similar?

Santiago Pastorino (Jun 21 2018 at 15:32, on Zulip):

all that code is in the old borrowck

nikomatsakis (Jun 21 2018 at 15:39, on Zulip):

we .. are doing something similar, aren't we?

nikomatsakis (Jun 21 2018 at 15:40, on Zulip):

do you mean, should we call the same helper fn?

nikomatsakis (Jun 21 2018 at 15:40, on Zulip):

oh I guess I see your question

nikomatsakis (Jun 21 2018 at 15:40, on Zulip):

well we don't have those data structures

nikomatsakis (Jun 21 2018 at 15:40, on Zulip):

let me look and see, I feel like there was a fn for this

nikomatsakis (Jun 21 2018 at 15:45, on Zulip):

@Santiago Pastorino so I was thinking of describe_place

nikomatsakis (Jun 21 2018 at 15:45, on Zulip):

however

nikomatsakis (Jun 21 2018 at 15:45, on Zulip):

it doesn't quite do what I think it should

nikomatsakis (Jun 21 2018 at 15:46, on Zulip):

in particular, Downcast doesn't seem to return Err

nikomatsakis (Jun 21 2018 at 15:46, on Zulip):

but I think that's a bug in describe_place

Santiago Pastorino (Jun 21 2018 at 15:46, on Zulip):

yeah, we are already calling describe_place in the code I've committed

nikomatsakis (Jun 21 2018 at 15:46, on Zulip):

yeah so maybe the bug is more that code

nikomatsakis (Jun 21 2018 at 15:47, on Zulip):

in particular this match arm seems wrong

nikomatsakis (Jun 21 2018 at 15:47, on Zulip):

I think it should return Err

Santiago Pastorino (Jun 21 2018 at 15:48, on Zulip):

I see

Santiago Pastorino (Jun 21 2018 at 16:20, on Zulip):

(deleted)

nikomatsakis (Jun 21 2018 at 16:53, on Zulip):

did you try that?

Santiago Pastorino (Jun 21 2018 at 16:58, on Zulip):

let me paste the output

Santiago Pastorino (Jun 21 2018 at 16:58, on Zulip):

https://gist.github.com/spastorino/6f29aa8772c5de988df8d73190233dee

Santiago Pastorino (Jun 21 2018 at 16:58, on Zulip):

well ... forget about the ones that are ICEing

Santiago Pastorino (Jun 21 2018 at 16:58, on Zulip):

need to ignore those

Santiago Pastorino (Jun 21 2018 at 16:59, on Zulip):

is that what you want?

Santiago Pastorino (Jun 21 2018 at 16:59, on Zulip):
-error[E0382]: use of moved value: `maybe.0` (Mir)
+error[E0382]: use of moved value: `_` (Mir)
Santiago Pastorino (Jun 21 2018 at 16:59, on Zulip):

is weird

nikomatsakis (Jun 21 2018 at 17:01, on Zulip):

that is most certainly not what we want :P

nikomatsakis (Jun 21 2018 at 17:02, on Zulip):

I would expect just "use of moved value" I think

Santiago Pastorino (Jun 21 2018 at 17:02, on Zulip):

:P

Santiago Pastorino (Jun 21 2018 at 17:03, on Zulip):

unsure why is that happening

Santiago Pastorino (Jun 21 2018 at 17:03, on Zulip):

let me investigate

Santiago Pastorino (Jun 21 2018 at 17:04, on Zulip):

so ...

Santiago Pastorino (Jun 21 2018 at 17:04, on Zulip):
[santiago@archlinux rust1 (error-note-field-after-move)]$ rustc +stage1 src/test/ui/borrowck/issue-41962.rs
warning: unused variable: `thing`
  --> src/test/ui/borrowck/issue-41962.rs:17:21
   |
17 |         if let Some(thing) = maybe {
   |                     ^^^^^ help: consider using `_thing` instead
   |
   = note: #[warn(unused_variables)] on by default

error[E0382]: use of partially moved value: `maybe`
  --> src/test/ui/borrowck/issue-41962.rs:17:30
   |
17 |         if let Some(thing) = maybe {
   |                     -----    ^^^^^ value used here after move
   |                     |
   |                     value moved here
   |
   = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `(maybe as std::prelude::v1::Some).0`
  --> src/test/ui/borrowck/issue-41962.rs:17:21
   |
17 |         if let Some(thing) = maybe {
   |                     ^^^^^ value moved here in previous iteration of loop
   |
   = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0382`.
Santiago Pastorino (Jun 21 2018 at 17:04, on Zulip):

vs

Santiago Pastorino (Jun 21 2018 at 17:04, on Zulip):
[santiago@archlinux rust1 (error-note-field-after-move)]$ rustc +stage1 -Zborrowck=mir -Ztwo-phase-borrows src/test/ui/borrowck/issue-41962.rs
warning: unused variable: `thing`
  --> src/test/ui/borrowck/issue-41962.rs:17:21
   |
17 |         if let Some(thing) = maybe {
   |                     ^^^^^ help: consider using `_thing` instead
   |
   = note: #[warn(unused_variables)] on by default

error[E0382]: use of moved value: `maybe`
  --> src/test/ui/borrowck/issue-41962.rs:17:9
   |
17 |           if let Some(thing) = maybe {
   |           ^           ----- value moved here
   |  _________|
   | |
18 | |         }
   | |_________^ value used here after move
   |
   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: borrow of moved value: `maybe`
  --> src/test/ui/borrowck/issue-41962.rs:17:9
   |
17 |           if let Some(thing) = maybe {
   |           ^           ----- value moved here
   |  _________|
   | |
18 | |         }
   | |_________^ value borrowed here after move
   |
   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `maybe`
  --> src/test/ui/borrowck/issue-41962.rs:17:16
   |
17 |         if let Some(thing) = maybe {
   |                ^^^^^-----^
   |                |    |
   |                |    value moved here
   |                value used here after move
   |
   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `_`
  --> src/test/ui/borrowck/issue-41962.rs:17:21
   |
17 |         if let Some(thing) = maybe {
   |                     ^^^^^ value moved here in previous iteration of loop
   |
   = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0382`.
Santiago Pastorino (Jun 21 2018 at 17:05, on Zulip):

NLL error displaying all that it's very confused

Santiago Pastorino (Jun 21 2018 at 17:05, on Zulip):

I think, I'm with you about just showing the first one

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

ok I'm prety confused

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

so, when you run in NLL mode,

Santiago Pastorino (Jun 21 2018 at 17:05, on Zulip):

anyway ... that's for another PR

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

it dumps 4 errors?

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

that's the first problem I guess

Santiago Pastorino (Jun 21 2018 at 17:05, on Zulip):

yes

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

I see, ok

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

yeah and there's a mix of spans

nikomatsakis (Jun 21 2018 at 17:05, on Zulip):

some of which are just not great

nikomatsakis (Jun 21 2018 at 17:06, on Zulip):

well I agree that would be good to handle in follow up PRs

Santiago Pastorino (Jun 21 2018 at 17:06, on Zulip):

would you just get the first one?

nikomatsakis (Jun 21 2018 at 17:07, on Zulip):

probably

nikomatsakis (Jun 21 2018 at 17:07, on Zulip):

ideally with a different span

nikomatsakis (Jun 21 2018 at 17:07, on Zulip):

it's not great that we highlight the if let

nikomatsakis (Jun 21 2018 at 17:08, on Zulip):

that will probably have to be tweaked in the MIR construction

Santiago Pastorino (Jun 21 2018 at 17:09, on Zulip):

I guess the problem happens here https://github.com/rust-lang/rust/blob/master/src/librustc_mir/util/borrowck_errors.rs#L364

Santiago Pastorino (Jun 21 2018 at 17:09, on Zulip):

moved_path is _

Santiago Pastorino (Jun 21 2018 at 17:10, on Zulip):

ahh I see

Santiago Pastorino (Jun 21 2018 at 17:11, on Zulip):

https://github.com/rust-lang/rust/blob/master/src/librustc_mir/borrow_check/error_reporting.rs#L63

Santiago Pastorino (Jun 21 2018 at 17:11, on Zulip):

should we use value there?

Santiago Pastorino (Jun 21 2018 at 17:11, on Zulip):

ok, let me try something

nikomatsakis (Jun 21 2018 at 17:51, on Zulip):

should we use value there?

I think that fn should probably take an Option, so we can leave off the : etc altogether

Santiago Pastorino (Jun 21 2018 at 17:52, on Zulip):

that's exactly what I did

Santiago Pastorino (Jun 21 2018 at 17:52, on Zulip):

I'm just compiling this thing

Santiago Pastorino (Jun 21 2018 at 17:52, on Zulip):

gonna send a diff when this finishes

Santiago Pastorino (Jun 21 2018 at 17:53, on Zulip):

this ...

Santiago Pastorino (Jun 21 2018 at 17:53, on Zulip):
diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
index 684fd10c8c..500903c1c3 100644
--- a/src/librustc_borrowck/borrowck/mod.rs
+++ b/src/librustc_borrowck/borrowck/mod.rs
@@ -680,7 +680,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                 let mut err = self.cannot_act_on_moved_value(use_span,
                                                              verb,
                                                              msg,
-                                                             &format!("{}", nl),
+                                                             Some(format!("{}", nl)),
                                                              Origin::Ast);
                 let need_note = match lp.ty.sty {
                     ty::TypeVariants::TyClosure(id, _) => {
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs
index 349e4ff933..0782d19d94 100644
--- a/src/librustc_mir/borrow_check/error_reporting.rs
+++ b/src/librustc_mir/borrow_check/error_reporting.rs
@@ -72,7 +72,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 span,
                 desired_action.as_noun(),
                 msg,
-                &self.describe_place(place).unwrap_or("_".to_owned()),
+                self.describe_place(&place),
                 Origin::Mir,
             );

@@ -706,6 +706,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     }
                     ProjectionElem::Downcast(..) => {
                         self.append_place_to_string(&proj.base, buf, autoderef)?;
+                        return Err(());
                     }
                     ProjectionElem::Field(field, _ty) => {
                         autoderef = true;
diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs
index d01b90ad26..a6dd32ce23 100644
--- a/src/librustc_mir/util/borrowck_errors.rs
+++ b/src/librustc_mir/util/borrowck_errors.rs
@@ -356,12 +356,14 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
                                  use_span: Span,
                                  verb: &str,
                                  optional_adverb_for_moved: &str,
-                                 moved_path: &str,
+                                 moved_path: Option<String>,
                                  o: Origin)
                                  -> DiagnosticBuilder<'cx>
     {
+        let moved_path = moved_path.map(|mp| format!(": `{}`", mp)).unwrap_or("".to_owned());
+
         let err = struct_span_err!(self, use_span, E0382,
-                                   "{} of {}moved value: `{}`{OGN}",
+                                   "{} of {}moved value{}{OGN}",
                                    verb, optional_adverb_for_moved, moved_path, OGN=o);

         self.cancel_if_wrong_origin(err, o)
Santiago Pastorino (Jun 21 2018 at 17:53, on Zulip):

is what I have for now

Santiago Pastorino (Jun 21 2018 at 17:53, on Zulip):

and I guess now it's right

Santiago Pastorino (Jun 21 2018 at 17:53, on Zulip):

let's see :)

nikomatsakis (Jun 21 2018 at 17:58, on Zulip):

looks good :)

Santiago Pastorino (Jun 21 2018 at 18:10, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51688

nikomatsakis (Jun 21 2018 at 18:17, on Zulip):

review: https://github.com/rust-lang/rust/pull/51688#pullrequestreview-130936085

nikomatsakis (Jun 21 2018 at 18:17, on Zulip):

had a question

Santiago Pastorino (Jun 21 2018 at 18:18, on Zulip):

https://github.com/rust-lang/rust/pull/51688#discussion_r197231795

nikomatsakis (Jun 21 2018 at 18:22, on Zulip):

ok, r+ -- so, we can now talk about the duplicate problems

nikomatsakis (Jun 21 2018 at 18:22, on Zulip):

I think just issuing the first error is what we want

Santiago Pastorino (Jun 21 2018 at 18:22, on Zulip):

:+1:

nikomatsakis (Jun 21 2018 at 18:22, on Zulip):

but it sounded to me like you thought this might arise from just not iterating over all the mois elements or something

nikomatsakis (Jun 21 2018 at 18:22, on Zulip):

if so, I think that's not the case

Santiago Pastorino (Jun 21 2018 at 18:23, on Zulip):

no no

Santiago Pastorino (Jun 21 2018 at 18:23, on Zulip):

just getting the first thing that matches if span == move_span {

Santiago Pastorino (Jun 21 2018 at 18:24, on Zulip):

we need to break from the loop at that point

nikomatsakis (Jun 21 2018 at 18:25, on Zulip):

ok wait

nikomatsakis (Jun 21 2018 at 18:25, on Zulip):

maybe I am confused actually

nikomatsakis (Jun 21 2018 at 18:26, on Zulip):

so first off I think your PR is not quite right then

nikomatsakis (Jun 21 2018 at 18:26, on Zulip):

er, no, never mind

nikomatsakis (Jun 21 2018 at 18:26, on Zulip):

I think I don't quite get what you mean

nikomatsakis (Jun 21 2018 at 18:26, on Zulip):

do you mean breaking at this point:

nikomatsakis (Jun 21 2018 at 18:27, on Zulip):

https://github.com/spastorino/rust/blob/fe264bb6f0aa7efb7be3432360edd1f8923c1f80/src/librustc_mir/borrow_check/error_reporting.rs#L88

Santiago Pastorino (Jun 21 2018 at 18:27, on Zulip):

what I've said is wrong

Santiago Pastorino (Jun 21 2018 at 18:27, on Zulip):

we need to call err.span_label twice in the else

Santiago Pastorino (Jun 21 2018 at 18:27, on Zulip):

I think

nikomatsakis (Jun 21 2018 at 18:27, on Zulip):

I don't udnerstand

nikomatsakis (Jun 21 2018 at 18:27, on Zulip):

each time around the loop

nikomatsakis (Jun 21 2018 at 18:27, on Zulip):

adds just one "labeled thing" to the error

nikomatsakis (Jun 21 2018 at 18:27, on Zulip):

it is not responsible for printing 4 distinct errors

Santiago Pastorino (Jun 21 2018 at 18:28, on Zulip):

ok

nikomatsakis (Jun 21 2018 at 18:28, on Zulip):

that has to be handled at a higher level

Santiago Pastorino (Jun 21 2018 at 18:28, on Zulip):

maybe I'm not understanding :D

Santiago Pastorino (Jun 21 2018 at 18:28, on Zulip):

we want to not display those 4 distinct errors, right?

Santiago Pastorino (Jun 21 2018 at 18:28, on Zulip):

and just display one?

Santiago Pastorino (Jun 21 2018 at 18:28, on Zulip):

or did I get it wrong?

nikomatsakis (Jun 21 2018 at 18:29, on Zulip):

so earlier I wrote this comment that gives an example where the loop has more than one iteration

nikomatsakis (Jun 21 2018 at 18:29, on Zulip):

we want to not display those 4 distinct errors, right?

correct

nikomatsakis (Jun 21 2018 at 18:29, on Zulip):

it's just that it's not that this for moi in mois loop is responsible for that

nikomatsakis (Jun 21 2018 at 18:29, on Zulip):

rather, the borrow check has an outer walk

nikomatsakis (Jun 21 2018 at 18:31, on Zulip):

checking all the accesses that occur in the MIR

nikomatsakis (Jun 21 2018 at 18:31, on Zulip):

and there are multiple cases that trigger an error here

Santiago Pastorino (Jun 21 2018 at 18:37, on Zulip):

hmm ok, need to check this then

Santiago Pastorino (Jun 21 2018 at 18:45, on Zulip):

@nikomatsakis after checking I don't see an obvious way to break out of the loop

Santiago Pastorino (Jun 21 2018 at 18:45, on Zulip):

isn't this called several times the visitors?

Santiago Pastorino (Jun 21 2018 at 18:46, on Zulip):

from what I've seen it all starts at visit_terminator_entry

nikomatsakis (Jun 21 2018 at 18:46, on Zulip):

I don't think this loop is relevant at all

nikomatsakis (Jun 21 2018 at 18:46, on Zulip):

I'm confused

nikomatsakis (Jun 21 2018 at 18:46, on Zulip):

wait, what loop are you talking about ? :)

nikomatsakis (Jun 21 2018 at 18:46, on Zulip):

regardless, I don't think we want to do any breaking to fix the multiple duplicate error problem

Santiago Pastorino (Jun 21 2018 at 18:46, on Zulip):

and there are multiple cases that trigger an error here

^^^

nikomatsakis (Jun 21 2018 at 18:46, on Zulip):

rather, I think we want to have some map (we have some already)

nikomatsakis (Jun 21 2018 at 18:47, on Zulip):

and when we would report an error, we check for an entry in this map that suggests we already reported a "similar" erro

nikomatsakis (Jun 21 2018 at 18:47, on Zulip):

if so, we do not report the new one

nikomatsakis (Jun 21 2018 at 18:47, on Zulip):

and just ignore it

nikomatsakis (Jun 21 2018 at 18:47, on Zulip):

I can show you some examples of this behavior

Santiago Pastorino (Jun 21 2018 at 18:47, on Zulip):

ahh ok

nikomatsakis (Jun 21 2018 at 18:47, on Zulip):

I feel though like there should be some consolidation possible here

Santiago Pastorino (Jun 21 2018 at 18:48, on Zulip):

so you want to change err.span_label calls to something that adds on a map?

Santiago Pastorino (Jun 21 2018 at 18:48, on Zulip):

and then on emit we do the right thing for this case?

nikomatsakis (Jun 21 2018 at 18:48, on Zulip):

e.g. we have these maps already

nikomatsakis (Jun 21 2018 at 18:48, on Zulip):

and this one too

Santiago Pastorino (Jun 21 2018 at 18:59, on Zulip):

ok

Santiago Pastorino (Jun 21 2018 at 19:05, on Zulip):

moved_error_reported is never used

Santiago Pastorino (Jun 21 2018 at 19:05, on Zulip):

I mean, only for debugging purposes

nikomatsakis (Jun 21 2018 at 19:06, on Zulip):

ok

Santiago Pastorino (Jun 21 2018 at 19:06, on Zulip):

anyway, I can use it for my case, I guess

Santiago Pastorino (Jun 21 2018 at 19:41, on Zulip):

@nikomatsakis about https://github.com/rust-lang/rust/pull/51688#issuecomment-399213060

Santiago Pastorino (Jun 21 2018 at 19:41, on Zulip):
diff --git a/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs b/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs
index 3a7e4a1374..74a62566b1 100644
--- a/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs
+++ b/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs
@@ -77,7 +77,7 @@ fn main() {
         match e { //[mir]~ ERROR cannot use `e` because it was mutably borrowed
             Baz::X(value) => value
             //[ast]~^ ERROR cannot use `e.0` because it was mutably borrowed
-            //[mir]~^^ ERROR cannot use `e.0` because it was mutably borrowed
+            //[mir]~^^ ERROR cannot use `_` because it was mutably borrowed
         };
         drop(x);
     }
@@ -120,7 +120,7 @@ fn main() {
         match *e { //[mir]~ ERROR cannot use `*e` because it was mutably borrowed
             Baz::X(value) => value
             //[ast]~^ ERROR cannot use `e.0` because it was mutably borrowed
-            //[mir]~^^ ERROR cannot use `e.0` because it was mutably borrowed
+            //[mir]~^^ ERROR cannot use `_` because it was mutably borrowed
         };
         drop(x);
     }
@@ -201,12 +201,12 @@ fn main() {
         match e { //[mir]~ ERROR cannot use `e` because it was mutably borrowed
             E::A(ref ax) =>
                 //[ast]~^ ERROR cannot borrow `e.0` as immutable because `e` is also borrowed as mutable
-                //[mir]~^^ ERROR cannot borrow `e.0` as immutable because it is also borrowed as mutable
+                //[mir]~^^ ERROR cannot borrow `_` as immutable because it is also borrowed as mutable
                 //[mir]~| ERROR cannot use `e` because it was mutably borrowed
                 println!("e.ax: {:?}", ax),
             E::B { x: ref bx } =>
                 //[ast]~^ ERROR cannot borrow `e.x` as immutable because `e` is also borrowed as mutable
-                //[mir]~^^ ERROR cannot borrow `e.x` as immutable because it is also borrowed as mutable
+                //[mir]~^^ ERROR cannot borrow `_` as immutable because it is also borrowed as mutable
                 println!("e.bx: {:?}", bx),
         }
         drop(x);
diff --git a/src/test/ui/error-codes/E0657.stderr b/src/test/ui/error-codes/E0657.stderr
index 737ae3a163..df0234a766 100644
--- a/src/test/ui/error-codes/E0657.stderr
+++ b/src/test/ui/error-codes/E0657.stderr
@@ -10,6 +10,8 @@ error[E0657]: `impl Trait` can only capture lifetimes bound at the fn or impl le
 LL |         -> Box<for<'a> Id<impl Lt<'a>>>
    |                                   ^^

+#0 [typeck_tables_of] processing `free_fn_capture_hrtb_in_impl_trait`
+#1 [type_of] processing `free_fn_capture_hrtb_in_impl_trait::{{exist-impl-Trait}}`
 error: aborting due to 2 previous errors

 For more information about this error, try `rustc --explain E0657`.
diff --git a/src/test/ui/impl_trait_projections.stderr b/src/test/ui/impl_trait_projections.stderr
index f6d58984ec..2802df1080 100644
--- a/src/test/ui/impl_trait_projections.stderr
+++ b/src/test/ui/impl_trait_projections.stderr
@@ -30,6 +30,7 @@ LL | fn projection_is_disallowed(x: impl Iterator) -> <impl Iterator>::Item {
    |
    = note: specify the type using the syntax `<impl std::iter::Iterator as Trait>::Item`

+#0 [type_of] processing `projection_is_disallowed::{{exist-impl-Trait}}`
 error: aborting due to 5 previous errors

 Some errors occurred: E0223, E0667.
Santiago Pastorino (Jun 21 2018 at 19:42, on Zulip):

and that would be wrong

Santiago Pastorino (Jun 21 2018 at 19:42, on Zulip):

will check these side effects

Santiago Pastorino (Jun 21 2018 at 20:08, on Zulip):

@nikomatsakis well yeah, the return Err in the Downcast is making this work like that

Santiago Pastorino (Jun 21 2018 at 20:08, on Zulip):

unsure what we can do about this

nikomatsakis (Jun 21 2018 at 20:24, on Zulip):

@Santiago Pastorino hmm, yeah. I've always considered those errors pretty suboptimal

nikomatsakis (Jun 21 2018 at 20:25, on Zulip):

that is, they are displaying a path to the user that isn't something you could type

nikomatsakis (Jun 21 2018 at 20:25, on Zulip):

and there is no particular reason you should be able to guess what it means

Santiago Pastorino (Jun 21 2018 at 20:25, on Zulip):

so ... should I do the same?

nikomatsakis (Jun 21 2018 at 20:25, on Zulip):

I'm torn between saying "we should match AST borrowck" and "we should improve on it"

nikomatsakis (Jun 21 2018 at 20:25, on Zulip):

@pnkfelix you don't happen to be around, do you? I'm curious as to your opinion there :)

Santiago Pastorino (Jun 21 2018 at 20:26, on Zulip):

well, I meant, what should I do then?

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

well first we should decide if we're trying to match AST or improve upon it :)

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

I guess that even if we were going to do the latter

Santiago Pastorino (Jun 21 2018 at 20:26, on Zulip):

I'm not sure if you were saying that the new behavior is wrong for this case or if I should ... ok

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

we'd probably prefer to do it in a separate PR

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

sounds like we should make two copies of that darn "describe-path" fn

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

or have a parameter

Santiago Pastorino (Jun 21 2018 at 20:26, on Zulip):

ok

nikomatsakis (Jun 21 2018 at 20:26, on Zulip):

for how to treat downcasts

nikomatsakis (Jun 21 2018 at 20:27, on Zulip):

I'm not wild about that

nikomatsakis (Jun 21 2018 at 20:27, on Zulip):

but I think it's better

nikomatsakis (Jun 21 2018 at 20:27, on Zulip):

and anyway if we are going to match the AST, it does not use paths in some of these cases

Santiago Pastorino (Jun 21 2018 at 21:21, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51688 force pushed, it's ugly :/

nikomatsakis (Jun 22 2018 at 10:28, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51688 force pushed, it's ugly :/

specifically, you mean the true, false, business?

lqd (Jun 22 2018 at 10:48, on Zulip):

(sorry @Santiago Pastorino my PR has made yours conflict, I think because of its rustfmt commit)

nikomatsakis (Jun 22 2018 at 11:10, on Zulip):

@Santiago Pastorino I left a kind of "style nit" suggestion — what do you think?

Santiago Pastorino (Jun 22 2018 at 12:28, on Zulip):

@lqd no worries, ya, that's the bad thing of committing rustfmt changes

Santiago Pastorino (Jun 22 2018 at 12:28, on Zulip):

I'd like to be able to do something like rustfmt file:88-100 or even rustfmt file --git-diff-only

Santiago Pastorino (Jun 22 2018 at 12:30, on Zulip):

@Santiago Pastorino I left a kind of "style nit" suggestion — what do you think?

@nikomatsakis makes sense

nikomatsakis (Jun 22 2018 at 13:06, on Zulip):

I'd like to be able to do something like rustfmt file:88-100 or even rustfmt file --git-diff-only

I know @nrc was talking about adding the latter thing at some point.

qmx (Jun 22 2018 at 13:07, on Zulip):

that would be a dream

nikomatsakis (Jun 22 2018 at 13:18, on Zulip):

mostly I just want to rustfmt all of rustc

Santiago Pastorino (Jun 22 2018 at 14:01, on Zulip):

@nikomatsakis let's do it now :P

Santiago Pastorino (Jun 22 2018 at 14:01, on Zulip):

rustfmt everything and break all the PRs

Santiago Pastorino (Jun 22 2018 at 14:01, on Zulip):

:D

Santiago Pastorino (Jun 22 2018 at 14:02, on Zulip):

or make bors do it on every merge commit

nikomatsakis (Jun 22 2018 at 14:10, on Zulip):

yes...

Santiago Pastorino (Jun 22 2018 at 16:29, on Zulip):

https://github.com/rust-lang/rust/pull/51688 force pushed again

Santiago Pastorino (Jun 22 2018 at 19:44, on Zulip):

@nikomatsakis ^^^ tests are green, unsure if you need to approve again or something

lqd (Jun 22 2018 at 19:57, on Zulip):

yes it needs to be r+'ed again any time a new commit is added, or the branch is force pushed

Santiago Pastorino (Jun 22 2018 at 20:20, on Zulip):

makes sense :+1:

Last update: Nov 21 2019 at 23:40UTC