Misleading diagnostic span using trait projections in arguments

[alexcrichton]

I came across an interesting case today where the span for a diagnostic isn't quite leading to the best location. When compiling the example code below

struct A;

trait IntoWasmAbi {
    type Abi;
    fn into_abi(self) -> Self::Abi;
}

type T = <A as IntoWasmAbi>::Abi;

fn foo(a: T) {}

you get:

error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied
  --> src/lib.rs:10:1
   |
10 | fn foo(a: T) {}
   | ^^^^^^^^^^^^^^^ the trait `IntoWasmAbi` is not implemented for `A`

but it's sort of misleading because neither A nor IntoWasmAbi is mentioned in the highlight. It'd be ideal if the error would point to the type definition to indicat that's where the source of the error comes from.

[estebank]

The following is where the obligation could be changed from MiscObligation to something more specific:

rust/src/librustc/infer/mod.rs

Lines 1528 to 1550 in c20654e

	/// Normalizes associated types in `value`, potentially returning
	/// new obligations that must further be processed.
	pub fn partially_normalize_associated_types_in<T>(
	&self,
	span: Span,
	body_id: hir::HirId,
	param_env: ty::ParamEnv<'tcx>,
	value: &T,
	) -> InferOk<'tcx, T>
	where
	T: TypeFoldable<'tcx>,
	{
	debug!("partially_normalize_associated_types_in(value={:?})", value);
	let mut selcx = traits::SelectionContext::new(self);
	let cause = ObligationCause::misc(span, body_id);
	let traits::Normalized { value, obligations } =
	traits::normalize(&mut selcx, param_env, cause, value);
	debug!(
	"partially_normalize_associated_types_in: result={:?} predicates={:?}",
	value, obligations
	);
	InferOk { value, obligations }
	}

And here is where we could peek at that new obligation and extend it with further information.

rust/src/librustc/traits/project.rs

Lines 417 to 455 in c20654e

	/// The guts of `normalize`: normalize a specific projection like `<T
	/// as Trait>::Item`. The result is always a type (and possibly
	/// additional obligations). If ambiguity arises, which implies that
	/// there are unresolved type variables in the projection, we will
	/// substitute a fresh type variable `$X` and generate a new
	/// obligation `<T as Trait>::Item == $X` for later.
	pub fn normalize_projection_type<'a, 'b, 'tcx>(
	selcx: &'a mut SelectionContext<'b, 'tcx>,
	param_env: ty::ParamEnv<'tcx>,
	projection_ty: ty::ProjectionTy<'tcx>,
	cause: ObligationCause<'tcx>,
	depth: usize,
	obligations: &mut Vec<PredicateObligation<'tcx>>,
	) -> Ty<'tcx> {
	opt_normalize_projection_type(selcx, param_env, projection_ty.clone(), cause.clone(), depth,
	obligations)
	.unwrap_or_else(move || {
	// if we bottom out in ambiguity, create a type variable
	// and a deferred predicate to resolve this when more type
	// information is available.
	let tcx = selcx.infcx().tcx;
	let def_id = projection_ty.item_def_id;
	let ty_var = selcx.infcx().next_ty_var(
	TypeVariableOrigin {
	kind: TypeVariableOriginKind::NormalizeProjectionType,
	span: tcx.def_span(def_id),
	},
	);
	let projection = ty::Binder::dummy(ty::ProjectionPredicate {
	projection_ty,
	ty: ty_var
	});
	let obligation = Obligation::with_depth(
	cause, depth + 1, param_env, projection.to_predicate());
	obligations.push(obligation);
	ty_var
	})
	}

Doing the naïve change cause.span = tcx.def_span(tcx.associated_item(projection_ty.item_def_id).container.id()) would cause this error to output:

error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied
 --> file.rs:3:1
  |
3 | / trait IntoWasmAbi {
4 | |     type Abi;
5 | |     fn into_abi(self) -> Self::Abi;
6 | | }
  | |_^ the trait `IntoWasmAbi` is not implemented for `A`

The proper solution would require to track types (which would probably need #21934 to be fixed), but would ideally look like this:

error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied
 --> file.rs:9:1
  |
1 |   struct A;
  |   -------- this struct doesn't implement `IntoWasmAbi`
2 | 
3 | / trait IntoWasmAbi {
4 | |     type Abi;
5 | |     fn into_abi(self) -> Self::Abi;
6 | | }
  | |_^ this trait is not implemented for `A`
7 | 
8 |   type T = <A as IntoWasmAbi>::Abi;
  |   -------------------------------- `IntoWasmAbi` expected to be implemented for `A` because of this
  | 
9 |   fn foo(a: T) {}
  |             - `IntoWasmAbi` is not implemented for `A`

[estebank]

@rust-lang/lang: we could assert well-formedness of hir::ItemKind::TyAlias early in check_item_well_formed (we don't today, we just skip them 👀), which would turn this case into the following more understandable output:

error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied
 --> file2.rs:8:10
  |
8 | type T = <A as IntoWasmAbi>::Abi;
  |          ^^^^^^^^^^^^^^^^^^^^^^^ the trait `IntoWasmAbi` is not implemented for `A`

error[E0277]: the trait bound `A: IntoWasmAbi` is not satisfied
  --> file2.rs:10:1
   |
10 | fn foo(a: T) {}
   | ^^^^^^^^^^^^^^^ the trait `IntoWasmAbi` is not implemented for `A`

Is this something we would consider doing, even if it is as a warning? I believe this would be effectively a breaking change (code that compiles today would stop compiling) but not "in effect" because the previously compiling code could never be used. This seems to me like an accident to me.

For example, there's a test type Y where i32: Foo = (); // OK - bound is ignored which would fail with "the trait bound i32: Foo is not satisfied" with my proposed change, which is the correct behavior of all other items.

---

Edit: I see now that because we explicitily do not accept trait bounds on types arguments, we cannot really perform the wf check always, otherwise we would need to allow trait bounds (and either propagate them or continue ignoring them but now without a warning) so that type T<X: Trait> = <X as Trait>::Foo; is allowed, or only do a wf check when there are no generic types involved (which feels like a hack, but a valid one that will expand to cover other invalid cases in the future). Thoughts?

---

Edit: #69741 has the simplest change that should be safe to introduce based on my current understanding of the situation.