Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer repeat expr Copy checks to end of type checking #137045

Merged
merged 3 commits into from
Mar 1, 2025

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Feb 14, 2025

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
Copy link
Member Author

BoxyUwU commented Feb 14, 2025

@bors try

@bors
Copy link
Contributor

bors commented Feb 14, 2025

⌛ Trying commit c09f183 with merge 9053a8a...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
…<try>

Defer repeat expr `Copy` checks to end of type checking

Fixes rust-lang#110443

r? `@ghost`

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

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

Copy link
Member Author

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
Copy link
Contributor

bors commented Feb 14, 2025

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

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Feb 14, 2025

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-137045 created and queued.
🤖 Automatically detected try build 9053a8a
🔍 You can check out the queue and this experiment's details.

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

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2025
@BoxyUwU BoxyUwU force-pushed the defer_repeat_expr_checks branch from c09f183 to 8966d60 Compare February 15, 2025 02:06
@craterbot
Copy link
Collaborator

🚧 Experiment pr-137045 is now running

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

@craterbot
Copy link
Collaborator

🎉 Experiment pr-137045 is completed!
📊 5 regressed and 3 fixed (583035 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Feb 16, 2025
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Feb 16, 2025

(all spurrious)

@BoxyUwU BoxyUwU force-pushed the defer_repeat_expr_checks branch 3 times, most recently from 75bb3ab to 1ff88a7 Compare February 18, 2025 12:58
@BoxyUwU BoxyUwU marked this pull request as ready for review February 18, 2025 13:01
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Feb 18, 2025

r? @compiler-errors

Comment on lines +1537 to +1538
// FIXME: Infer `?ct = {const error}`?
ty::Const::new_error(self.tcx, e)
Copy link
Member Author

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.

Copy link
Member

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
Copy link
Member Author

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.

Comment on lines +1537 to +1538
// FIXME: Infer `?ct = {const error}`?
ty::Const::new_error(self.tcx, e)
Copy link
Member

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
Copy link
Member

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 BoxyUwU force-pushed the defer_repeat_expr_checks branch from 1ff88a7 to 6c3243f Compare February 27, 2025 22:33
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Feb 27, 2025

(rebased)

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Feb 27, 2025

📌 Commit 6c3243f has been approved by compiler-errors

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 27, 2025

🌲 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
…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
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#137045 (Defer repeat expr `Copy` checks to end of type checking)
 - rust-lang#137171 (Suggest swapping equality on E0277)
 - rust-lang#137634 (Update `compiler-builtins` to 0.1.149)
 - rust-lang#137686 (Handle asm const similar to inline const)
 - rust-lang#137689 (Use `Binder<Vec<Ty>>` instead of `Vec<Binder<Ty>>` in both solvers for sized/auto traits/etc.)
 - rust-lang#137718 (Use original command for showing sccache stats)
 - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`)
 - rust-lang#137730 (checked_ilog tests: deal with a bit of float imprecision)
 - rust-lang#137735 (Update E0133 docs for 2024 edition)
 - rust-lang#137742 (unconditionally lower match arm even if it's unneeded for never pattern in match)
 - rust-lang#137771 (Tweak incorrect ABI suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#137045 (Defer repeat expr `Copy` checks to end of type checking)
 - rust-lang#137171 (Suggest swapping equality on E0277)
 - rust-lang#137634 (Update `compiler-builtins` to 0.1.149)
 - rust-lang#137686 (Handle asm const similar to inline const)
 - rust-lang#137689 (Use `Binder<Vec<Ty>>` instead of `Vec<Binder<Ty>>` in both solvers for sized/auto traits/etc.)
 - rust-lang#137718 (Use original command for showing sccache stats)
 - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`)
 - rust-lang#137730 (checked_ilog tests: deal with a bit of float imprecision)
 - rust-lang#137735 (Update E0133 docs for 2024 edition)
 - rust-lang#137742 (unconditionally lower match arm even if it's unneeded for never pattern in match)
 - rust-lang#137771 (Tweak incorrect ABI suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#137045 (Defer repeat expr `Copy` checks to end of type checking)
 - rust-lang#137171 (Suggest swapping equality on E0277)
 - rust-lang#137634 (Update `compiler-builtins` to 0.1.149)
 - rust-lang#137686 (Handle asm const similar to inline const)
 - rust-lang#137689 (Use `Binder<Vec<Ty>>` instead of `Vec<Binder<Ty>>` in both solvers for sized/auto traits/etc.)
 - rust-lang#137718 (Use original command for showing sccache stats)
 - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`)
 - rust-lang#137730 (checked_ilog tests: deal with a bit of float imprecision)
 - rust-lang#137735 (Update E0133 docs for 2024 edition)
 - rust-lang#137742 (unconditionally lower match arm even if it's unneeded for never pattern in match)
 - rust-lang#137771 (Tweak incorrect ABI suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 28, 2025
…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 Mar 1, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#137045 (Defer repeat expr `Copy` checks to end of type checking)
 - rust-lang#137171 (Suggest swapping equality on E0277)
 - rust-lang#137686 (Handle asm const similar to inline const)
 - rust-lang#137689 (Use `Binder<Vec<Ty>>` instead of `Vec<Binder<Ty>>` in both solvers for sized/auto traits/etc.)
 - rust-lang#137718 (Use original command for showing sccache stats)
 - rust-lang#137730 (checked_ilog tests: deal with a bit of float imprecision)
 - rust-lang#137735 (Update E0133 docs for 2024 edition)
 - rust-lang#137742 (unconditionally lower match arm even if it's unneeded for never pattern in match)
 - rust-lang#137771 (Tweak incorrect ABI suggestion and make suggestion verbose)

Failed merges:

 - rust-lang#137723 (Make `rust.description` more general-purpose and pass `CFG_VER_DESCRIPTION`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 91eb721 into rust-lang:master Mar 1, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 1, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inferred repeat expression length unnecessarily needs Copy
5 participants