Skip to content
This repository

Irrefutable bindings matching code is broken #3235

Closed
bblum opened this Issue August 20, 2012 · 11 comments

6 participants

Ben Blum Tim Chevalier Niko Matsakis James Miller Alex Crichton Graydon Hoare
Ben Blum
Collaborator
fn main() {
    let x = "hello";
    let ref y = x;
    // let y = match x { ref y => y };  // this way works fine
    error!("%?", *y);
}

produces:

rustc: /home/bblum/rust/src/llvm/lib/VMCore/Instructions.cpp:280:
void llvm::CallInst::init(llvm::Value*, llvm::ArrayRef<llvm::Value*>,
const llvm::Twine&): Assertion `(i >= FTy->getNumParams() ||
FTy->getParamType(i) == Args[i]->getType()) && "Calling a function
with a bad signature!"' failed.
Aborted (core dumped)

Happens with structs, tuples, etc too.

Niko Matsakis
Collaborator

for some reason that is fairly unclear to me, trans/alt has a completely distinct code path for irrefutable patterns. that path should be deleted and they should all use the normal alt path, imo. anyway, that particular path is hard-coded to copy and doesn't know about ref bindings.

Niko Matsakis
Collaborator

I changed the title to my preferred solution.

Tim Chevalier
Collaborator

Ugh, this has been a pain in the you-know-what. Not going to be ready for 0.5

James Miller
Collaborator
Aatch commented April 02, 2013

Also causes this strange behaviour:

fn main() -> () {
    let a = ~(1, 2);
    test(a);
}

fn test<T,U>(a : &(T, U)) {
    let (ref _x, ref _y) = *a;
}

Returns error: moving out of dereference of immutable & pointer

However:

fn main() -> () {
    let a = ~(1, 2);
    test(a);
}

fn test<T,U>(a : &(T, U)) {
    let (_a, _b) = match *a {
        (ref a, ref b) => (a, b)
    };
}

Is fine.

Alex Crichton
Collaborator

Nominating for the well-defined milestone.

This makes working with non-copyable tuples laborious, and actually has serious performance implications for destructuring assignment (at least I think that's related to this bug, copies are generated when they shouldn't be). Failing the well-defined milestone, I think this would fit well into the feature-complete milestone.

Tim Chevalier
Collaborator

I don't feel like this fits under anything except production-ready, but I agree with nominating it.

Niko Matsakis
Collaborator

For what it's worth, I don't think that this is necessarily the right fix anymore. But the bug is real.

Niko Matsakis
Collaborator

Update the title to reflect the general bug.

Niko Matsakis
Collaborator

Very close to fixing this. I've got a branch that's been slowly progress towards stability.

Graydon Hoare
Owner

accepted for backwards-compatible milestone

Tim Chevalier catamorphism closed this July 08, 2013
Tim Chevalier
Collaborator

a48ca32 fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.