Default Type Parameter Fallback: Revival

[leoyvens]

See #27336 for background. This is about using type parameter defaults to inform inference. This was once implemented in #26870. It turns out there were issues with the design, the implementation was finally removed in #40570. But this feature is awesome and we should not let it die.

As discussed in the tracking issue, we want adding defaults to type parameters to be backwards compatible, but the original design did not take this into account. @eddyb suggests that we future-proof this by not applying a fallback where a conflict could possibly occur. That would make defaults on fns and impls pretty much useless. So I suggested that we invert the situation and give priority to the defaults on fns and impls, taking care to be future-proof. For details of how this is done see the comments and implementation in fn apply_user_type_parameter_fallback. The tests show what works and what doesn't under this design. Numeric and diverging fallbacks take precedence over user supplied fallbacks.

Does this need an RFC? Does the RFC block accepting the PR? Should it be an ammendment to RFC 213?

A gotcha:
As commented in the test trait_impl.rs, if a type var is partially inferred then it's default will not be considered. For example if we know that T == Vec<U> then the default for T will be ignored while the default of U may apply. This avoids conflicting defaults and is consistent with not applying user fallbacks for numerics, a numeric var can be seen as having been partially inferred.

Error messages:
If fallback fails, we preserve the error message that would occur if no fallback was present, so "type annotations needed" or "the type of this value must be known in this context". A custom error message would be complicated to craft and would not be useful as most of the time the only thing the user can do is annotate a type.
To preserve error messages I need to rollback any constraints made when trying to solve defaults. Currently everything is rolled back on any failure, throwing away any successes along with it. This is bad but we can easily implement a heuristic that solves the easy cases and bundles only the hard cases together, making pathological cases rare. So I'm satisfied with this strategy.

Todo:

1. Implement the heuristic for avoiding rolling back successes.
2. Not working for defaults in type aliases.
3. Fix interaction with specialization described below.
4. Decide if the behaviour in trait_def_rules_over_impl.rs is expected. If so, forbid writing defaults in implementations of trait methods.

The first commits are a rebase an old branch by @jroesch and @nikomatsakis. I ended up not salvaging much, but some refactorings were useful.

Disclaimer: I'm learning as I go, I'm not really familiar with any of this stuff.

[rust-highfive]

r? @petrochenkov

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

[eddyb]

r? @nikomatsakis

[leoyvens]

@kennytm tidy thinks some .rs test files are binary files, what do?

[kennytm]

@leodasvacas Run chmod a-x path/of/those/files.rs and then commit them again.

[bors]

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

[leoyvens]

I've tested the interaction with specialization, and the following interaction is concerning:

#![feature(default_type_parameter_fallback)]
#![feature(specialization)]

use std::fmt::Debug;

trait B { fn b(&self) -> Self; }

impl<T=String> B for Option<T> where T: Default
{
    default fn b(&self) -> Option<T> {
        Some(T::default())
    }
}
// When there specialized but generic impls, their defaults
// are ignored no matter what they are.
// This code does not compile because `x` in main fails to infer.
// However if we commented out this impl, `x` would be inferred to `String`.
impl<T=String> B for Option<T> where T: Default + Debug
{
    fn b(&self) -> Option<T> { Some(T::default()) }
}

fn main() {
    let x = None;
    let y = x.b();
}

This is bad because even though it's normal for new trait impls to break inference, in this case we are completely ignoring the defaults, which means fixing this to consider the defaults is not backwards compatible, so it goes in the "todo before stabilization" list.

Options:

1. Pick an impl and apply its default. The most specialized impl? The most general impl? What about lattice specialization, all of the most general candidate impls must agree?
2. Apply the defaults of all candidate impls. Have specialized impls always inherit the default from the parent impls.

I'm not familiar with trait selection so I don't know what is feasible here.

Edit: I think the solution would be to use the default of the most general candidate impl, or impls in the case of lattices.

[nikomatsakis]

Sorry for not commenting. It was thanksgiving, and now I've been traveling. I haven't had a chance to look or think deeply about this proposal, but I feel some level of skepticism. This isn't specific to the PR in particular. Rather, I've become skeptical on the idea of defaulted type parameters in general, and also because I feel like we've been approaching their design in a bit of a ... haphazard way. I'd like to see an RFC (or at least "proto RFC") that clearly lays out the motivation for such changes, enumerating the use cases we are trying to fix and those we are not, and also lays out the possible dangers for "incoherence" and describes how they are addressed. That said, I don't want to have pure stop energy here, I also like the idea (in general) of iterating in tree, so I'll try to give this PR the time it deserves in a day or two!

[leoyvens]

@nikomatsakis Thanks for the comment! I'll write that pre-RFC, but it may take some time. Also it's easier to discuss an RFC with an implementation available on nightly.

It's seems better to focus on having a good story for adding a new type parameter with a default rather than on adding defaults to existing parameters. This makes the feature simpler and more powerful, and we don't really lose any use cases. I'll flesh out the full design in an RFC. This PR remains entirely relevant anyways, it's a small adjustment to the algorithm.

[shepmaster]

Ping from triage! It's been over 7 days since we heard from reviewer @nikomatsakis. Please assign a new reviewer @rust-lang/compiler.

[kennytm]

Ping from triage @nikomatsakis! I see that you commented in #46714 (comment). Is there any update from that?

[leoyvens]

It's not clear whether this needs an RFC, but some structured rationale would help this PR. Niko and me will schedule a chat to see how to move this forward. Thanks triage guys for the pings!

[bors]

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

[alexcrichton]

@leodasvacas looks like there may be some merge conflicts now?

[leoyvens]

@alexcrichton Thanks for the triage! This is S-blocked at the moment as Niko and me still need to schedule a chat.

[leoyvens]

After meeting with Niko we've decided it's best not to land anything until we get a new design through as an RFC. So closing until that happens.