ocaml-flambda / flambda-backend

The Flambda backend project for OCaml
104 stars 76 forks source link

Implement the nullability axis for jkinds #2731

Closed dkalinichenko-js closed 3 months ago

dkalinichenko-js commented 3 months ago

Implement the Nullability axis for jkinds, with Non_null < Or_null.

Add legacy layouts any_non_null and value_or_null. Create stubs for corresponding creation reasons.

Default jkinds created from sort variables to Non_null. In a future PR, most of those will be updated to accept Or_null.

Test those new jkinds.

dkalinichenko-js commented 3 months ago

Thought: here, I split off concrete_non_null_creation_reason from concrete_creation_reason. However, this naming doesn't really explain why we need that distinction.

I think we should rename those to something like concrete_default_creation_reason and concrete_permissive_creation_reason respectively. Before this patch, we always wanted the topmost jkind on the lattice everywhere. However, now we need to default legacy code to Non_null, but still accept Or_null in as much places as we can.

This will also help us implement the variable syntax: we would just need to change the default in concrete_default_creation_reason per file.

goldfirere commented 3 months ago

Thought: here, I split off concrete_non_null_creation_reason from concrete_creation_reason. However, this naming doesn't really explain why we need that distinction.

I think we should rename those to something like concrete_default_creation_reason and concrete_permissive_creation_reason respectively. Before this patch, we always wanted the topmost jkind on the lattice everywhere. However, now we need to default legacy code to Non_null, but still accept Or_null in as much places as we can.

This will also help us implement the variable syntax: we would just need to change the default in concrete_default_creation_reason per file.

Something seems a little off here, in that I think that the choice between or_null and non_null happens independently from the choice to be concrete or not. I don't much care about the names of those types, but I don't think this change will accomplish the goal you're describing, unless I'm missing something.