Extend E0623 for LateBound and EarlyBound Regions #44079

gaurikholkar-zz · 2017-08-25T06:33:49Z

This is a fix for #43882

fn foo<'a,'b>(x: &mut Vec<&'a u8>, y: &'b u8) {
    x.push(y);
}

now gives

error[E0623]: lifetime mismatch
  --> $DIR/ex3-both-anon-regions-latebound-regions.rs:12:12
   |
11 | fn foo<'a,'b>(x: &mut Vec<&'a u8>, y: &'b u8) {
   |                           ------      ------ these two types are declared with different lifetimes...
12 |     x.push(y);
   |            ^ ...but data from `y` flows into `x` here

cc @nikomatsakis @arielb1

Please ignore the second commit. It will be merged in a separate PR.

rust-highfive · 2017-08-25T06:34:01Z

r? @nikomatsakis

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

bors · 2017-08-25T15:36:18Z

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

shepmaster · 2017-08-25T19:21:57Z

Please ignore the second commit. It will be merged in a separate PR.

Based on this and the merge conflicts, I'm going to treat this as a kind of WIP.

gaurikholkar-zz · 2017-08-26T04:56:40Z

@shepmaster the first commit was #43700 itself which just got merged. I'll fix the merge conflicts and the compile - tests now

gaurikholkar-zz · 2017-08-27T17:25:27Z

@arielb1 can you review this please. The error code works fine for LateBound, not working yet for EarlyBound.

gaurikholkar-zz · 2017-08-28T08:20:05Z

@nikomatsakis can you please review the compile-fail tests?

arielb1 · 2017-08-28T11:11:33Z

src/librustc/infer/error_reporting/anon_anon_conflict.rs

-                        if debruijn_index.depth == 1 && anon_index == br_index {
-                            self.found_type = Some(arg);
-                            return; // we can stop visiting now
+                        match self.bound_region {


style: you could do a

match (self.infcx.tcx.named_region_map.defs.get(&lifetime.id), self.bound_region) { (Some(&rl::Region::LateBoundAnon(debruijn_index, anon_index)), ty::BrAnon(br_index)) => { // .. } // .. }

arielb1 · 2017-08-28T11:12:57Z

src/librustc/infer/error_reporting/named_anon_conflict.rs

        }
        return true;
    }
-}
+}


style: no newline at end of file

arielb1 · 2017-08-28T11:14:43Z

src/librustc/infer/error_reporting/util.rs

+                .as_local_node_id(suitable_region_binding_scope)
+                .unwrap();
+            let mut is_impl_item = false;
+            match self.tcx.hir.find(node_id) {


style:

let is_impl_item = match self.tcx.hir.find(node_id) { Some(hir_map::NodeItem(..)) | Some(hir_map::NodeTraitItem(..)) => { false } Some(hir_map::NodeImplItem(..)) => { self.is_bound_region_in_impl_item(suitable_region_binding_scope) } _ => return None };

arielb1 · 2017-08-28T11:16:09Z

src/test/compile-fail/regions-outlives-projection-container-hrtb.rs

@@ -40,7 +40,6 @@ fn with_assoc<'a,'b>() {
    // We get an error because 'b:'a does not hold:

    let _: &'a WithHrAssoc<TheType<'b>> = loop { };


Where did the error go?

Does this code compile with your patch? Could you try to figure out why an error is not generated?

arielb1 · 2017-08-28T11:16:41Z

src/test/compile-fail/regions-outlives-projection-container.rs

 }

 fn call<T>() { }

 fn main() {
-}
+}p


stray p in program

arielb1 · 2017-08-28T11:17:10Z

src/test/ui/lifetime-errors/ex3-both-anon-regions-latebound-regions.stderr

+   |            ^ ...but data from `y` flows into `x` here
+
+error: aborting due to previous error
+


Also add a test for early-bound regions (using the 'static: 'a, 'static: 'b trick?)

The example we had discussed with where clause and sized trait doesn't seem to be working. So.still trying to get the code working for early bound

arielb1 · 2017-08-28T11:19:48Z

I would say r=me after style fixes, but

WTF is happening with the error messages?

gaurikholkar-zz · 2017-08-28T11:22:32Z

@arielb some of the tests keep failing, I fixed them but they failed again.

arielb1 · 2017-08-28T11:28:59Z

@gaurikholkar

Are you on IRC?

gaurikholkar-zz · 2017-08-28T11:40:46Z

Afk for a few hours. @gkholkar on irc.

nikomatsakis

Left a few comments. This looks pretty nice altogether, though I am concerned about the "disappearing errors" that @arielb1 noted. That said, I see travis isn't passing, so maybe those errors are still being generated?

nikomatsakis · 2017-08-28T15:17:05Z

src/librustc/infer/error_reporting/anon_anon_conflict.rs

@@ -46,9 +46,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
        };

        // Determine whether the sub and sup consist of both anonymous (elided) regions.
