Fix some crashing bugs around local variable declarations. (#385)

The basic problem here arises when a local variable is used either before its own declaration:

```hlsl
int a = b;
...
int b = 0;
```

or when a local variable is used *in* its own decalration:

```hlsl
int b = b;
```

In each case, Slang considers the scope of the `{}`-enclosed function body (or nested statement) as a whole, and so the lookup can "see" the declaration even if it is later in the same function.
This behavior isn't really correct for HLSL semantics, so the right long-term fix is to change our scoping rules, but for now users really just want the compiler to not crash on code like this, and give an error message that points at the issue.

This change makes both of the above examples print an error message saying that variable `b` was used before its declaration, which is accurate to the way that Slang is interpreting those code examples.
This is currently treated as a fatal error, so that compilation aborts right away, to avoid all of the downstream crashes that these cases were causing.

7 files changed, 88 insertions, 2 deletions

diff --git a/source/slang/check.cpp b/source/slang/check.cpp
index e2d519a4c..ea7f1be2a 100644
--- a/source/slang/check.cpp
+++ b/source/slang/check.cpp
@@ -481,7 +481,33 @@ namespace Slang
             if (decl->checkState == DeclCheckState::CheckingHeader)
             {
                 // We tried to reference the same declaration while checking it!
+                //
+                // TODO: we should ideally be tracking a "chain" of declarations
+                // being checked on the stack, so that we can report the full
+                // chain that leads from this declaration back to itself.
+                //
                 sink->diagnose(decl, Diagnostics::cyclicReference, decl);
+                return;
+            }
+
+            // Hack: if we are somehow referencing a local variable declaration
+            // before the line of code that defines it, then we need to diagnose
+            // an error.
+            //
+            // TODO: The right answer is that lookup should have been performed in
+            // the scope that was in place *before* the variable was declared, but
+            // this is a quick fix that at least alerts the user to how we are
+            // interpreting their code.
+            if (auto varDecl = decl.As<Variable>())
+            {
+                if (auto parenScope = varDecl->ParentDecl->As<ScopeDecl>())
+                {
+                    // TODO: This diagnostic should be emitted on the line that is referencing
+                    // the declaration. That requires `EnsureDecl` to take the requesting
+                    // location as a parameter.
+                    sink->diagnose(decl, Diagnostics::localVariableUsedBeforeDeclared, decl);
+                    return;
+                }
             }
 
             if (DeclCheckState::CheckingHeader > decl->checkState)
@@ -2151,7 +2177,7 @@ namespace Slang
         {
             for (auto decl : declGroup->decls)
             {
-                checkDecl(decl);
+                DeclVisitor::dispatch(decl);
             }
         }
 
@@ -2808,6 +2834,7 @@ namespace Slang
 
             stmt->varDecl->type.type = getSession()->getIntType();
             addModifier(stmt->varDecl, new ConstModifier());
+            stmt->varDecl->SetCheckState(DeclCheckState::Checked);
 
             RefPtr<IntVal> rangeBeginVal;
             RefPtr<IntVal> rangeEndVal;
diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index 7227156fa..bae501a3c 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -198,7 +198,8 @@ DIAGNOSTIC(33070, Error, expectedFunction, "expression preceding parenthesis of
 DIAGNOSTIC(30300, Error, assocTypeInInterfaceOnly, "'associatedtype' can only be defined in an 'interface'.")
 DIAGNOSTIC(30301, Error, globalGenParamInGlobalScopeOnly, "'__generic_param' can only be defined global scope.")
 // TODO: need to assign numbers to all these extra diagnostics...
-DIAGNOSTIC(39999, Error, cyclicReference, "cyclic reference '$0'.")
+DIAGNOSTIC(39999, Fatal, cyclicReference, "cyclic reference '$0'.")
+DIAGNOSTIC(39999, Fatal, localVariableUsedBeforeDeclared, "local variable '$0' is being used before its declaration.")
 
 DIAGNOSTIC(39999, Error, expectedIntegerConstantWrongType, "expected integer constant (found: '$0')")
 DIAGNOSTIC(39999, Error, expectedIntegerConstantNotConstant, "expression does not evaluate to a compile-time constant")
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index 4c9ecf8a8..e4bf251fc 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -1045,6 +1045,12 @@ SLANG_API int spCompile(
         int anyErrors = req->executeActions();
         return anyErrors;
     }
+    catch (Slang::AbortCompilationException&)
+    {
+        // This should only be thrown if we already emitted a fatal error
+        // message, so there is no reason to print something else.
+        return 1;
+    }
     catch (...)
     {
         req->mSink.diagnose(Slang::SourceLoc(), Slang::Diagnostics::compilationAborted);
diff --git a/tests/diagnostics/local-used-before-declared.slang b/tests/diagnostics/local-used-before-declared.slang
new file mode 100644
index 000000000..96d68b9eb
--- /dev/null
+++ b/tests/diagnostics/local-used-before-declared.slang
@@ -0,0 +1,21 @@
+//TEST:SIMPLE:-target dxbc -profile ps_5_0 -entry main
+
+// Early versions of the front-end had bugs when local
+// variables were used before their definition, or
+// were defined using a reference to themselves.
+//
+// This file tries to ensure that we emit proper error
+// diagnostics in these cases.
+
+void usedBeforeDeclared()
+{
+    // b is used before it is declared
+	float c = d + 1.0;
+	int d = 0;
+}
+
+float4 main()
+{
+	usedBeforeDeclared();
+	return 0;
+}
\ No newline at end of file
diff --git a/tests/diagnostics/local-used-before-declared.slang.expected b/tests/diagnostics/local-used-before-declared.slang.expected
new file mode 100644
index 000000000..a248fa969
--- /dev/null
+++ b/tests/diagnostics/local-used-before-declared.slang.expected
@@ -0,0 +1,6 @@
+result code = -1
+standard error = {
+tests/diagnostics/local-used-before-declared.slang(14): fatal error 39999: local variable 'd' is being used before its declaration.
+}
+standard output = {
+}
diff --git a/tests/diagnostics/local-used-in-own-declaration.slang b/tests/diagnostics/local-used-in-own-declaration.slang
new file mode 100644
index 000000000..9802acbb9
--- /dev/null
+++ b/tests/diagnostics/local-used-in-own-declaration.slang
@@ -0,0 +1,19 @@
+//TEST:SIMPLE:-target dxbc -profile ps_5_0
+
+// Early versions of the front-end had bugs when local
+// variables were used before their definition, or
+// were defined using a reference to themselves.
+//
+// This file tries to ensure that we emit proper error
+// diagnostics in these cases.
+
+void usedInOwnDeclaration()
+{
+	int e = e;
+}
+
+float4 main()
+{
+	usedInOwnDeclaration();
+	return 0;
+}
\ No newline at end of file
diff --git a/tests/diagnostics/local-used-in-own-declaration.slang.expected b/tests/diagnostics/local-used-in-own-declaration.slang.expected
new file mode 100644
index 000000000..a1421a158
--- /dev/null
+++ b/tests/diagnostics/local-used-in-own-declaration.slang.expected
@@ -0,0 +1,6 @@
+result code = -1
+standard error = {
+tests/diagnostics/local-used-in-own-declaration.slang(12): fatal error 39999: local variable 'e' is being used before its declaration.
+}
+standard output = {
+}