Nullability in value_kind

[goldfirere]

There should be more tests, but we're moving or_null back to beta and will write those tests as the move out of beta.

This includes promotion of two test changes. I'm not sure what's going on here, so we need to make sure this gets reviewed by someone who understands these changes, perhaps @lthls.

[lthls]

The changes in the testsuite are expected: a few blocks went from having shapes looking like Non_nullable Pgenval to Nullable Pgenval, and there is a piece of code in Printlambda.block_shape that avoids printing shapes where all fields are generic (nullable) values which now triggers.

The change in Typeopt looks correct (assuming that the mechanism to get the nullability is correct; I don't know enough about that part to review it).
However, I think it would make sense to split value_kind into value_kind_non_null, which would keep the current code but return values kinds without nullability (of type Lambda.value_kind_non_null), and value_kind which would be a small wrapper that computes nullability from the type and adds it to the raw kind returned by value_kind_non_null. It depends on having a reliable way to get nullability from an arbitrary type expression (and environment), but would simplify the code and in particular remove the concern about using type declarations.

[dkalinichenko-js]

The mechanism to get the nullability looks good, and changes in typeopt also look like what I'd expect. I agree about splitting the function, although maybe it should have a less ambiguous name (_no_nullability_info?)

[goldfirere]

I don't think I agree about splitting the function. In many cases, we know the nullability within value_kind. (There are only three calls to add_nullability_from_jkind, in fact.) If we split the function, we will effectively forget this, and will have to recompute it, which can be expensive.

[goldfirere]

I'm glad that you both agree that the new implementation is correct. Given that we're trying to roll out the fix today, could one of you approve? If need be, we can address the refactoring later.

[dkalinichenko-js]

Done. You might want to rebase to pick up the change moving or_null to beta.

[lthls]

I don't think I agree about splitting the function. In many cases, we know the nullability within value_kind. (There are only three calls to add_nullability_from_jkind, in fact.) If we split the function, we will effectively forget this, and will have to recompute it, which can be expensive.

I'm fine with your argument, but I'm not very confident that the current version, where nullability is hardcoded in each case, is future-proof enough. So I would welcome a way to at least check that the nullability we compute is consistent with what we would get by looking at the type's jkind independently (that could be turned on if/when weird bugs involving or_null start appearing again).