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

LLVM assertion failed in code with closures #20193

Closed
netvl opened this issue Dec 24, 2014 · 1 comment · Fixed by #20244
Closed

LLVM assertion failed in code with closures #20193

netvl opened this issue Dec 24, 2014 · 1 comment · Fixed by #20244

Comments

@netvl
Copy link
Contributor

netvl commented Dec 24, 2014

This code (as reported in this SO question):

fn foo(t: &mut int){
    println!("{}", t);
}

fn main() {
    let test = 10;

    let h = move || {
        let mut r = &mut test.clone();
        foo(r);
    };

    h();
}

causes an assertion failure in LLVM code:

Assertion failed: (S->getType()->isPtrOrPtrVectorTy() && "Invalid cast"), function CreatePointerCast, file /Users/rustbuild/src/rust-buildbot/slave/nightly-mac/build/src/llvm/lib/IR/Instructions.cpp, line 2522.
@netvl netvl changed the title LLVM assertion failed in closures code LLVM assertion failed in code with closures Dec 24, 2014
@japaric
Copy link
Member

japaric commented Dec 24, 2014

I think this is a duplicate of #19141. Right now, when you assign a closure to a variable you always end with a boxed closure, and using move with a boxed closure shouldn't be allowed (the bug is that the compiler lets you do that) because that kind of closure doesn't capture by value. Once you've created this invalid closure, depending on how you created it or what you do with it, you'll get different LLVM assertions.

The workaround here is to annotate the closure (like move |:|) to make the compiler do the Right Thing and give you back an "unboxed" closure.

cc @nikomatsakis

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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants