Rollup merge of #88336 - jackh726:gats-where-constraints, r=estebank

Manishearth · web-flow · commit 6d4f27ebc7b9 · 2021-09-12T03:44:53.000-07:00
Detect stricter constraints on gats where clauses in impls vs trait

I might try to see if I can do a bit more to improve these diagnostics, but any initial feedback is appreciated. I can also do any additional work in a followup PR.

r? `@estebank`
diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/trait_impl_difference.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/trait_impl_difference.rs
@@ -2,7 +2,7 @@
 
 use crate::infer::error_reporting::nice_region_error::NiceRegionError;
 use crate::infer::lexical_region_resolve::RegionResolutionError;
-use crate::infer::{Subtype, ValuePairs};
+use crate::infer::{SubregionOrigin, Subtype, ValuePairs};
 use crate::traits::ObligationCauseCode::CompareImplMethodObligation;
 use rustc_errors::ErrorReported;
 use rustc_hir as hir;
@@ -11,44 +11,53 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
 use rustc_middle::ty::error::ExpectedFound;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_span::{MultiSpan, Span};
+use rustc_span::{MultiSpan, Span, Symbol};
 
 impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
     /// Print the error message for lifetime errors when the `impl` doesn't conform to the `trait`.
     pub(super) fn try_report_impl_not_conforming_to_trait(&self) -> Option<ErrorReported> {
-        if let Some(ref error) = self.error {
-            debug!("try_report_impl_not_conforming_to_trait {:?}", error);
-            if let RegionResolutionError::SubSupConflict(
-                _,
-                var_origin,
-                sub_origin,
-                _sub,
-                sup_origin,
-                _sup,
-            ) = error.clone()
-            {
-                if let (&Subtype(ref sup_trace), &Subtype(ref sub_trace)) =
-                    (&sup_origin, &sub_origin)
+        let error = self.error.as_ref()?;
+        debug!("try_report_impl_not_conforming_to_trait {:?}", error);
+        if let RegionResolutionError::SubSupConflict(
+            _,
+            var_origin,
+            sub_origin,
+            _sub,
+            sup_origin,
+            _sup,
+        ) = error.clone()
+        {
+            if let (&Subtype(ref sup_trace), &Subtype(ref sub_trace)) = (&sup_origin, &sub_origin) {
+                if let (
+                    ValuePairs::Types(sub_expected_found),
+                    ValuePairs::Types(sup_expected_found),
+                    CompareImplMethodObligation { trait_item_def_id, .. },
+                ) = (&sub_trace.values, &sup_trace.values, &sub_trace.cause.code)
                 {
-                    if let (
-                        ValuePairs::Types(sub_expected_found),
-                        ValuePairs::Types(sup_expected_found),
-                        CompareImplMethodObligation { trait_item_def_id, .. },
-                    ) = (&sub_trace.values, &sup_trace.values, &sub_trace.cause.code)
-                    {
-                        if sup_expected_found == sub_expected_found {
-                            self.emit_err(
-                                var_origin.span(),
-                                sub_expected_found.expected,
-                                sub_expected_found.found,
-                                *trait_item_def_id,
-                            );
-                            return Some(ErrorReported);
-                        }
+                    if sup_expected_found == sub_expected_found {
+                        self.emit_err(
+                            var_origin.span(),
+                            sub_expected_found.expected,
+                            sub_expected_found.found,
+                            *trait_item_def_id,
+                        );
+                        return Some(ErrorReported);
                     }
                 }
             }
         }
+        if let RegionResolutionError::ConcreteFailure(origin, _, _) = error.clone() {
+            if let SubregionOrigin::CompareImplTypeObligation {
+                span,
+                item_name,
+                impl_item_def_id,
+                trait_item_def_id,
+            } = origin
+            {
+                self.emit_associated_type_err(span, item_name, impl_item_def_id, trait_item_def_id);
+                return Some(ErrorReported);
+            }
+        }
         None
     }
 
@@ -107,6 +116,25 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
         }
         err.emit();
     }
+
+    fn emit_associated_type_err(
+        &self,
+        span: Span,
+        item_name: Symbol,
+        impl_item_def_id: DefId,
+        trait_item_def_id: DefId,
+    ) {
+        let impl_sp = self.tcx().def_span(impl_item_def_id);
+        let trait_sp = self.tcx().def_span(trait_item_def_id);
+        let mut err = self
+            .tcx()
+            .sess
+            .struct_span_err(span, &format!("`impl` associated type signature for `{}` doesn't match `trait` associated type signature", item_name));
+        err.span_label(impl_sp, &format!("found"));
+        err.span_label(trait_sp, &format!("expected"));
+
+        err.emit();
+    }
 }
 
 struct TypeParamSpanVisitor<'tcx> {
diff --git a/compiler/rustc_infer/src/infer/error_reporting/note.rs b/compiler/rustc_infer/src/infer/error_reporting/note.rs
@@ -99,6 +99,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                     "...so that the definition in impl matches the definition from the trait",
                 );
             }
+            infer::CompareImplTypeObligation { span, .. } => {
+                label_or_note(
+                    span,
+                    "...so that the definition in impl matches the definition from the trait",
+                );
+            }
         }
     }
 
@@ -356,6 +362,18 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 trait_item_def_id,
                 &format!("`{}: {}`", sup, sub),
             ),
+            infer::CompareImplTypeObligation {
+                span,
+                item_name,
+                impl_item_def_id,
+                trait_item_def_id,
+            } => self.report_extra_impl_obligation(
+                span,
+                item_name,
+                impl_item_def_id,
+                trait_item_def_id,
+                &format!("`{}: {}`", sup, sub),
+            ),
         }
     }
 
diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs
@@ -427,6 +427,15 @@ pub enum SubregionOrigin<'tcx> {
         impl_item_def_id: DefId,
         trait_item_def_id: DefId,
     },
+
+    /// Comparing the signature and requirements of an impl associated type
+    /// against the containing trait
+    CompareImplTypeObligation {
+        span: Span,
+        item_name: Symbol,
+        impl_item_def_id: DefId,
+        trait_item_def_id: DefId,
+    },
 }
 
 // `SubregionOrigin` is used a lot. Make sure it doesn't unintentionally get bigger.
@@ -1810,6 +1819,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
             ReferenceOutlivesReferent(_, a) => a,
             CallReturn(a) => a,
             CompareImplMethodObligation { span, .. } => span,
+            CompareImplTypeObligation { span, .. } => span,
         }
     }
 
@@ -1833,6 +1843,17 @@ impl<'tcx> SubregionOrigin<'tcx> {
                 trait_item_def_id,
             },
 
+            traits::ObligationCauseCode::CompareImplTypeObligation {
+                item_name,
+                impl_item_def_id,
+                trait_item_def_id,
+            } => SubregionOrigin::CompareImplTypeObligation {
+                span: cause.span,
+                item_name,
+                impl_item_def_id,
+                trait_item_def_id,
+            },
+
             _ => default(),
         }
     }
diff --git a/src/test/ui/generic-associated-types/impl_bounds.rs b/src/test/ui/generic-associated-types/impl_bounds.rs
@@ -15,7 +15,7 @@ impl<T> Foo for Fooy<T> {
     type A<'a> where Self: 'static = (&'a ());
     //~^ ERROR the parameter type `T` may not live long enough
     type B<'a, 'b> where 'b: 'a = (&'a(), &'b ());
-    //~^ ERROR lifetime bound not satisfied
+    //~^ ERROR `impl` associated type
     //~| ERROR lifetime bound not satisfied
     type C where Self: Copy = String;
     //~^ ERROR the trait bound `T: Copy` is not satisfied
diff --git a/src/test/ui/generic-associated-types/impl_bounds.stderr b/src/test/ui/generic-associated-types/impl_bounds.stderr
@@ -5,24 +5,16 @@ LL |     type A<'a> where Self: 'static = (&'a ());
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding an explicit lifetime bound `T: 'static`...
-   = note: ...so that the type `Fooy<T>` will meet its required lifetime bounds
+   = note: ...so that the definition in impl matches the definition from the trait
 
-error[E0478]: lifetime bound not satisfied
+error: `impl` associated type signature for `B` doesn't match `trait` associated type signature
   --> $DIR/impl_bounds.rs:17:5
    |
+LL |     type B<'a, 'b> where 'a: 'b;
+   |     ---------------------------- expected
+...
 LL |     type B<'a, 'b> where 'b: 'a = (&'a(), &'b ());
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-note: lifetime parameter instantiated with the lifetime `'b` as defined on the associated item at 17:16
-  --> $DIR/impl_bounds.rs:17:16
-   |
-LL |     type B<'a, 'b> where 'b: 'a = (&'a(), &'b ());
-   |                ^^
-note: but lifetime parameter must outlive the lifetime `'a` as defined on the associated item at 17:12
-  --> $DIR/impl_bounds.rs:17:12
-   |
-LL |     type B<'a, 'b> where 'b: 'a = (&'a(), &'b ());
-   |            ^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found
 
 error[E0478]: lifetime bound not satisfied
   --> $DIR/impl_bounds.rs:17:5
diff --git a/src/test/ui/generic-associated-types/issue-86787.stderr b/src/test/ui/generic-associated-types/issue-86787.stderr
@@ -9,7 +9,7 @@ LL | |     <Left as HasChildrenOf>::T: 'a,
 LL | |     <Right as HasChildrenOf>::T: 'a
    | |                                    - help: consider adding a where clause: `, <Left as HasChildrenOf>::T: 'a`
 LL | |     = Either<&'a Left::T, &'a Right::T>;
-   | |________________________________________^ ...so that the type `<Left as HasChildrenOf>::T` will meet its required lifetime bounds
+   | |________________________________________^ ...so that the definition in impl matches the definition from the trait
 
 error[E0309]: the associated type `<Right as HasChildrenOf>::T` may not live long enough
   --> $DIR/issue-86787.rs:23:5
@@ -22,7 +22,7 @@ LL | |     <Left as HasChildrenOf>::T: 'a,
 LL | |     <Right as HasChildrenOf>::T: 'a
    | |                                    - help: consider adding a where clause: `, <Right as HasChildrenOf>::T: 'a`
 LL | |     = Either<&'a Left::T, &'a Right::T>;
-   | |________________________________________^ ...so that the type `<Right as HasChildrenOf>::T` will meet its required lifetime bounds
+   | |________________________________________^ ...so that the definition in impl matches the definition from the trait
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/generic-associated-types/missing-where-clause-on-trait.rs b/src/test/ui/generic-associated-types/missing-where-clause-on-trait.rs
@@ -0,0 +1,13 @@
+// check-fail
+
+#![feature(generic_associated_types)]
+
+trait Foo {
+    type Assoc<'a, 'b>;
+}
+impl Foo for () {
+    type Assoc<'a, 'b> where 'a: 'b = ();
+    //~^ `impl` associated type
+}
+
+fn main() {}
diff --git a/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr b/src/test/ui/generic-associated-types/missing-where-clause-on-trait.stderr
@@ -0,0 +1,11 @@
+error: `impl` associated type signature for `Assoc` doesn't match `trait` associated type signature
+  --> $DIR/missing-where-clause-on-trait.rs:9:5
+   |
+LL |     type Assoc<'a, 'b>;
+   |     ------------------- expected
+...
+LL |     type Assoc<'a, 'b> where 'a: 'b = ();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found
+
+error: aborting due to previous error
+

Original file line number	Diff line number	Diff line change
`@@ -99,6 +99,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {`
`99`	`99`	`"...so that the definition in impl matches the definition from the trait",`
`100`	`100`	`);`
`101`	`101`	`}`
	`102`	`+ infer::CompareImplTypeObligation { span, .. } => {`
	`103`	`+ label_or_note(`
	`104`	`+ span,`
	`105`	`+ "...so that the definition in impl matches the definition from the trait",`
	`106`	`+ );`
	`107`	`+ }`
`102`	`108`	`}`
`103`	`109`	`}`
`104`	`110`
`@@ -356,6 +362,18 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {`
`356`	`362`	`trait_item_def_id,`
`357`	`363`	&format!("`{}: {}`", sup, sub),
`358`	`364`	`),`
	`365`	`+ infer::CompareImplTypeObligation {`
	`366`	`+ span,`
	`367`	`+ item_name,`
	`368`	`+ impl_item_def_id,`
	`369`	`+ trait_item_def_id,`
	`370`	`+ } => self.report_extra_impl_obligation(`
	`371`	`+ span,`
	`372`	`+ item_name,`
	`373`	`+ impl_item_def_id,`
	`374`	`+ trait_item_def_id,`
	`375`	+ &format!("`{}: {}`", sup, sub),
	`376`	`+ ),`
`359`	`377`	`}`
`360`	`378`	`}`
`361`	`379`