Fixes for IR generics (#1311)

* Fixes for IR generics

There are a few different fixes going on here (and a single test that covers all of them).

1. Fix optionality of trailing semicolon for `struct`s
======================================================

We have logic in the parser that tries to make a trailing `;` on a `struct` declaration optional. That logic is a bit subtle and couild potentially break non-idiomatic HLSL input, so we try to only trigger it for files written in Slang (and not HLSL). For command-line `slangc` this is based on the file extension (`.slang` vs. `.hlsl`), and for the API it is based on the user-specified language.

The missing piece here was that the path for handling `import`ed code was *not* setting the source language of imported files at all, and so those files were not getting opted into the Slang-specific behavior. As a result, `import`ed code couldn't leave off the semicolon.

2. Fix generic code involving empty `interface`s
================================================

We have logic that tries to only specialize "definitions," but the definition-vs-declaration distinction at the IR level has historically been slippery. One corner case was that a witness table for an interface with no methods would always be considered a declaration, because it was empty. The notion of what is/isn't a definition has been made more nuanced so that it amounts to two main points:

* If something is decorated as `[import(...)]`, it is not a definition

* If something is a generic/func (a declaration that should have a body), and it has no body, it is a declaration

Otherwise we consider anything a definition, which means that non-`[import(...)]` witness tables are now definitions whether or not they have anything in them.

3. Fix IR lowering for members of generic types
===============================================

The IR lowering logic was trying to be a little careful in how it recurisvely emitted "all" `Decl`s to IR  code. In particular, we don't want to recurse into things like function parameters, local variables, etc. since those can never be directly referenced by external code (they don't have linkage).

The existing logic was basically emitting everything at global scope, and then only recursing into (non-generic) type declarations. This created a problem where a method declared inside a generic `struct` would not be emitted to the IR for its own module at all *unless* it happened to be called by other code in the same module.

The fix here was to also recurse into the inner declaration of `GenericDecl`s. I also made the code recurse into any `AggTypeDeclBase` instead of just `AggTypeDecl`s, which means that members in `extension` declarations should not properly be emitted to the IR.

Conclusion
==========

These fixes should clear up some (but not all) cases where we might emit an `/* unhandled */` into output HLSL/GLSL. A future change will need to make that path a hard error and then clean up the remaining cases.

* fixup: tabs->spaces

7 files changed, 117 insertions, 12 deletions

