Diagnose attempts to write to fields in methods (#530)

* Diagnose attempts to write to fields in methods

Work on #529

This helps to avoid the case where a Slang user writes a struct with helpful `setter` methods, and finds that it doesn't work as expected because the `this` parameter is currently handled like an `in` parameter (passed by value, but mutable in the callee).

Fixing this issue actually involved making a more broad fix to how l-value-ness is propagated. The existing checking logic was assuming that l-value-ness is just a property of a particular member declaration (e.g., a field is either mutable or not), and didn't take into account whether the "base expression" was mutable. This change fixes that oversight, which might lead to additional errors being issued if we aren't correctly making things mutable when we should.

A `ThisExpr` was already immutable by default, so that part didn't actually need to change. Just propagating its immutability through was enough.

As an additional assistance to users, I have added an extra diagnostic that triggers when a "destination of assignment is not an l-value" error occurs and the left-hand-side expression seems to be based on `this` (whether implicitly or explicitly). This will ideally help users to understand that the "setter" idiom is not yet supported.

* Fixed setRadius typo

2 files changed, 60 insertions, 1 deletions

diff --git a/source/slang/check.cpp b/source/slang/check.cpp
index 5fd8be2d5..6f9aa8adc 100644
--- a/source/slang/check.cpp
+++ b/source/slang/check.cpp
@@ -204,6 +204,19 @@ namespace Slang
                     expr->BaseExpression = baseExpr;
                     expr->name = declRef.GetName();
                     expr->declRef = declRef;
+
+                    // When referring to a member through an expression,
+                    // the result is only an l-value if both the base
+                    // expression and the member agree that it should be.
+                    //
+                    // We have already used the `QualType` from the member
+                    // above (that is `type`), so we need to take the
+                    // l-value status of the base expression into account now.
+                    if(!baseExpr->type.IsLeftValue)
+                    {
+                        expr->type.IsLeftValue = false;
+                    }
+
                     return expr;
                 }
             }
@@ -3991,6 +4004,46 @@ namespace Slang
                 else
                 {
                     getSink()->diagnose(expr, Diagnostics::assignNonLValue);
+
+                    // As a special case, check if the LHS expression is derived
+                    // from a `this` parameter (implicitly or explicitly), which
+                    // is immutable. We can give the user a bit more context into
+                    // what is going on.
+                    //
+                    // We will try to handle expressions of the form:
+                    //
+                    //      e ::= "this"
+                    //          | e . name
+                    //          | e [ expr ]
+                    //
+                    // We will unwrap the `e.name` and `e[expr]` cases in a loop.
+                    RefPtr<Expr> e = expr->left;
+                    for(;;)
+                    {
+                        if(auto memberExpr = e.As<MemberExpr>())
+                        {
+                            e = memberExpr->BaseExpression;
+                        }
+                        else if(auto subscriptExpr = e.As<IndexExpr>())
+                        {
+                            e = subscriptExpr->BaseExpression;
+                        }
+                        else
+                        {
+                            break;
+                        }
+                    }
+                    //
+                    // Now we check to see if we have a `this` expression,
+                    // and if it is immutable.
+                    if(auto thisExpr = e.As<ThisExpr>())
+                    {
+                        if(!thisExpr->type.IsLeftValue)
+                        {
+                            getSink()->diagnose(thisExpr, Diagnostics::thisIsImmutableByDefault);
+                        }
+                    }
+
                 }
             }
             expr->type = type;
@@ -7800,7 +7853,12 @@ namespace Slang
         {
             QualType qualType;
             qualType.type = GetType(varDeclRef);
-            qualType.IsLeftValue = true; // TODO(tfoley): allow explicit `const` or `let` variables
+
+            bool isLValue = true;
+            if(varDeclRef.getDecl()->FindModifier<ConstModifier>())
+                isLValue = false;
+
+            qualType.IsLeftValue = isLValue;
             return qualType;
         }
         else if (auto typeAliasDeclRef = declRef.As<TypeDefDecl>())
diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index c77e75817..0dbacafe0 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -193,6 +193,7 @@ DIAGNOSTIC(30035, Error, componentOverloadTypeMismatch, "'$0': type of overloade
 DIAGNOSTIC(30041, Error, bitOperationNonIntegral, "bit operation: operand must be integral type.")
 DIAGNOSTIC(30047, Error, argumentExpectedLValue, "argument passed to parameter '$0' must be l-value.")
 DIAGNOSTIC(30048, Note,  implicitCastUsedAsLValue, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value")
+DIAGNOSTIC(30049, Note,  thisIsImmutableByDefault, "a 'this' parameter is currently immutable in Slang")
 
 DIAGNOSTIC(30051, Error, invalidValueForArgument, "invalid value for argument '$0'")
 DIAGNOSTIC(30052, Error, invalidSwizzleExpr, "invalid swizzle pattern '$0' on type '$1'")