-        let anon_reg_sup = or_false!(self.is_suitable_anonymous_region(sup));
+        let anon_reg_sup = or_false!(self.is_suitable_region(sup));


Maybe we should rename this variable, if it's not always anonymous? (For that matter, this file and fn?)

How about both_params_anon_or_named_conflict.rs ?

hmm, maybe something like different_lifetimes.rs? (since this generates the "types declared with different lifetimes..." message)

nikomatsakis · 2017-08-28T15:19:23Z

src/librustc/infer/error_reporting/util.rs

@@ -193,4 +191,4 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
        }
        false
    }
-}
+}


Nit: newline at end of file

nikomatsakis · 2017-08-28T15:19:38Z

src/test/compile-fail/regions-assoc-type-in-supertrait-outlives-container.rs

@@ -47,7 +47,7 @@ fn with_assoc<'a,'b>() {
    // outlive 'a. In this case, that means TheType<'b>::TheAssocType,
    // which is &'b (), must outlive 'a.

-    let _: &'a WithAssoc<TheType<'b>> = loop { }; //~ ERROR reference has a longer lifetime
+    let _: &'a WithAssoc<TheType<'b>> = loop { };//~ ERROR reference has a longer lifetime


Nit: no need to remove this space, I'd say...

nikomatsakis · 2017-08-29T14:13:57Z

Status update: doesn't fully work for early bound regions yet. Have to investigate to see why.

nikomatsakis

Lookin' good! Left a few more nits. We have to solve the disappearing error problem, obviously.

nikomatsakis · 2017-09-05T16:10:23Z

src/librustc/infer/error_reporting/mod.rs

    pub fn report_region_errors(&self, errors: &Vec<RegionResolutionError<'tcx>>) {
        debug!("report_region_errors(): {} errors to start", errors.len());

        // try to pre-process the errors, which will group some of them
        // together into a `ProcessedErrors` group:
        let errors = self.process_errors(errors);

-        debug!("report_region_errors: {} errors after preprocessing", errors.len());
+        debug!("report_region_errors: {} errors after preprocessing",
+               errors.len());

        for error in errors {
            debug!("report_region_errors: error = {:?}", error);

            if !self.try_report_named_anon_conflict(&error) &&
               !self.try_report_anon_anon_conflict(&error) {


we should rename try_report_anon_anon_conflict() to try_report_different_lifetime_conflict()

nikomatsakis · 2017-09-05T16:12:42Z

src/librustc/infer/error_reporting/util.rs

-    // and returns the DefId and the BoundRegion corresponding to the given region.
-    pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> Option<FreeRegionInfo> {
+    // This method returns the DefId and the BoundRegion corresponding to the given region.
+    pub fn is_suitable_region(&self, region: Region<'tcx>) -> Option<FreeRegionInfo> {


Maybe we can call this is_suitable_free_region()? Seems a touch more informative.

nikomatsakis · 2017-09-05T16:13:00Z

src/test/compile-fail/region-lifetime-bounds-on-fns-where-clause.rs

@@ -15,7 +15,7 @@ fn a<'a, 'b>(x: &mut &'a isize, y: &mut &'b isize) where 'b: 'a {

 fn b<'a, 'b>(x: &mut &'a isize, y: &mut &'b isize) {
    // Illegal now because there is no `'b:'a` declaration.
-    *x = *y; //~ ERROR E0312
+    *x = *y; //~ ERROR E0623


nikomatsakis · 2017-09-05T16:14:11Z

src/test/compile-fail/regions-free-region-ordering-caller.rs

@@ -15,20 +15,16 @@
 struct Paramd<'a> { x: &'a usize }

 fn call2<'a, 'b>(a: &'a usize, b: &'b usize) {
-    let z: Option<&'b &'a usize> = None;
-    //~^ ERROR reference has a longer lifetime than the data it references
+    let z: Option<&'b &'a usize> = None;//~ ERROR E0623


I guess that the error in this case could be a touch misleading -- we'll say that "data flows from a into b here" or whatever, when in fact no dataflow actually occurs. This seems kind of like an edge case though. I'm inclined to leave it as is for now. We may want to reconsider in the future though. Maybe worth opening an issue about at some point.

nikomatsakis · 2017-09-05T16:14:43Z

src/test/compile-fail/regions-outlives-projection-container-hrtb.rs

@@ -40,7 +40,6 @@ fn with_assoc<'a,'b>() {
    // We get an error because 'b:'a does not hold:

    let _: &'a WithHrAssoc<TheType<'b>> = loop { };
-    //~^ ERROR reference has a longer lifetime


indeed, this error should not disappear -- when I investigated a bit, it appeared we were ICEing...?

nikomatsakis · 2017-09-05T16:14:51Z

src/test/compile-fail/regions-outlives-projection-container-hrtb.rs

@@ -61,7 +60,7 @@ fn with_assoc_sub<'a,'b>() {
    // below to be well-formed, it is not related to the HR relation.

    let _: &'a WithHrAssocSub<TheType<'b>> = loop { };
-    //~^ ERROR reference has a longer lifetime


error should not disappear

nikomatsakis · 2017-09-05T16:14:55Z

src/test/compile-fail/regions-outlives-projection-container-wc.rs

@@ -42,7 +42,7 @@ fn with_assoc<'a,'b>() {
    // which is &'b (), must outlive 'a.

    let _: &'a WithAssoc<TheType<'b>> = loop { };
-    //~^ ERROR reference has a longer lifetime


error should not disappear

nikomatsakis · 2017-09-05T16:15:01Z

src/test/compile-fail/regions-outlives-projection-container.rs

 }

 fn call_without_assoc<'a,'b>() {
    // As `without_assoc`, but in a distinct scenario.

-    call::<&'a WithoutAssoc<TheType<'b>>>(); //~ ERROR reference has a longer lifetime
+    call::<&'a WithoutAssoc<TheType<'b>>>();


error should not disappear

gaurikholkar-zz · 2017-09-08T03:09:51Z

@GuillaumeGomez, changed the diagnostics.rs for E0312

GuillaumeGomez · 2017-09-08T08:56:55Z

src/librustc/diagnostics.rs

@@ -1383,38 +1383,6 @@ struct Foo<T: 'static> {
 ```
 "##,

-E0312: r##"


The error E0312 still exists so why not writing its long error diagnostic instead of just removing the old one?

At it. Just thought you should know.

nikomatsakis · 2017-09-08T13:22:22Z

@bors r+

bors · 2017-09-08T13:22:23Z

📌 Commit 1c304d2 has been approved by nikomatsakis

bors · 2017-09-08T14:42:19Z

🔒 Merge conflict

bors · 2017-09-08T14:43:15Z

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

nikomatsakis · 2017-09-08T17:23:23Z

@bors r+

bors · 2017-09-08T17:23:24Z

📌 Commit f2ac5dd has been approved by nikomatsakis

vitiral · 2017-09-08T18:17:13Z

I was directed here from this comment.

I may be missing what this message is trying to say, but it looks like the compiler should suggest an "outlives" annotation in this case.

^ ...but data from `y` flows into `x` here`

The above message doesn't tell me what to do. I believe it is saying that foo needs to be declared as:

fn foo<'a, 'b: 'a>

since 'b needs to outlive 'a.

Could this be modified to do so, or a new issue opened to add the Help message?

nikomatsakis · 2017-09-10T10:09:21Z

@vitiral

The above message doesn't tell me what to do

Thanks for the suggestion. =) This is tricky. This message is intended, in part, as a fallback message -- with the extended error (--explain) giving advice on possible remedies. On of the bits of follow-up work we intended was to give a more concrete suggestion for how to fix things -- including possibly adding a where-clause. But that is not always appropriate (e.g., if you are implementing a trait, you may not be at liberty to add a where-clause, unless you an also modify the trait). So yes, I think opening up a follow-up issue is the way to go -- I forget if we have already done so or not, but it was the plan.

nikomatsakis · 2017-09-10T10:11:48Z

@bors r+

bors · 2017-09-10T10:11:48Z

📌 Commit 88e4bf6 has been approved by nikomatsakis

bors · 2017-09-10T15:32:25Z

⌛ Testing commit 88e4bf6 with merge b413f34...

@nikomatsakis

Extend E0623 for LateBound and EarlyBound Regions This is a fix for #43882 ``` fn foo<'a,'b>(x: &mut Vec<&'a u8>, y: &'b u8) { x.push(y); } ``` now gives ``` error[E0623]: lifetime mismatch --> $DIR/ex3-both-anon-regions-latebound-regions.rs:12:12 | 11 | fn foo<'a,'b>(x: &mut Vec<&'a u8>, y: &'b u8) { | ------ ------ these two types are declared with different lifetimes... 12 | x.push(y); | ^ ...but data from `y` flows into `x` here ``` cc @nikomatsakis @arielb1 Please ignore the second commit. It will be merged in a separate PR.

bors · 2017-09-10T18:37:22Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing b413f34 to master...

rust-highfive assigned nikomatsakis Aug 25, 2017

gaurikholkar-zz force-pushed the named_conf branch 3 times, most recently from 5823310 to 3665a7f Compare August 25, 2017 09:04

shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 25, 2017

gaurikholkar-zz force-pushed the named_conf branch from 1c9d960 to b8559a5 Compare August 26, 2017 05:38

gaurikholkar-zz changed the title ~~Extend E0623 for LateBound Regions~~ Extend E0623 for LateBound and EarlyBound Regions Aug 26, 2017

gaurikholkar-zz force-pushed the named_conf branch 2 times, most recently from 6cdae88 to a944650 Compare August 27, 2017 17:18

arielb1 reviewed Aug 28, 2017

View reviewed changes

src/librustc/infer/error_reporting/named_anon_conflict.rs Outdated

}

return true;

}

}

}

Copy link

Contributor

arielb1 Aug 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: no newline at end of file

arielb1 reviewed Aug 28, 2017

View reviewed changes

nikomatsakis requested changes Aug 28, 2017

View reviewed changes

gaurikholkar-zz force-pushed the named_conf branch 2 times, most recently from 5442498 to ef2f0e1 Compare September 2, 2017 14:54

nikomatsakis requested changes Sep 5, 2017

View reviewed changes

GuillaumeGomez reviewed Sep 8, 2017

View reviewed changes

gaurikholkar-zz force-pushed the named_conf branch from 25fa4e8 to 44810d9 Compare September 8, 2017 13:13

nikomatsakis approved these changes Sep 8, 2017

View reviewed changes

gaurikholkar-zz force-pushed the named_conf branch 2 times, most recently from f91c3fe to f2ac5dd Compare September 8, 2017 16:55

nikomatsakis mentioned this pull request Sep 8, 2017

Infer T: 'x outlives requirements on structs rust-lang/rfcs#2093

Merged

gaurikholkar-zz force-pushed the named_conf branch 2 times, most recently from 1770e90 to 9caab73 Compare September 9, 2017 03:30

gaurikholkar-zz changed the title ~~Extend E0623 for LateBound and EarlyBound Regions WIP~~ Extend E0623 for LateBound and EarlyBound Regions Sep 9, 2017

gaurikholkar-zz force-pushed the named_conf branch from 9caab73 to 09d1a43 Compare September 9, 2017 04:16

adding E0623 for LateBound regions

cfc7cf3

gaurikholkar-zz force-pushed the named_conf branch from 09d1a43 to cfc7cf3 Compare September 9, 2017 05:42

fix "correct" case in diagnostic error message

88e4bf6

bors merged commit 88e4bf6 into rust-lang:master Sep 10, 2017

This was referenced Sep 10, 2017

adding E0623 for return types - both parameters are anonymous #44124

Closed

Improve SubSupConflict with a named and an anonymous lifetime parameter #42701 #44167

Merged

		@@ -40,7 +40,6 @@ fn with_assoc<'a,'b>() {
		// We get an error because 'b:'a does not hold:

		let _: &'a WithHrAssoc<TheType<'b>> = loop { };

		\| ^ ...but data from `y` flows into `x` here

		error: aborting due to previous error

@@ @@ -193,4 +191,4 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { @@
                       }
                       false
                   }
-              }
+              }

Extend E0623 for LateBound and EarlyBound Regions #44079

Extend E0623 for LateBound and EarlyBound Regions #44079

Conversation

gaurikholkar-zz commented Aug 25, 2017 • edited Loading

rust-highfive commented Aug 25, 2017

bors commented Aug 25, 2017

shepmaster commented Aug 25, 2017

gaurikholkar-zz commented Aug 26, 2017

gaurikholkar-zz commented Aug 27, 2017 • edited Loading

gaurikholkar-zz commented Aug 28, 2017

arielb1 Aug 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 Aug 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 commented Aug 28, 2017

gaurikholkar-zz commented Aug 28, 2017

arielb1 commented Aug 28, 2017

gaurikholkar-zz commented Aug 28, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Aug 29, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaurikholkar-zz commented Sep 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Sep 8, 2017

bors commented Sep 8, 2017

bors commented Sep 8, 2017

bors commented Sep 8, 2017

nikomatsakis commented Sep 8, 2017

bors commented Sep 8, 2017

vitiral commented Sep 8, 2017 • edited Loading

nikomatsakis commented Sep 10, 2017

nikomatsakis commented Sep 10, 2017

bors commented Sep 10, 2017

bors commented Sep 10, 2017

bors commented Sep 10, 2017

gaurikholkar-zz commented Aug 25, 2017 •

edited

Loading

gaurikholkar-zz commented Aug 27, 2017 •

edited

Loading

arielb1 Aug 28, 2017 •

edited

Loading

arielb1 Aug 28, 2017 •

edited

Loading

vitiral commented Sep 8, 2017 •

edited

Loading