ref in function parameter patterns

[ben0x539]

The following program doesn't print "drop". Looks unsound to me.

struct S; 
impl Drop for S {
    fn drop(&self) {
        println("drop"); 
    } 
} 

fn f(ref _s: S) {}

fn main() { 
    let s = S;
    f(s);
}

In #5239, @nikomatsakis suggested that ref shouldn't be allowed in argument patterns at all, but then that bug got closed because the ICE/llvm asserts over there stopped occuring.

[nikomatsakis]

Huh. I remember putting in some effort to make sure this kind of test worked. I am not sure why I said ref should not appear in argument patterns, I don't think that anymore.

[nikomatsakis]

Yes, there is a test src/test/run-pass/func-arg-ref-pattern.rs that is supposed to test this case. I wonder if there is a problem with the destructor on the unit struct instead?

[ben0x539]

Also occurs with a non-unit struct:

struct S { i: int }
impl Drop for S {
    fn drop(&self) {
        println("drop");
    }
}

fn f(ref _s: S) {}

fn main() {
    let s = S { i: 42 };
    f(s);
}

(and either version will print "drop" if I remove the ref).

I assumed the problem with my code is that main() feels absolved of its ownership of the S because it's clearly moving it into f as that takes a parameter by value, and f doesn't feel responsible because it's only matching a ref pattern on the argument...? This seems vaguely different than what the func-arg-ref-pattern.rs test tests for, where ref isn't at the top level of the argument pattern.

[nikomatsakis]

Presumably that is the problem. main() is correct, f() is not. f() should free its arguments upon return. In any case, clearly something IS diffierent, since one test works and the other not, though I would have thought they would be the same. Needs more investigation.

[alexcrichton]

This is not good. Nominating.

[pnkfelix]

Accepted for "first major release, P-high"

[edwardw]

Both cases work for me as expected now.

$ rustc --version
rustc 0.10-pre
host: x86_64-apple-darwin

[alexcrichton]

Hurray! flagging as needstest.

[edwardw]

The issue can be closed now.

[sfackler]

FYI if you put "closes #8860" in the commit message, Github will automatically close the issue when the commit merges.

[edwardw]

I did but the PR was integrated as part of a rollup. So it didn't get closed properly.

[alexcrichton]

We recommend putting the "Closes #XXX" in the commit message rather than just the PR for the reason that it'll still get closed during a rollup.

[edwardw]

Ah, I misunderstood @sfackler's comment. The "Closes #xxxx" in both commit and PR message, now I see.