diff --git a/source/slang/slang-ir-specialize.cpp b/source/slang/slang-ir-specialize.cpp
index d4f236138..cf475f1ff 100644
--- a/source/slang/slang-ir-specialize.cpp
+++ b/source/slang/slang-ir-specialize.cpp
@@ -267,6 +267,13 @@ struct SpecializationContext
         IRGeneric* g = generic;
         for(;;)
         {
+            // We can't specialize a generic if it is marked as
+            // being imported from an external module (in which
+            // case its definition is not available to us).
+            //
+            if(!isDefinition(g))
+                return false;
+
             // Given the generic `g`, we will find the value
             // it appears to return in its body.
             //
diff --git a/source/slang/slang-ir.cpp b/source/slang/slang-ir.cpp
index 45cbd65ff..9a44787ff 100644
--- a/source/slang/slang-ir.cpp
+++ b/source/slang/slang-ir.cpp
@@ -5068,6 +5068,13 @@ namespace Slang
         // the value they return.
         for(;;)
         {
+            // An instruciton marked `[import(...)]` cannot
+            // be a definition, since it is claiming that
+            // the actual body comes from another module.
+            //
+            if(val->findDecoration<IRImportDecoration>())
+                return false;
+
             auto genericInst = as<IRGeneric>(val);
             if(!genericInst)
                 break;
@@ -5079,13 +5086,12 @@ namespace Slang
             val = returnVal;
         }
 
-        // TODO: the logic here should probably
-        // be that anything with an `IRImportDecoration`
-        // is considered to be a declaration rather than definition.
-
+        // Some cases of instructions have structural
+        // rules about when they are considered to have
+        // a definition (e.g., a function must have a body).
+        //
         switch (val->op)
         {
-        case kIROp_WitnessTable:
         case kIROp_Func:
         case kIROp_Generic:
             return val->getFirstChild() != nullptr;
@@ -5093,14 +5099,15 @@ namespace Slang
         case kIROp_GlobalConstant:
             return cast<IRGlobalConstant>(val)->getValue() != nullptr;
 
-        case kIROp_StructType:
-        case kIROp_GlobalVar:
-        case kIROp_GlobalParam:
-            return true;
-
         default:
-            return false;
+            break;
         }
+
+        // In all other cases, if we have an instruciton
+        // that has *not* been marked for import, then
+        // we consider it to be a definition.
+
+        return true;
     }
 
     void markConstExpr(
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index 009e776bb..aaa9d1a4c 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -6711,13 +6711,17 @@ static void ensureAllDeclsRec(
     // Aggregate types are the main case where we can emit an outer declaration
     // and not the stuff nested inside of it.
     //
-    if(auto containerDecl = as<AggTypeDecl>(decl))
+    if(auto containerDecl = as<AggTypeDeclBase>(decl))
     {
         for (auto memberDecl : containerDecl->Members)
         {
             ensureAllDeclsRec(context, memberDecl);
         }
     }
+    else if (auto genericDecl = as<GenericDecl>(decl))
+    {
+        ensureAllDeclsRec(context, genericDecl->inner);
+    }
 }
 
 IRModule* generateIRForTranslationUnit(
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index aefe70a4a..1f8f79af2 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -1544,6 +1544,7 @@ RefPtr<Module> Linkage::loadModule(
     RefPtr<TranslationUnitRequest> translationUnit = new TranslationUnitRequest(frontEndReq);
     translationUnit->compileRequest = frontEndReq;
     translationUnit->moduleName = name;
+    translationUnit->sourceLanguage = SourceLanguage::Slang;
 
     auto module = translationUnit->getModule();
 
diff --git a/tests/language-feature/generics/struct-generic-value-param-import.slang b/tests/language-feature/generics/struct-generic-value-param-import.slang
new file mode 100644
index 000000000..edc1b0c39
--- /dev/null
+++ b/tests/language-feature/generics/struct-generic-value-param-import.slang
@@ -0,0 +1,19 @@
+// struct-generic-value-param-import.slang
+//TEST_IGNORE_FILE:
+
+// This file is used by `struct-generic-value-param.slang`,
+// and also incidentally tests that a trailing `;` is optional
+// for `struct` decalrations in Slang files, including
+// any `import`ed code.
+
+interface IData {}
+
+struct Data<let kCount : int> : IData
+{
+    int state;
+
+    [mutating] void doStuff()
+    {
+        state++;
+    }
+}
diff --git a/tests/language-feature/generics/struct-generic-value-param.slang b/tests/language-feature/generics/struct-generic-value-param.slang
new file mode 100644
index 000000000..857468165
--- /dev/null
+++ b/tests/language-feature/generics/struct-generic-value-param.slang
@@ -0,0 +1,63 @@
+// struct-generic-value-param.slang
+
+// This test reproduces a few bugs related to declarations
+// not being emitted IR and specialized correctly.
+//
+// First, it tests that a method in a generic `struct`
+// gets properly emitted to the IR of its own module.
+//
+// Second, it tests that witness tables for an empty
+// `interface` can be emitted and used to specialize
+// code with generic type parameters constrained to
+// those interfaces.
+
+// This file is attempting to stress-test use of a `struct`
+// type with a generic value parameter. In particular, it
+// it can reproduce a bug that was encountered by a user
+// when trying out the feature.
+
+//TEST(compute):COMPARE_COMPUTE:
+
+import struct_generic_value_param_import;
+
+Data<N> makeData<let N : int>( int val )
+{
+    Data<N> result = { val };
+    return result;
+}
+
+void doThings<D : IData>(D data, inout int v)
+{
+    v++;
+}
+
+int test(int val)
+{
+    var data = makeData<4>(val);
+
+    // Note: with the original bug, this call emitted
+    // as `/* unhandled */(data)` which meant the call
+    // acted as a no-op.
+    //
+    data.doStuff();
+
+    // Note: with the original bug, this call emitted
+    // as `/* uhandled */(data, val)` which is also
+    // a no-op (that happens to use `operator,`).
+    //
+    doThings(data, val);
+
+    return data.state + val*16;
+}
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer
+RWStructuredBuffer<int> outputBuffer;
+
+[numthreads(4, 1, 1)]
+void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+    uint tid = dispatchThreadID.x;
+    int inVal = tid;
+    int outVal = test(inVal);
+    outputBuffer[tid] = outVal;
+}
diff --git a/tests/language-feature/generics/struct-generic-value-param.slang.expected.txt b/tests/language-feature/generics/struct-generic-value-param.slang.expected.txt
new file mode 100644
index 000000000..e4511c4c7
--- /dev/null
+++ b/tests/language-feature/generics/struct-generic-value-param.slang.expected.txt
@@ -0,0 +1,4 @@
+11
+22
+33
+44