Add basic support for [mutating] methods (#667)

By default, when writing a "method" (aka "member function") in Slang, the `this` parameter is implicitly an `in` parameter. So this:

```hlsl
struct Foo
{
    int state;
    int getState() { return state; }
    void setState(int s) { state = s; }
};
```

is desugared into something like this:

```hlsl
struct Foo { int state };
int Foo_getState(Foo this) { return this.state; }

// BAD:
void Foo_setState(Foo this, int s) { this.state = s; }
```

That "setter" doesn't really do what was intended. It modifies a local copy of type `Foo`, because `in` parameters in HLSL represent by-value copy-in semantics, and are mutable in the body the function. Slang was updated to give a static error on the original code to catch this kind of mistake (so that `this` parameters are unlike ordinary function parameters, and no longer mutable).

Of course, sometimes users *want* a mutable `this` parameter. Rather than make a mutable `this` the default (there are arguments both for and against this), this change adds a new attribute `[mutating]` that can be put on a method (member function) to indicate that its `this` parameter should be an `in out` parameter:

```hlsl
[mutating] void setState(int s) { state = s; }
```

The above will translate to, more or less:

```hlsl
void Foo_setState(inout Foo this, int s) { this.state = s; }
```

One added detail is that `[mutating]` can also be used on interface requirements, with the same semantics. A `[mutating]` requirement can be satisfied with a `[mutating]` or non-`[mutating]` method, while a non-`[mutating]` requirement can't be satisfied with a `[mutating]` method (the call sites would not expect mutation to happen).

The design of `[mutating]` here is heavily influenced by the equivalent `mutating` keyword in Swift.

Notes on the implementation:

* Adding the new attribute was straightforward using the existing support, but I had to change around where attributes get checked in the overall sequencing of static checks, because attributes were being checked *after* function bodies, but with this change I need to look at semantically-checked attributes to determine the mutability of `this`

* The check to restrict it so that `[mutating]` methods cannot satisfy non-`[mutating]` requirements was easy to add, but it points out the fact that there is a huge TODO comment where the actual checking of method *signatures* is supposed to happen. That is a bug waiting to bite users and needs to be fixed!

* While we had special-case logic to detect attempts to modify state accessed through an immutable `this` (e.g., `this.state = s`), that logic didn't trigger when the mutation happened through a function/operator call (e.g., `this.state += s`), so this change factors out the validation logic for that case and calls through to it from both the assignment and `out` argument cases.

* The error message for the special-case check was updated to note that the user could apply `[mutating]` to their function declaration to get rid of the error.

* The semantic checking logic for an explicit `this` expression was already walking up through the scopes (created during parsing) and looking for a scope that represents an outer type declaration that `this` might be referring to. We simply extend it to note when it passes through the scope for a function or similar declaration (`FunctionDeclBase`) and check for the `[mutating]` attribute. If the attribute is seen, it returns a mutable `this` expression, and otherwise leaves it immutable.

* The IR lowering logic then needed to be updated so that when adding an IR-level parameter to represent `this`, it gives it the appropriate "direction" based on the attributes of the function declaration being lowered. The rest of the IR logic works as-is, because it will treat `this` just like an other parameter (whether it is `in` or `inout`).

* This biggest chunk of work was the "implicit `this`" case, because ordinary name lookup may resolve an expression like `state` into `this.state`, so that the `this` expression comes out of "thin air." To handle this case, I extended the structure of the "breadcrumbs" that come along with a lookup result (the breadcrumbs are used for any case where a single identifier like `state` needs to be embellished to a more complex expression as a result of lookup), so that it can identify whether a `Breadcrumb::Kind::This` node comes from a `[mutating]` context or not. Similar to the logic for an explicit `this`, we handle this by noting when we pass through a `FunctionDeclBase` when moving up through scopes, and look for the `[mutating]` attribute on it. The rest of the work was just plumbing the additional state through.

1 files changed, 18 insertions, 12 deletions

diff --git a/source/slang/lower-to-ir.cpp b/source/slang/lower-to-ir.cpp
index 7e22c4694..86cf63b25 100644
--- a/source/slang/lower-to-ir.cpp
+++ b/source/slang/lower-to-ir.cpp
@@ -4494,19 +4494,10 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
     // parameter for the enclosing type:
     //
     void addThisParameter(
+        ParameterDirection  direction,
         Type*               type,
         ParameterLists*     ioParameterLists)
     {
-        // For now we make any `this` parameter default to `in`. Eventually
-        // we should add a way for the user to opt in to mutability of `this`
-        // in cases where they really want it.
-        //
-        // Note: an alternative here might be to have the built-in types like
-        // `Texture2D` actually use `class` declarations and say that the
-        // `this` parameter for a class type is always `in`, while `struct`
-        // types can default to `in out`.
-        ParameterDirection direction = kParameterDirection_In;
-
         ParameterInfo info;
         info.type = type;
         info.decl = nullptr;
@@ -4516,6 +4507,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         ioParameterLists->params.Add(info);
     }
     void addThisParameter(
+        ParameterDirection  direction,
         AggTypeDecl*        typeDecl,
         ParameterLists*     ioParameterLists)
     {
@@ -4526,6 +4518,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         auto declRef = createDefaultSpecializedDeclRef(typeDecl);
         RefPtr<Type> type = DeclRefType::Create(context->getSession(), declRef);
         addThisParameter(
+            direction,
             type,
             ioParameterLists);
     }
@@ -4558,13 +4551,26 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             // parameter corresponding to the outer declaration.
             if( innerMode != kParameterListCollectMode_Static )
             {
+                // For now we make any `this` parameter default to `in`.
+                //
+                ParameterDirection direction = kParameterDirection_In;
+                //
+                // Applications can opt in to a mutable `this` parameter,
+                // by applying the `[mutating]` attribute to their
+                // declaration.
+                //
+                if( decl->HasModifier<MutatingAttribute>() )
+                {
+                    direction = kParameterDirection_InOut;
+                }
+
                 if( auto aggTypeDecl = dynamic_cast<AggTypeDecl*>(parentDecl) )
                 {
-                    addThisParameter(aggTypeDecl, ioParameterLists);
+                    addThisParameter(direction, aggTypeDecl, ioParameterLists);
                 }
                 else if( auto extensionDecl = dynamic_cast<ExtensionDecl*>(parentDecl) )
                 {
-                    addThisParameter(extensionDecl->targetType, ioParameterLists);
+                    addThisParameter(direction, extensionDecl->targetType, ioParameterLists);
                 }
             }
         }