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

Cannot index and immediately call FnMut #65682

Closed
timotree3 opened this issue Oct 22, 2019 · 6 comments
Closed

Cannot index and immediately call FnMut #65682

timotree3 opened this issue Oct 22, 2019 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@timotree3
Copy link
Contributor

timotree3 commented Oct 22, 2019

Running this code on the latest stable (1.38.0) as well as nightly (2019-10-20)

fn foo<F, I>(i: &mut I)
where
    F: FnMut(),
    I: std::ops::IndexMut<(), Output = F>,
{
    i[()](); // fails
    (&mut i[()])(); // works
}

gives the error

error[E0596]: cannot borrow data in an index of `I` as mutable
 --> src/lib.rs:6:5
  |
6 |     i[()](); // fails
  |     ^^^^^ cannot borrow as mutable
  |
  = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `I`

which is clearly wrong.

Notably, the same error does not occur if we switch everything to Fn:

fn foo<F, I>(i: &I)
where
    F: Fn(),
    I: std::ops::Index<(), Output = F>,
{
    i[()](); // works
}
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 22, 2019
@timotree3
Copy link
Contributor Author

I can still reproduce this on latest nightly (nightly-2019-12-21)

@timotree3
Copy link
Contributor Author

I can still reproduce this on the playground with nightly-2020-05-08 and stable 1.43.1

@estebank
Copy link
Contributor

There seems to be something unaccounted for in mir's check_access_permissions.

CC @oli-obk in case they have context around this already

@estebank
Copy link
Contributor

Looking further I think that the problem is that the place for i[()] is a Deref instead of DerefMut before we get to the MIR borrow checker, which is why it rejects it as not mutable. This makes me think that somewhere in typeck we don't flow the mutable obligation from the i[()]() call back to the i[()]. This also explains why (&mut i[()])() works, because the mutable obligation is explicitly stated and flowed forward, making the call work incidentally.

@estebank
Copy link
Contributor

I'm not sure what the correct thing will be, but I suspect that changing the following so that it modifies the adjustments for callee_expr to be mutable Deref/Borrow will do the trick, but I am not overly familiar with this part of the codebase:

// Now, we look for the implementation of a Fn trait on the object's type.
// We first do it with the explicit instruction to look for an impl of
// `Fn<Tuple>`, with the tuple `Tuple` having an arity corresponding
// to the number of call parameters.
// If that fails (or_else branch), we try again without specifying the
// shape of the tuple (hence the None). This allows to detect an Fn trait
// is implemented, and use this information for diagnostic.
self.try_overloaded_call_traits(call_expr, adjusted_ty, Some(arg_exprs))
.or_else(|| self.try_overloaded_call_traits(call_expr, adjusted_ty, None))
.map(|(autoref, method)| {
let mut adjustments = autoderef.adjust_steps(self, Needs::None);
adjustments.extend(autoref);
self.apply_adjustments(callee_expr, adjustments);
CallStep::Overloaded(method)
})
}

@compiler-errors
Copy link
Member

This code works now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants