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

revert 74989 ("Implement Index and IndexMut for arrays") #79416

Closed
wants to merge 2 commits into from

Conversation

matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Nov 25, 2020

This PR reverts #74989 and adds a testcase for

const fn foo() {
    let mut array = [[0; 1]; 1];
    array[0][0] = 1;
}

which compiles on stable and beta but not on nightly due to #74989 .
The offending commit had a crater run but the breaking change was missed which means it probably would not have landed if the breaking change had been noticed in time.

This PR fixes compilation of real-world crates such as https://github.com/alacritty/vte/

Fixes #79152

This reverts commit c03dfa6.

The commit broke code that worked on stable and beta:

const fn foo() {
    let mut array = [[0; 1]; 1];
    array[0][0] = 1;
}

But this was not noticed until the commit landed.

Fixes rust-lang#79152
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2020
@Aaron1011
Copy link
Member

This will break any code that started relying on the (insta-stable) Index and IndexMut impls.
I think the targeted fix suggested by @lcnr would be a better idea: #79152 (comment)

@Aaron1011
Copy link
Member

I've opened #79427 - the issue turned out to be a missing call to resolve_vars_if_possible

@oli-obk
Copy link
Contributor

oli-obk commented Nov 26, 2020

we decided in last weeks compiler team meeting that we'd do the revert and then immediately open a PR with the fix and impl again. I guess that only makes sense if we crater it, considering that there's a fix PR open. Since we didn't discuss running crater, and we do have a fix PR, let's favour the fix PR.

cc @rust-lang/compiler I'm making a decision against the meeting decision, since I think there's little benefit in reverting, only to readd with the fix and merge that immediately after.

@oli-obk oli-obk closed this Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alacritty/vte fails to build on rustc master (mutable references are not allowed in constant functions)
5 participants