Diagnose attempts to call instance methods from static methods (#1330)

Currently we fail to diagnose code that calls an instance method from a static method using implicit `this`, and instead crash during lowering of the AST to the IR.

This change introduces a bit more detail to the "this parameter mode" that is computed during lookup, so that it differentiates three cases. The existing two cases of a mutable `this` and immutable `this` remain, but we add a third case where the "this parameter mode" only allows for a reference to the `This` type.

When turning lookup "breadcrumb" information into actual expressions, we respect this setting to construct either a `This` or `this` expression.

In order to actually diagnose the incorrect reference, I had to add code around an existing `TODO` comment that noted how we should diagnose attempts to refer to instance members through a type. Enabling that diagnostic revealed a missing case needed by generics (including those in the stdlib) - a type-constraint member is always referenced statically.

Putting the diagnostic for a static reference to a non-static member in its new bottleneck location meant that some code higher up the call static that handles explicit static member references had to be tweaked to not produce double error messages.

This change includes a new diagnostic test to show that we now give an error message on code that makes this mistake, instead of crashing.

1 files changed, 41 insertions, 9 deletions

diff --git a/source/slang/slang-lookup.cpp b/source/slang/slang-lookup.cpp
index 5f2927ccb..7fefa1241 100644
--- a/source/slang/slang-lookup.cpp
+++ b/source/slang/slang-lookup.cpp
@@ -670,10 +670,10 @@ static void _lookUpInScopes(
             if (auto aggTypeDeclBaseRef = containerDeclRef.as<AggTypeDeclBase>())
             {
                 // When reconstructing the final expression for a result
-                // looked up through the tyep or extension, we will need
+                // looked up through the type or extension, we will need
                 // a `this` expression (or a `This` type expression) to
                 // mark the base of the member reference, so we create
-                // a "breadcrumb" here to trakc that fact.
+                // a "breadcrumb" here to track that fact.
                 //
                 BreadcrumbInfo breadcrumb;
                 breadcrumb.kind = LookupResultItem::Breadcrumb::Kind::This;
@@ -714,25 +714,57 @@ static void _lookUpInScopes(
 
             // Before we proceed up to the next outer scope to perform lookup
             // again, we need to consider what the current scope tells us
-            // about how to interpret uses of `this`. For example, if
-            // we are inside a `[mutating]` method, then the implicit `this`
-            // that we use for lookup should be an l-value.
+            // about how to interpret uses of implicit `this` or `This`. For
+            // example, if we are inside a `[mutating]` method, then the implicit
+            // `this` that we use for lookup should be an l-value.
+            //
+            // Similarly, if we look up a member in a type from the scope
+            // of some nested type, then there shouldn't be an implicit `this`
+            // expression for the outer type, but instead an implicit `This`.
             //
             if( containerDeclRef.is<ConstructorDecl>() )
             {
-                thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Mutating;
+                // In the context of an `__init` declaration, the members of
+                // the surrounding type are accessible through a mutable `this`.
+                //
+                thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::MutableValue;
             }
             else if( auto funcDeclRef = containerDeclRef.as<FunctionDeclBase>() )
             {
-                if( funcDeclRef.getDecl()->HasModifier<MutatingAttribute>() )
+                // The implicit `this`/`This` for a function-like declaration
+                // depends on modifiers attached to the declaration.
+                //
+                if( funcDeclRef.getDecl()->HasModifier<HLSLStaticModifier>() )
                 {
-                    thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Mutating;
+                    // A `static` method only has access to an implicit `This`,
+                    // and does not have a `this` expression available.
+                    //
+                    thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Type;
+                }
+                else if( funcDeclRef.getDecl()->HasModifier<MutatingAttribute>() )
+                {
+                    // In a non-`static` method marked `[mutating]` there is
+                    // an implicit `this` parameter that is mutable.
+                    //
+                    thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::MutableValue;
                 }
                 else
                 {
-                    thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Default;
+                    // In all other cases, there is an implicit `this` parameter
+                    // that is immutable.
+                    //
+                    thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::ImmutableValue;
                 }
             }
+            else if( containerDeclRef.as<AggTypeDeclBase>() )
+            {
+                // When lookup moves from a nested typed declaration to an
+                // outer scope, there is no ability to use an implicit `this`
+                // expression, and we have only the `This` type available.
+                //
+                thisParameterMode = LookupResultItem::Breadcrumb::ThisParameterMode::Type;
+            }
+            // TODO: What other cases need to be enumerated here?
         }
 
         if (result.isValid())