Defer repeat expr Copy checks to end of type checking by BoxyUwU · Pull Request #137045 · rust-lang/rust (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation18 Commits3 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

BoxyUwU

Fixes #110443

Defers repeat expr checks that the element type is Copy when the length is > 1 (or generic) to end of typeck so that under generic_arg_infer repeat exprs are able to have an inferred count, e.g. let a: [_; 1] = [String::new(); _];.

Currently the deferring is gated under generic_arg_infer though I intend to separately types FCP deferring the checks even outside of generic_arg_infer if we wind up not going with an alternative.

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Feb 14, 2025

@BoxyUwU

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 14, 2025

@bors

…

Defer repeat expr Copy checks to end of type checking

Fixes rust-lang#110443

r? @ghost

BoxyUwU
fn main() {
let x = [Foo(PhantomData); 2];
//~^ ERROR: type annotations needed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always deferring the check is a breaking change in cases where the Copy impl has inference constraints that are then used in order to perform method lookup (or any other kind of hir typeck check that relies on matching on a type).

want to crater this to see how breaking it is but it would be very easy to just not break this and think that is my preference :3

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my preference now is actually to either always defer and eat the breaking change or do a less-hacky attempt to avoid the breaking change by introducing a new goal kind for repeat expr checks that can be deferred exactly as long as is necessary and no furthur

@bors

☀️ Try build successful - checks-actions
Build commit: 9053a8a (9053a8ad740a9e131b53ea98acecf68cf79134fe)

@BoxyUwU

@craterbot

@craterbot

🚧 Experiment pr-137045 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

@BoxyUwU

@BoxyUwU BoxyUwU marked this pull request as ready for review

February 18, 2025 13:01

@BoxyUwU

BoxyUwU

Comment on lines +1537 to +1538

// FIXME: Infer `?ct = {const error}`?
ty::Const::new_error(self.tcx, e)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no fcx.demand_subtype for consts yet.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just self.at(..).eq(ct, err)?

// first as errors from not having inferred array lengths yet seem less confusing than errors from inference
// fallback arbitrarily inferring something incompatible with `Copy` inference side effects.
//
// This should also be forwards compatible with moving repeat expr checks to a custom goal kind or using

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel somewhat hesitant to add a custom goal kind for repeat expr checks but I do think it would be "optimal" behaviour-wise as the goals could stall on the repeat count infer var and be deferred ~as long as necessary while also not delaying inference constraints from them any later than is needed.

compiler-errors

Comment on lines +1537 to +1538

// FIXME: Infer `?ct = {const error}`?
ty::Const::new_error(self.tcx, e)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just self.at(..).eq(ct, err)?

@compiler-errors

lgtm, r=me with or without applying the nit; i think it may not be needed b/c all it's gonna do is slightly improve duplicated/unnecessary errors.

@BoxyUwU

(rebased)

@bors r=compiler-errors

@bors

📌 Commit 6c3243f has been approved by compiler-errors

It is now in the queue for this repository.

@bors

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Feb 27, 2025

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Feb 28, 2025

@matthiaskrgr

…r=compiler-errors

Defer repeat expr Copy checks to end of type checking

Fixes rust-lang#110443

Defers repeat expr checks that the element type is Copy when the length is > 1 (or generic) to end of typeck so that under generic_arg_infer repeat exprs are able to have an inferred count, e.g. let a: [_; 1] = [String::new(); _];.

Currently the deferring is gated under generic_arg_infer though I intend to separately types FCP deferring the checks even outside of generic_arg_infer if we wind up not going with an alternative.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 28, 2025

@bors

…iaskrgr

Rollup of 11 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 28, 2025

@bors

…iaskrgr

Rollup of 11 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 28, 2025

@bors

…iaskrgr

Rollup of 11 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Feb 28, 2025

@matthiaskrgr

…r=compiler-errors

Defer repeat expr Copy checks to end of type checking

Fixes rust-lang#110443

Defers repeat expr checks that the element type is Copy when the length is > 1 (or generic) to end of typeck so that under generic_arg_infer repeat exprs are able to have an inferred count, e.g. let a: [_; 1] = [String::new(); _];.

Currently the deferring is gated under generic_arg_infer though I intend to separately types FCP deferring the checks even outside of generic_arg_infer if we wind up not going with an alternative.

This was referenced

Feb 28, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 1, 2025

@bors

…iaskrgr

Rollup of 9 pull requests

Successful merges:

Failed merges:

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 1, 2025

@rust-timer

Rollup merge of rust-lang#137045 - BoxyUwU:defer_repeat_expr_checks, r=compiler-errors

Defer repeat expr Copy checks to end of type checking

Fixes rust-lang#110443

Defers repeat expr checks that the element type is Copy when the length is > 1 (or generic) to end of typeck so that under generic_arg_infer repeat exprs are able to have an inferred count, e.g. let a: [_; 1] = [String::new(); _];.

Currently the deferring is gated under generic_arg_infer though I intend to separately types FCP deferring the checks even outside of generic_arg_infer if we wind up not going with an alternative.

Labels

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.