Split overloaded uses of RefType in front-end (#8427)

Overview
========

This change is the start of an attempt to address how the Slang compiler
codebase has ended up conflating two similar, but semantically distinct,
concepts:

* The long-standing notion of `ref` parameters (only allowed for use in
the builtin modules), which are encoded using a wrapper `Type` in the
AST as part of the representation of the parameters of a `FuncType`.

* A recently-introduced notion of explicit reference types that mirror
the built-in `Ptr` type, with a relationship comparable to that between
pointer and reference types in C++.

The change splits the `Ref<T>` type in the core module into two distinct
types, with one for each of the two use cases. Similarly, the `RefType`
class in the compiler's AST is split into two distinct classes, to
represent the two cases.

Background
==========

The `Ref<T>` type in the core module (hidden and not intended for users
to ever see or use) was originally introduced to encode the `ref`
parameter-passing mode, comparable to the hidden `Out<T>` and `InOut<T>`
types used to encode `out` and `inout` parameter-passing modes. The
`Ref<T>` type in the core module was encoded as a instance of the
`RefType` class in the Slang AST (similar to how `Out<T>` mapped to an
`OutType`). These AST classes were *only* intended to be used by the
compiler front-end as part of its encoding of function types. The
`FuncType` class needed a way to distinguish an `inout int` parameter
from a plain (implicitly `in`) `int` parameter, so these wrapper like
`RefType` and `OutType` were introduced to encode both the parameter
type (`T`) and the parameter-passing mode in a form that could be passed
around as a `Type`.

Notably, the `Ref<T>` type (and `Out<T>`, etc.) were *not* intended to
be type names that ever get uttered in Slang code (not even in the
builtin modules), and the vast majority of the compiler code was not
supposed to ever encounter them. They were an implementation detail of
`FuncType`, and nothing else.

(In hindsight it may have been a mistake to use a nominal type declared
in the core module to implement these wrappers; it might have been a
good idea to use an entirely separate class of `Type` for this case...)

Recent changes to the builtin modules introduced functions that wanted
to *return* a reference (so that the parameter-passing-mode modifiers
like `ref` could not trivially be used), and as part of those changes
the appealingly-named `Ref<T>` type in the core module was re-used for
this new case. Builtin operations were declared with an explicit
`Ref<T>` return type, and parts of the compiler front-end that had
previously been blissfully unaware of the AST's `RefType` (and
`InOutType`, etc.) had to start accounting for the possibility that an
explicit `Ref<T>` would show up.

Related changes also introduced a comparable conflation of the
(unfortunately-named) `constref` parameter-passing modifier and builtin
operations that wanted to return an explicit reference that is
read-only. Both use cases were mapped to the core-module `ConstRef<T>`
type, which appeared in the AST as an instance of the `ConstRefType`
class.

