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

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

Closed
matthiaskrgr opened this issue Nov 17, 2020 · 10 comments · Fixed by #79427
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

Upstream ticket: alacritty/vte#68

Code

https://github.com/alacritty/vte/
I tried this code:

<code>

I expected to see this happen: explanation
The code compiles on

   stable-x86_64-unknown-linux-gnu unchanged - rustc 1.47.0 (18bf6b4f0 2020-10-07)
     beta-x86_64-unknown-linux-gnu unchanged - rustc 1.48.0-beta.10 (8236dc07b 2020-11-15)
  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.50.0-nightly (f5230fbf7 2020-11-16)

but fails to build on rustc master rustc 1.50.0-nightly (c919f490b 2020-11-17)
which I assumes makes this a regression:

    Checking vte v0.9.0 (/tmp/vte)
error[E0658]: mutable references are not allowed in constant functions
   --> src/table.rs:9:1
    |
9   | / generate_state_changes!(state_changes, {
10  | |     Anywhere {
11  | |         0x18 => (Ground, Execute),
12  | |         0x1a => (Ground, Execute),
...   |
170 | |     }
171 | | });
    | |___^
    |
    = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
    = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0015]: calls in constant functions are limited to constant functions, tuple structs and tuple variants
   --> src/table.rs:9:1
    |
9   | / generate_state_changes!(state_changes, {
10  | |     Anywhere {
11  | |         0x18 => (Ground, Execute),
12  | |         0x1a => (Ground, Execute),
...   |
170 | |     }
171 | | });
    | |___^
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

Version it worked on

It most recently worked on:

Version with regression

rustc --version --verbose:

<version>

Backtrace

Backtrace

<backtrace>

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 18, 2020
@jonas-schievink jonas-schievink added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Nov 18, 2020
@matthiaskrgr
Copy link
Member Author

cargo-bisect-rustc says the regression is caused by c6a6105

@matthiaskrgr
Copy link
Member Author

Maybe #79032 ?

@jyn514
Copy link
Member

jyn514 commented Nov 18, 2020

cc @lcnr

@matthiaskrgr matthiaskrgr changed the title alacritty/vte fails to build on rustc master alacritty/vte fails to build on rustc master (mutable references are not allowed in constant functions) Nov 18, 2020
@matthiaskrgr
Copy link
Member Author

@SNCPlay42
Copy link
Contributor

MCVE:

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

@SNCPlay42
Copy link
Contributor

Could be #74989.

@SNCPlay42
Copy link
Contributor

SNCPlay42 commented Nov 18, 2020

Reverting #74989 fixes the regression.

The regression also appears in the crater report for #74989, but it appears either it was overlooked or the decision was made to merge anyway? cc @KodrAus

@spastorino spastorino added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 18, 2020
@spastorino
Copy link
Member

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@lcnr
Copy link
Contributor

lcnr commented Nov 18, 2020

I don't think we should use the Index impl here but instead keep using builtin indexing

we probably have to update something here:

if let Some(ty::Ref(_, base_ty, _)) = base_ty {
let index_ty = typeck_results.expr_ty_adjusted_opt(&index).unwrap_or_else(|| {
// When encountering `return [0][0]` outside of a `fn` body we would attempt
// to access an unexistend index. We assume that more relevant errors will
// already have been emitted, so we only gate on this with an ICE if no
// error has been emitted. (#64638)
self.fcx.tcx.ty_error_with_message(
e.span,
&format!("bad index {:?} for base: `{:?}`", index, base),
)
});
let index_ty = self.fcx.resolve_vars_if_possible(index_ty);
if base_ty.builtin_index().is_some() && index_ty == self.fcx.tcx.types.usize {
// Remove the method call record
typeck_results.type_dependent_defs_mut().remove(e.hir_id);
typeck_results.node_substs_mut().remove(e.hir_id);
if let Some(a) = typeck_results.adjustments_mut().get_mut(base.hir_id) {
// Discard the need for a mutable borrow
// Extra adjustment made when indexing causes a drop
// of size information - we need to get rid of it
// Since this is "after" the other adjustment to be
// discarded, we do an extra `pop()`
if let Some(Adjustment {
kind: Adjust::Pointer(PointerCast::Unsize), ..
}) = a.pop()
{
// So the borrow discard actually happens here
a.pop();
}
}

@matthiaskrgr
Copy link
Member Author

@rustbot label E-needs-bisection E-needs-mcve

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 25, 2020
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 25, 2020
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
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 26, 2020
…i-obk

Resolve inference variables before trying to remove overloaded indexing

Fixes rust-lang#79152

This code was already set up to handle indexing an array. However, it
appears that we never end up with an inference variable for the slice
case, so the missing call to `resolve_vars_if_possible` had no effect
until now.
@bors bors closed this as completed in 0b64110 Nov 26, 2020
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-bug Category: This is a bug. E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants