fix(compiler): Apply correct allocation type to numbers

[ospencer]

We can potentially be a little smarter here and only do this for numbers that get allocated on the heap; it'd save a few incref/decrefs.

[marcusroberts]

LGTM!

[peblair]

@ospencer Can you either add at least some basic check for whether a number will be stack-allocated (and set the allocation_type accordingly) or at least create a follow-on issue so we don't lose track of this?

[ospencer]

I was thinking about this and realized that might be too clever... I think it'd cause the slot to be considered StackAllocated for its full life, which for let mut could mean that a heap-allocated number might not get decref'd.

So maybe we should just forget I said that 😛

[peblair]

Got it. This makes me think (not for this PR) that the allocation_type logic should maybe be three-valued, though:

• stack-allocated
• heap-allocated
• unknown (assume heap-allocated)

[ospencer]

What would unknown get us?

[ospencer]

Though I can certainly see some terminology changes, like calling them Managed and Unmanaged or something like that.

[peblair]

Well, my interpretation of your last comment is, "we have to (conservatively) say that it's heap-allocated, since we don't know whether or not it is"; I feel like we should explicitly encode the fact that we don't know whether it is, and then the codegen can explicitly say, "if we don't know what it is, then we'll assume it's heap-allocated" (as opposed to the implicit statement of that idea in the current code)

[ospencer]

Yeah. I think that's why I could get on board with something like Managed/Unmanaged since we care more about whether it's getting GC'd than if that value is a pointer or not (since largely we've been saying "that thing is a pointer, so GC it") which gets in the way of our hybrid-allocated types like numbers.

[peblair]

Sounds good to me. Can you open an issue? Also, maybe we should have a separate ~memory_management= field? Whatever you think is best.

[ospencer]

Yeah, splitting the managed apart from the wasm type?

[peblair]

Yeah, exactly. Adding a separate field for the memory management and updating the codegen to use it for its incref/decref decision-making

[phated]

:approved: