Allow arena.Arena to compress arbitrary Go pointers, if it owns them

[mcy]

This PR adds Arena[T].Compress, which will take *T and compress it into an arena.Pointer[T] if the arena owns that pointer.

This in turn allows us to shrink the size of every AST node by a whole machine word, by dropping redundant arena pointers in typed exported nodes like ast.Syntax. We also don't bother to store the kind of the node either, since it can be recomputed from its type.

This change also renames Arena.New to Arena.NewCompressed, and now Arena.New returns a *T.

[jhump]

This looks great, but I'm still a little worried about the code that actually does the pointer comparisons.

[jhump]

The Go docs say to only use runtime.KeepAlive to prevent a finalizer from running. So this feels like it could be brittle and thus dangerous.

Also, this construction -- passing an array of pointers instead of a pointer -- is very counter-intuitive. I guess this relies on the fact that pointers in the heap can never point to address on the stack? It feels safe to do this earlier with the original pointers instead of with the unsafe.Pointer aliases, something like so:

runtime.KeepAlive(p)
runtime.KeepAlive(&s[0])

[mcy]

Oh the reason for doing it this way is to only have to pay for one function call. KeepAlive cannot be inlined. Sorry, I am very used to working around Go not having tuples by using arrays. XD

Also, &s[0] is incorrect, because it will trigger a redundant bounds check. You still want unsafe.SliceHeader.

Separately, reproduced from Slack:

it's not the right way to do it. there are a few alternatives. one is to open-code KeepAlive, because the current implementation will escape the pointers. we could instead mark the function as go:nosplit. this is used within the runtime in many places as "do not move the pointers I just received as arguments", sufficiently that people have copied the pattern and changing it would almost certainly break the world. note that even if we don't annotate it, the current version of Go won't fuck us: stack growth only happens on function entry, and that is the only situation under which go will move pointers
also, i struggle to think of a way they could move the pointers out from under us after casting to uintptr, and somehow discriminating from the case where we have an unsafe.Pointer. and also, this requires the move to happen after we've cast one of the pointers but not the others, and the result is that we report that the arena does not contain a pointer it actually contains
like with my native compiler person hat on i think the risk is nonexistent
i'm happy to write an essay about this in the comments

[jhump]

Yeah, I'm trying to think if we even need anything. Maybe //go:nosplit out of an abundance of caution and omit the runtime.KeepAlive? The only danger is when a pointer is in a uintptr var, in which case it won't be updated if it points to a stack address that gets moved. So the only risk is if stack motion occurred in the middle of the calculation on line 31.

[mcy]

Sure let's do that.

[jhump]

Seems like we might want to also check that diff % size == 0?

[jhump]

Is there any risk that size might not include trailing padding bytes? In other words, is it possible that diff/size could return the wrong index and it should instead be something like diff/(size+padding)?

[mcy]

Sizeof is the stride size, so it has to contain padding. https://go.dev/play/p/vYH_8QPvMTW

Consider T = [2]byte. If p - s[0] lies outside of the range of s, we exit early. Otherwise we perform this division, so p is between s[0] and s[len(s)-1], inclusive (it is not one past, we test for that). For diff to be divisible by size, p must point to some s[n], and not, say, to &s[n] + 1. It cannot point to &s[n] + 1 because that would be a *[2]byte that does not point to an allocated [2]byte value (instead, it stradles two of them), which is not something Go permits. Thus, we don't need the % check.

[jhump]

Yeah, I'm trying to think if we even need anything. Maybe //go:nosplit out of an abundance of caution and omit the runtime.KeepAlive? The only danger is when a pointer is in a uintptr var, in which case it won't be updated if it points to a stack address that gets moved. So the only risk is if stack motion occurred in the middle of the calculation on line 31.