impl Trait in argument position

[nikomatsakis]

(Part of #34511)

As part of the latest impl Trait RFC, we decided to accept impl Trait in argument position:

fn foo(x: impl Iterator<Item = u32>) {
    ...
}

this is roughly equivalent to fn foo<T: Iterator<Item = u32>>(t: T), except that explicitly specifying the type is not permitted (i.e., foo::<u32> is an error).

Mentoring instructions can be found here.

Questions to resolve:

• Should we permit specifying types if some parameters are implicit and some are explicit? e.g., fn foo<T>(x: impl Iterator<Item = T>>)? I think yes, foo::<u32> would be accepted (thus binding T = u32 explicitly).
  • Answer: for now, just forbid explicit bindings altogether in the presence of impl Trait arguments.

[petrochenkov]

except that explicitly specifying the type is not permitted (i.e., foo::<u32> is an error).

This is a showstopper, IMO.
Suppose you write a function in the sugary form first, but then need to make the parameter explicit for some reason. Or you want to use the sugar for some already existing code.
You can't do it, because with this rule impl Trait in args is not actually a sugar and using (or un-using) it breaks code.

[est31]

As part of the latest impl Trait RFC, we decided to accept impl Trait in argument position

Am I going to expect a shift in what is considered "idiomatic" Rust by this? As in, will the new rule say that you should almost always use impl Trait? I don't really care if its an optional feature, but I don't think impl Trait is in any way better or nicer than actual generics.

[nikomatsakis]

@petrochenkov as a safe starting point, we can simply make it illegal to use explicit argument lists if impl Trait is used in argument position.

[nikomatsakis]

I'd prefer not to debate too much the "policy" of this question in this particular issue, since I think this should focus more on how to implement.

[nikomatsakis]

Mentoring instructions

The goal is to desugar impl Trait during HIR lowering -- or at least partially.

Basically, we will make it so that when the AST looks like this:

fn foo(x: impl Iterator) { ... }

The HIR looks like this:

fn foo<T: Iterator>(x: T) { ... }

We have to be careful though. For example, we don't want people to be able to explicitly supply type arguments (e.g., foo::<U>) for impl Iterator arguments -- or at least we don't know if we want to. So, for now, if there are any "synthetic type parameters" of this kind introduced, we want to issue an error.

First step:

Introduce the idea of a synthetic type parameter into the HIR. We could do this in a few ways. One would be to have a flag on the HIR Generics struct indicating if synthetic things were added at all. But it strikes me as mildly better to add the flag to the TyParam struct that represents a type parameter, somewhat similar to the existing pure_wrt_drop field (which is completely orthogonal). Probably the ideal would be to introduce a field synthetic of type SyntheticKind:

enum SyntheticKind { ImplTrait }

This way, if we add more kinds of synthetic type parameters in the future, we can track why they are synthetic for better error messages. =)

To allow us to write unit tests, we can add a testing attribute #[rustc_synthetic] that can be applied to type parameters. In HIR lowering, we can check for the presence of this attribute and set the flag to true, just as we do now for the pure_wrt_drop field (which is otherwise completely orthogonal from this work). (To add this attribute, you will need to add some code to feature_gate.rs, analogous to the code that exists for #[rustc_variance].) We could either add a special kind for this ("UnitTest") or just use ImplTrait. Whatever.

Next, we need to adjust the ty::TypeParameterDef struct and add a similar field. This struct is the compiler's semantic representation of the type parameters (whereas the HIR struct is meant to represent the syntax that the user wrote). As an example of how they are different, we only have HIR structs for the current crate, but we can load ty::TypeParameterDef from other crates that have already been compiled. Anyway, we want to add the same field, and initialize it from this code in collect.rs, which is the code that converts a hir::TypeParameterDef into a ty::TypeParameterDef.

OK, now, finally, we want to make it an error to specify explicit type parameters for some value (e.g., a function) if any of the type parameters on that function are synthetic. To do that, we want to modify the function that resolves paths during typeck, which is the instantiate_value_path function. I think we want to add a check that takes place after these existing checks. Here, the variables type_segment and fn_segment contain the generic type parameter declarations (if any) for the path being referenced (in the case of a path to the function foo, fn_segment would be populated and type_segment would be empty; in the case of a path like HashSet::<u32>::contains::<u32>, the type_segment would contain the generic parameter T from HashSet, and the fn_segment would contain the generic parameter Q from contains()).

What we want to do is to iterate through all the TypeParameterDefs from both segments and check if any of them are synthetic: if so, we can issue an error, using some code like:

let err = match synthetic_kind {
  hir::SyntheticKind::ImplTrait => {
    struct_span_err!(
      self.tcx.sess,
      span,
      EXXX, // we'll have to add a new error code...
      "cannot provide explicit type parameters when `impl Trait` is used in argument position")
  }
};
// bonus (described below) would go here
err.emit();

For bonus points, we could include a err.span_label() call on the error to point at where impl Trait is used in the function definition (or just the function definition itself), but that would a touch of plumbing.

With all this, we should be able to add a test like this (maybe named src/test/compile-fail/synthetic-param.rs):

fn foo<#[rustc_synthetic] T>(_: T) {
}

fn main() {
    let x = foo::<u32>; //~ ERROR cannot provide explicit type parameters
    foo(22); // ok
}

This seems like enough for a first PR!

[nikomatsakis]

I'm tagging this as WG-compiler-middle and WG-compiler-traits since I think it could go either way.