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

Assignment unwrapping fails when return args are involved #890

Closed
fasterthanlime opened this Issue Jul 9, 2015 · 1 comment

Comments

Projects
None yet
1 participant
@fasterthanlime
Collaborator

fasterthanlime commented Jul 9, 2015

This will crash at runtime:

use sam-assert

Ditto: class {
    s: String
    init: func (=s)

    operator + (other: This) -> This {
        new(s + other s)
    }
}

Container: class {
    ditto := Ditto new("john")
    init: func
}

Peeker: class {
    inner: Object
    init: func (=inner)
    peek: func <T> (T: Class) -> T { inner }
}

describe("should unwrap += correctly even with returnArgs", ||
    c := Container new()
    p := Peeker new(c)

    // forced to unwrap, only has '+' operator
    p peek(Container) ditto += Ditto new(" doe")

    expect("john doe", c ditto s)
)

Because:

p peek(Container) ditto += Ditto new(" doe")

Is turned into:

p peek(Container) ditto = p peek(Container) ditto + Ditto new(" doe")

But really, what happens is that

p peek(Container) ditto += Ditto new(" doe")

Is turned into: (mix of C & ooc syntax.. (a, b, c) is a comma sequence - executes all statements, evaluates to last element)

tmp: Container
(p peek(Container, tmp&), tmp) ditto += Ditto new(" doe")

And then unwrapped, since Ditto only has a + operator, but somehow it's the commaSequence that's being cloned, and FunctionCall doesn't clone its returnArgs, so it ends up looking like:

tmp: Container
(p peek(Container, tmp&), tmp) ditto = (p peek(Container, NULL), tmp) ditto + Ditto new(" doe")

And obviously that crashes, not when calling the rhs peek (it'll happily write nothing if returnArg is NULL) but when trying to access "ditto" from it (since, you know, it's null).

@fasterthanlime fasterthanlime changed the title from Wtf to Assignment unwrapping fails when return args are involved Jul 9, 2015

@fasterthanlime

This comment has been minimized.

Collaborator

fasterthanlime commented Jul 9, 2015

So, since we can't really tell the FunctionCall in advance that it's going to be cloned at some point so we can unwrap an assignment... the "fix" is to let CommaSequence know where it comes from, so that when a CommaSequence is cloned, if it comes from an unwrapped FunctionCall, it returns a clone of that instead, leaving it a chance to unwrap again, in the new context.

Thus:

p peek(Container) ditto += Ditto new(" doe")

Is turned into (mix of C & ooc)

tmp1, tmp2: Container
(p peek(Container, tmp1&), tmp1) ditto = (p peek(Container, tmp2&), tmp2) + Ditto new(" doe")

And even though that's really wasteful, at least it's correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment