Finalize repeat expr inference behaviour with inferred repeat counts by BoxyUwU · Pull Request #139635 · rust-lang/rust (original) (raw)

Background context

I would like to move forwards with stabilizing generic_arg_infer soon, in preparation for that I'd like us to decide on the specifics of how type inference should behaved around inferred repeat expr count ahead of time:

fn foo(a: &mut [u8; 32]) { *a = [10; _]; }

This introduces some complexities around handling the check that the element type implements Copy. There are a few options for how to handle this:

Forbid _ as repeat expr counts

Status quo on stable without generic_arg_infer. I think it would be unfortunate to forbid this, it "feels" inconsistent to allow inferring array lengths and const generic arguments but not repeat expr counts.

Conservatively require Copy

When encountering _ as the length of an array repeat expr we could conservatively assume it will wind up being a length >=1 and require the element type implements Copy.

This would result in confusing errors in cases where the repeat expr count would eventually wind up inferred to 0 or 1 and the element type does not implement Copy:

fn foo() { let _: [String; 1] = ["my_string".to_string(), _]; //~^ ERROR: The trait bound String: Copy is not satisfied }

It can also result in weird behaviour where by conservatively requring Copy as part of a RepeatExprCopy(Foo<?m>, ?n) check, the Copy impl might wind up inferring ?n=1 removing the need to have ever required Copy in the first place.

A side effect of this is that region constraints from the Copy goal would never wind up proven, as the built mir would have ?n=1, and the MIR typecker has no need to require Copy for a repeat expr with length 1. This is not a soundness issue as Copy isn't actually required since the length is 1.

Altogether this solution exposes users to surprising Copy requirements on element types and weird errors from arbitrary inference guidance. So I do not believe this is a good option.

Defer repeat expr Copy checks until the end of typechecking

When encountering repeat exprs we could just defer the copy check to the end of type checking when in all likelyhood the array length will have been inferred.

There are a number of subtleties around how this could be implemented:

Defer all or only some checks

Defering all checks is technically a regression in type inference as proving Copy might have inference side effects that are required for method lookup or field accesses to succeed. By deferring the check to the end of typeck, the inference side effects will also be deffered:

To avoid this we could only defer checks in cases where the repeat expr count is inferred with a _. There were no crater regressions detected from always deferring Copy checks (crater run).

Test that fails when all copy checks are deferred

Before or after integer fallback

Repeat expr copy checks that have been deferred to the end of type checking can either be run before or after we fallback integer variables to i32.

Neither option is strictly more permissive than the other. Running checks before integer fallback can fail due to not arbitrarily deciding a type should be an integer and so having ambiguity. Running checks after fallback can fail due to arbitrarily inferring an int variable to i32 when it would have otherwise been inferred by copy checks.

Performing repeat expr copy checks both before and after inference fallback would be the best of both worlds; integer fallback only affecting repeat expr checks if they would have failed without it.

Test that checks occur before integer fallback
Test that checks occur after integer fallback

Perform the checks in a loop, sequentially or "isolated"

As proving Copy can have inference side effects, it's possible for the order that repeat exprs are checked to affect whether an error is emitted or not. We could potentially avoid such weird inference errors by running the repeat expr copy checks in a loop until we stop making inference progress.

We could also avoid any kind of order-dependence by first erroring on uninferred repeat counts then proving copy bounds, disallowing repeat expr copy requirements' inference side effects from affecting the repeat counts of other repeat exprs (this would result in both examples failing to compile).

Test that checks how inference side effects from repeat expr checks behave

If we were to adopt this solution I believe we should pick the most conservative/weakest inference options:

This largely minimizes implementation complexity while also being powerful enough to work in the large majority of cases.

The inference shortcomings of this are relatively "simple" compared to other choices which would result in weirder failure modes. I believe these inference shortcomings are also unlikely to be encountered in the wild as Copy impls with inference constraints are unlikely to be encountered.

I prefer performing repeat expr checks before integer fallback instead of after (even though neither is strictly more powerful than the other) as the failure mode is less confusing. An error due to weak inference should be easier for users to understand (and resolve) compared to an error due to an arbitrary inference choice that could have been unnecessary.

I also believe that if we would like for inference around repeat expr checks to be stronger then we should adopt the next solution (adding a new predicate kind) which will be maximally flexible, as opposed to introducing many hacks to how we check repeat exprs which will never be as flexible.

Deferring repeat expr checks to the end of typeck with the impl nuances resolved the way I specified above would be my preferred solution.

Introduce a new obligation for repeat expr checks

We could introduce a new PredicateKind::RepeatExprCopyCheck(Ty<'tcx>, Const<'tcx>) which handles the logic for requiring Ty: Copy if the length is not 0 or 1.

This would have "optimal" inference behaviour as obligations can stall on inference variables, while also being able to proven at arbitrary points in type checking when inference progress is required.

It effectively resolves all of the subtleties of the previous solution in favour of maximal flexibility (more so than the previous solution is capable of). The check would be deferred exactly as much as necessary without hindering inference, and there would be no weird order dependent behaviour of repeat expr checks (ignoring any existing incompleteness in the trait system).

I think this would be an acceptable solution, though im not convinced the additional power is actually worth it compared to the implementation complexity.

One final subtlty of inference around inferred repeat exprs is how we handle repeat exprs with inferred counts and element exprs that are consts. On stable we do not require Copy for element types in those cases:

fn foo() { let a = [const { String::new() }; 2]; }

I have implemented things such that we avoid doing any repeat count related checks if the element expr is a constant. I don't think this needs too much justification as it feels somewhat "obvious" that it would be weird to not care about the repeat count (because the element is a constant) while also erroring due to the repeat count being unknown at the time of the repeat count check (see this test which would otherwise fail).

Note that we do still require the length to be eventually inferred, it is only that the repeat expr check does not require it to be inferred in cases where the element expr is a constant (see const_block_but_uninferred in this test).

Concrete FCP Summary

While I don't intend this to be an FCP for stabilizing generic_arg_infer (this would need lang sign off) I expect this to cover the behaviour of inference of inferred repeat counts and as such not need re-litigating as part of stabilization of generic_arg_infer (i.e. there will not be a need for a second types FCP later).

Also, please note that this is about repeat expressions not array lengths. None of this matters for array types such as let a: [u8; _];.

@rfcbot fcp merge