The overlapping use of `ConstRef<T>`` is actually significantly more
troublesome than the `Ref<T>` case because, despite what its name
implies, `constref` was not really supposed to be the read-only analogue
of `ref`, but rather it is closer to the "immutable value borrow"
analogue to `inout`'s "mutable value borrow." The semantics of a "value
borrow" vs. a "memory reference" in Slang have not been very carefully
codified, and the conflation around `ConstRef<T>` has contributed to
things becoming increasingly muddy in the compiler back-end.

Main Changes
============

Core Module
-----------

The `Ref<T>` type has been replaced with two distinct types, with one
for each use case:

* `RefParam<T>` is intended for use when encoding a `ref` parameter in a
function type

* `ExplicitRef<T>` is intended for use when an operation in a builtin
module wants to return a reference

The other types used to represent parameter-passing modes (e.g.,
`InOut<T>`) were renamed to better indicate that their role in defining
parameter types (e.g., `InOutParam<T>`).

The `ExplicitRef<T>` type was given additional generic parameters for
the allowed access and the address space, akin to what `Ptr<T>` now
supports. The pointer dereference operator (prefix `*`) in the core
module should now properly propagate the access and address space of the
pointer over to the reference that gets returned.

The two distinct use cases of `ConstRef<T>` were not split in the way as
`Ref<T>`, instead the case for the `constref` parameter-passing mode
uses `ConstParamRef<T>`, while cases that previously used `ConstRef<T>`
to represent a read-only explicit reference instead now use
`ExplicitRef<T, Access.Read>`.

Prior to this change there were two subscripts declared on pointers: one
in the `Ptr` type itself, and another in an `extension` for pointers
with `Access.ReadWrite`. The comments on the code seemed to indicate
that the catch-all subscript used to only have a `get` accessor, while
the `ref` was only available on read-write pointers, but it seems that
subsequent changes converted the default subscript to support `ref`.
This change eliminates the subscript added via `extension`, since it is
redundant.

AST and Front-End
=================

Similar to the changes in the core module, the AST `RefType` class was
split into:

* `RefParamType` for the case of encoding `ref` parameters

* `ExplicitRefType` for the case where the user meant an explicit
reference type

All the other classes that represent wrappers for encoding
parameter-passing modes (e.g., `OutType`) were similarly renamed (e.g.,
`OutParamType`).

The `ConstRefType` class was simply renamed to `ConstRefParamType`,
because any use cases of `ConstRefType` that intended an explicit
reference type will now use `ExplicitRefType` with `Acccess.Read`.

For convenience, this change includes type aliases to map the old names
for these types over to the new ones (e.g., `using OutType =
OutParamType`) so that the change doesn't need to affect quite so many
lines of code. The `RefType` and `ConstRefType` names are intentionally
left undefined, since it woudl be unsafe to assume that existing use
sites should default to either of the two possible interpretations.

All use cases of `RefType` and `ConstRefType` (and their former shared
base class `RefTypeBase`) were audited and updated to refer to either
`RefParamType`/`ConstRefParamType` or `ExplicitRefType`, as appropriate
(based on whether the context of the code indicated it was working with
parameter-passing mode wrapper types, or explicit reference types).

In many (many) cases comments were added to the code that was updated
(and some unrelated code that needed to be audited along the way) to
note cases where there appears to be something fishy going on in the
compiler and/or there are obvious opportunities for next-step
improvement.

The `QualType` constructor used to infer l-value-ness when passed a
`RefType` or `ConstRefType`; that code was introduced to support
explicit reference types. The code was updated to consult the access
argument of an `ExplicitRefType` to try and determine the right
l-value-ness to use. There is some ambiguity about what should be done
in the case where the value of the generic argument representing the
access cannot be statically determined; a better solution may be needed.

Many other cases in the front-end that were working with `RefType` and
`ConstRefType` for explicit references also need to figure out
l-value-ness, and these were changed to rely on the logic already added
to `QualType` so that it wouldn't have to be duplicated. It isn't clear
if this structure is the best way to tackle the problem, but it seems to
at least be an upgrade over the more strictly ad-hoc logic that was in
place before.

Future Work
===========

IR-Level Work
-------------

The most obvious next step to take is that the split that was made in
the compiler front-end needs to be properly plumbed through all of the
back-end. There appears to be a lot of code in the back end of the
compiler that has made the same conflation of `ref` parameters and
explicit reference types that the front-end did. In practice, any uses
of `ExplicitRef<T>` in the front-end should desugar into plain
pointer-based code in the IR.

Clean Up Parameter-Passing Modes
--------------------------------

The code that handles different parameter-passing modes
(`ParameterDirection`s) and their wrapper types is somewhat scattered
and messy (as found while auditing use cases of `RefType`). A cleanup
pass is warranted to ensure that most code only needs to think about
`ParameterDirection`s. There should ideally be only a single operation
in the front-end that handles determining the `ParameterDirection` of a
parameter based on its modifiers. Similarly, there should be one
operation to wrap a value type based on a parameter direction, and one
operation to derive a `ParameterDirection` from the wrapper type.
Ideally, the accessors for `FuncType` should not provide unrestricted
access to the potentially-wrapped parameter types, and should instead
return some kind of `ParamInfo` struct that encodes both a
`ParameterDirection` and the unwrapped `Type` of the parameter.

Clean Up `QualType`
-------------------

A significant piece of future work that appears required is to
drastically clean up and improve the way that `QualType`s are represente
and handled in the front-end. There are currently various distinct
`bool` flags in `QualType` (some with very unclear meaning) and
differnet parts of the codebase consult/modify only subsets of them; a
clear enumeration of the "value categories" (to use the C++ terminology)
that Slang supports could be quite helpful. Naively, a `QualType` should
at least encode the basic information that a `Ptr` type encodes:

* A value type
* Allowed access (read-only, read-write, etc.)
* Address space

The main additional thing that a `QualType` needs is a way to
distinguish cases where an expression evaluates to:

* A reference to a memory location, where all the information from a
`Ptr` is relevant
* A simple value, such that the access and address space are irrelevant
* A reference to an abstract storage location (a `property`,
`subscript`, or an implicit conversion that needs to support being an
l-value), in which case address space is irrelevant and the "allowed
access" basically amounts to a listing of the accessors the storage
location supports

Eliminate Explicit Reference Types
----------------------------------

Finally, twe should eventually eliminate the `ExplicitRef<T>` type from
the core module (and all of the supporting code from the front-end),
since the feature is not a good fit for the Slang language. We should
find some other way to decorate operations in the builtin module that
need to returns a reference rather than a value (note how `ref`
accessors already avoided exposing explicit reference types, by design).

---------

Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>

1 files changed, 19 insertions, 2 deletions

diff --git a/source/slang/slang-type-layout.cpp b/source/slang/slang-type-layout.cpp
index 519b3ab06..0de6348bf 100644
--- a/source/slang/slang-type-layout.cpp
+++ b/source/slang/slang-type-layout.cpp
@@ -5088,9 +5088,26 @@ static TypeLayoutResult _createTypeLayout(TypeLayoutContext& context, Type* type
     }
     else if (auto optionalType = as<OptionalType>(type))
     {
-        // OptionalType should be laid out the same way as Tuple<T, bool>.
-        if (isNullableType(optionalType->getValueType()))
+        // Sometimes a type `T` has an unused bit pattern that
+        // can be used to represent the null/absent optional value,
+        // and for such types the size of an `Optional<T>` can be
+        // the same as a `T`, by making use of that unused pattern.
+        //
+        if (doesTypeHaveAnUnusedBitPatternThatCanBeUsedForOptionalRepresentation(
+                optionalType->getValueType()))
             return _createTypeLayout(context, optionalType->getValueType());
+
+        // For all other types, an `Optional<T>` is laid out more-or-less
+        // as tuple of a `T` and a `bool`.
+        //
+        // TODO(tfoley): This code implements the `(T,bool)` ordering,
+        // which provides more easy opportunities to generate compact
+        // layouts by using "tail padding" than the `(bool, T)` ordering.
+        // However the "natural layout" implementation does not match
+        // what is being done here (it uses the `(bool, T)` ordering).
+        // The discrepancy should probably be fixed, but doing so would
+        // technically be a breaking change.
+        //
         Array<Type*, 2> types =
             makeArray(optionalType->getValueType(), context.astBuilder->getBoolType());
         auto tupleType = context.astBuilder->getTupleType(types.getView());