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

Crash with move || closure and self #20228

Closed
simias opened this issue Dec 25, 2014 · 2 comments · Fixed by #20244
Closed

Crash with move || closure and self #20228

simias opened this issue Dec 25, 2014 · 2 comments · Fixed by #20244
Labels
A-closures Area: Closures (`|…| { … }`) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@simias
Copy link

simias commented Dec 25, 2014

The following code:

struct S;

impl S {
    fn foo(&self) {
        let _ = move || { self };
    }
}

fn main() {
}

Triggers an assertion in LLVM:

crash.rs:1:1: 1:10 warning: struct is never used: `S`, #[warn(dead_code)] on by default
crash.rs:1 struct S;
           ^~~~~~~~~
crash.rs:4:5: 6:6 warning: method is never used: `foo`, #[warn(dead_code)] on by default
crash.rs:4     fn foo(&self) {
crash.rs:5         let _ = move || { self };
crash.rs:6     }
rustc: /home/rustbuild/src/rust-buildbot/slave/nightly-linux/build/src/llvm/lib/IR/Instructions.cpp:1086: void llvm::StoreInst::AssertOK(): Assertion `getOperand(0)->getType() == cast<PointerType>(getOperand(1)->getType())->getElementType() && "Ptr must be a pointer to Val type!"' failed.

I get a slight variation if I change the code in the closure to call a member function instead:

struct S;

impl S {
    fn foo(&self) {
        let _ = move || { self.foo() };
    }
}

fn main() {
}

Gives:

crash.rs:1:1: 1:10 warning: struct is never used: `S`, #[warn(dead_code)] on by default
crash.rs:1 struct S;
           ^~~~~~~~~
crash.rs:4:5: 6:6 warning: method is never used: `foo`, #[warn(dead_code)] on by default
crash.rs:4     fn foo(&self) {
crash.rs:5         let _ = move || { self.foo() };
crash.rs:6     }
rustc: /home/rustbuild/src/rust-buildbot/slave/nightly-linux/build/src/llvm/lib/IR/Instructions.cpp:2522: static llvm::CastInst* llvm::CastInst::CreatePointerCast(llvm::Value*, llvm::Type*, const llvm::Twine&, llvm::Instruction*): Assertion `S->getType()->isPtrOrPtrVectorTy() && "Invalid cast"' failed.
@huonw huonw added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-closures Area: Closures (`|…| { … }`) labels Dec 25, 2014
@japaric
Copy link
Member

japaric commented Dec 26, 2014

I think this is a duplicate of #19141 and #20193. See this #20193 (comment) for rationale.

As a workaround use move |:| instead of move ||.

japaric pushed a commit to japaric/rust that referenced this issue Dec 26, 2014
bors added a commit that referenced this issue Dec 27, 2014
closes #19141
closes #20193
closes #20228

---

Currently whenever we encounter `let f = || {/* */}`, we *always* type check the RHS as a *boxed* closure. This is wrong when the RHS is `move || {/* */}` (because boxed closures can't capture by value) and generates all sort of badness during trans (see issues above). What we *should* do is always type check `move || {/* */}` as an *unboxed* closure, but ~~I *think* (haven't tried)~~ (2) this is not feasible right now because we have a limited form of kind (`Fn` vs `FnMut` vs `FnOnce`) inference that only works when there is an expected type (1).

In this PR, I've chosen to generate a type error whenever `let f = move || {/* */}` is encountered. The error asks the user to annotate the kind of the unboxed closure (e.g. `move |:| {/* */}`). Once annotated, the compiler will type check the RHS as an unboxed closure which is what the user wants.

r? @nikomatsakis 

(1) AIUI it only triggers in this scenario:

``` rust
fn is_uc<F>(_: F) where F: FnOnce() {}

fn main() {
    is_uc(|| {});  // type checked as unboxed closure with kind `FnOnce`
}
```

(2) I checked, and it's not possible because `check_unboxed_closure` expects a `kind` argument, but we can't supply that argument in this case (i.e. `let f = || {}`, what's the kind?). We could force the `FnOnce` kind in that case, but that's ad hoc. We should try to infer the kind depending on how the closure is used afterwards, but there is no inference mechanism to do that (at least, not right now).
@simias
Copy link
Author

simias commented Dec 27, 2014

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants