Stream: t-compiler/wg-nll

Topic: issue-52739


davidtwco (Jul 26 2018 at 16:20, on Zulip):

I assume for #52739 (the example from the in-band-lifetimes topic) that this is experimenting with disabling some of the nice region errors?

davidtwco (Jul 26 2018 at 16:21, on Zulip):

cc @nikomatsakis

nikomatsakis (Jul 26 2018 at 16:21, on Zulip):

I also filed https://github.com/rust-lang/rust/issues/52742

nikomatsakis (Jul 26 2018 at 16:21, on Zulip):

@David Wood I think we should just disable the "nice region errors" altogether in NLL, at least as an experiment

nikomatsakis (Jul 26 2018 at 16:22, on Zulip):

I'd be curious to see the impact

davidtwco (Jul 26 2018 at 16:22, on Zulip):

Sure thing.

davidtwco (Jul 26 2018 at 16:22, on Zulip):

Should I self-assign that issue too?

nikomatsakis (Jul 26 2018 at 16:22, on Zulip):

in particular, I think that maybe you can make a case that the '1 errors are better even in cases where the nice region errors work perfectly -- the reason being consistency

nikomatsakis (Jul 26 2018 at 16:22, on Zulip):

sounds good

nikomatsakis (Jul 26 2018 at 16:22, on Zulip):

(but I'm not sure about)

nikomatsakis (Jul 26 2018 at 16:23, on Zulip):

I could imagine us wanting to port some of the "nice regon error" logic over to NLL in more restricted form

nikomatsakis (Jul 26 2018 at 16:23, on Zulip):

but first step is to turn them off and see the effects

davidtwco (Jul 27 2018 at 14:24, on Zulip):

@nikomatsakis @simulacrum here's the output from running all the UI tests without "nice region errors": https://gist.github.com/davidtwco/49400ddc324fcdd809eaed7886d153a7

There are certainly some cases where it's a straight improvement:

-   error[E0621]: explicit lifetime required in the type of `x`
+   error: unsatisfied lifetime constraints
8     --> $DIR/impl-trait-captures.rs:21:5
9      |
10  LL | fn foo<'a, T>(x: &T) -> impl Foo<'a> {

-      |               - consider changing the type of `x` to `&ReEarlyBound(0, 'a) T`
+      |                  - let's call the lifetime of this reference `'1`
12  LL |     x
-      |     ^ lifetime `ReEarlyBound(0, 'a)` required
+      |     ^ return requires that `'1` must outlive `'a`
14
15  error: aborting due to previous error
16

-   For more information about this error, try `rustc --explain E0621`.
18

But there are others where I'm not sure. We can take a look and work out whether some of the nice region errors are worth keeping - I'm not great at deciding what is or isn't a good diagnostic.

Jake Goulding (Jul 27 2018 at 14:41, on Zulip):

I like the concept of "let's call the lifetime of this reference '1"; wonder if there's a slightly less informal phrasing...

Jake Goulding (Jul 27 2018 at 14:42, on Zulip):

Removing ReEarlyBound is a strong improvement ;-)

davidtwco (Jul 27 2018 at 20:30, on Zulip):

Wrote up some thoughts I have on the three different types of nice region error (as far as I can tell, that's how many there are) in this document:
https://paper.dropbox.com/doc/NLL-Nice-Region-Errors--AItgZNRfVxL1lME1ynRELQyAAQ-Ci1DqrXWuM1mVO0CMGkCS

nikomatsakis (Jul 28 2018 at 04:00, on Zulip):

@David Wood that one you quoted isn't a real test, because it includes -Z verbose in the compile-flags, which is why we print the ReEarlyBound

nikomatsakis (Jul 28 2018 at 04:00, on Zulip):

iirc

nikomatsakis (Jul 28 2018 at 04:02, on Zulip):

I'll skim the results; I think that the nice-error-messages (and to a lesser extent our new ones) also suffer from the problem sometimes of not giving you enough info. In particular, I've gotten suggests like "consider changing the type of tcx to TyCtxt<'tcx, 'tcx, 'tcx> -- which I know is wrong, and I wish it would just tell me why it thinks I should do that

davidtwco (Jul 28 2018 at 08:38, on Zulip):

Ah, should've noticed that.

nikomatsakis (Jul 28 2018 at 13:12, on Zulip):

this is an interesting one

4   LL |     &*x
5      |     ^^^
6
-   error[E0623]: lifetime mismatch
+   error: unsatisfied lifetime constraints
8     --> $DIR/region-lbr1-does-not-outlive-ebr2.rs:19:5
9      |
-   LL | fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'b u32 {
-      |                   -------                 -------
-      |                   |
-      |                   this parameter and the return type are declared with different lifetimes...
14  LL |     &*x
-      |     ^^^ ...but data from `x` is returned here
+      |     ^^^ requires that `'a` must outlive `'b`
16
17  error: aborting due to previous error
18

-   For more information about this error, try `rustc --explain E0623`.
20
nikomatsakis (Jul 28 2018 at 13:12, on Zulip):

that is the specific message where I think -- in general -- the new-style messages are superior, as they give more info

nikomatsakis (Jul 28 2018 at 13:13, on Zulip):

though I think maybe a "medley" would be even better

nikomatsakis (Jul 28 2018 at 13:14, on Zulip):

e.g.

-   error[E0623]: lifetime mismatch
8     --> $DIR/region-lbr1-does-not-outlive-ebr2.rs:19:5
9      |
    LL | fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'b u32 {
       |                   -                       -
       |                   |
       |                   these two references have different lifetimes
14  LL |     &*x
       |     ^^^ ...but data from `x` is returned here
16
17  error: aborting due to previous error
18
nikomatsakis (Jul 28 2018 at 13:14, on Zulip):

er, I don't know, in this case it doesn't matter so much. I am more thinking of cases though where the type is more complex

nikomatsakis (Jul 28 2018 at 13:15, on Zulip):

alternatively, in the new style message, it might be nice if we would highlight the 'a in the reference

nikomatsakis (Jul 28 2018 at 13:15, on Zulip):

even though we have a proper name to use

davidtwco (Jul 28 2018 at 14:28, on Zulip):

Yeah, for that example, new errors would be better if they highlighted the arguments like the nice region errors do.

davidtwco (Jul 30 2018 at 11:37, on Zulip):

@nikomatsakis what would you like me to do for this issue (and #52742)? - I've got time this week to work on a PR but not sure how we want to address this issue.

nikomatsakis (Jul 30 2018 at 13:27, on Zulip):

@David Wood I don't know exactly either. I was considering trying to re-implement the nice-region-errors but adjust them? I guess we have to study that gist of yours with the diff from turning them off and decide what to keep and what not

Last update: Nov 22 2019 at 00:35UTC