Finalize repeat expr inference behaviour with inferred repeat counts

[BoxyUwU]

I believe this should be the last change of how repeat exprs are handled before it's finished for generic_arg_infer. Assuming we don't wind up deciding to replace this all with a new predicate kind :)

This PR has three actual changes:

• Always defer the checks to end of typeck even when generic arg infer is not enabled (needs an FCP)
• Properly handle element exprs that are constants when the repeat count is inferred
• "Isolate" each repeat expr check so that inference constraints from Copy goals dont affect other repeat expr checks resulting in weird order-dependent inference

The commit history and tests should be relatively helpful for understanding this PR's impl.

r? compiler-errors

[BoxyUwU]

I changed this to an explicit match instead of the to_target_usize().is_none_or() as the is_none_or felt like somewhat subtle logic given the soundness sensitive nature of how we should be handling each variant 🤔

[BoxyUwU]

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:

• Error on _ as we need to structurally match on the length
• Conservatively require Copy when the repeat expr count is uninferred
• Defer the check to a fixed location at the end of type checking
• Introduce a new obligation for checking repeat expr wfness

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
• Before or after integer fallback
• Perform the checks in a loop, sequentially, or "isolated"

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:

• Unconditionally defer the checks even if the length is already inferred
  • Technically breaking but no regressions found
• Perform the checks before integer fallback
• "Isolate" each repeat expr check, disallowing inference side effects to affect other repeat expr checks

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

• Are we fine with the third solution (deferring repeat expr checks to end of typeck) with the outlined choices for each impl nuance. This is my preference as I don't expect the improved inference from the fourth solution to really come up in practice.
• Are we fine with the breaking change to type inference involving Copy goals from repeat exprs is acceptable (crater report with no breakage detected)
• Are we fine with the specific behaviour of allowing repeat expr checks to pass with uninferred repeat counts when the element expr is a const (test example)

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

[rfcbot]

Team member @BoxyUwU has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[compiler-errors]

gamer

[lcnr]

Introduce a new obligation for repeat expr checks

To my understanding this will not be necessary once we have stable marker traits, will it? At this point we could have

#[marker]
trait NonConstValueRepeatExpr {}
impl<T: Copy, const N: usize> ValidRepeatExpr for [T; N] {}
impl<T> ValidRepeatExpr for [T; 0] {}
impl<T> ValidRepeatExpr for [T; 1] {}

This would then need some diagnostics handling/#[diagnostics::on_unimplemented] annotations, but feels otherwise fairly clean.

Given that marker traits aren't stable and their stabilization is blocked on the next-generation trait solver, I feel happy with this current approach of deferring them in HIR typeck.

@rfcbot reviewed

[BoxyUwU]

Indeed, "add a new obligation for repeat expr checks" can be replaced with "add a lang item'd trait in core" one day. should have put that in the FCP :)

[bors]

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

[jackh726]

Reading through the summary (btw @BoxyUwU very nice), I was fairly concerned about special casing deferring Copy. But, given that it seems like the principled solution is marker traits, and I think that's something we will eventually get to, I'm okay with this.

I would like a FIXME or an issue opened something so we have some way to know to come back to this with marker traits. But, checking my box.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[BoxyUwU]

I'll make sure to open an issue once the FCP completes 🤔 and probably add a FIXME(#18439339393) in the source where we check the deferred checks

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[BoxyUwU]

Opened an issue to track looking at making inference around repeat expr counts stronger once marker traits are further along #140855. Also rebased. This should be ready now that the FCP is complete.

[lcnr]

r=me after nits

[BoxyUwU]

@bors r=lcnr

[bors]

📌 Commit cc10370 has been approved by lcnr

It is now in the queue for this repository.