Merge pull request #173 from tfoleyNV/resources-in-structs-fixes

Fix some resources-in-structs bugs

5 files changed, 142 insertions, 1 deletions

diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp
index 70c153902..a1b6dcf17 100644
--- a/source/slang/lower.cpp
+++ b/source/slang/lower.cpp
@@ -308,6 +308,11 @@ struct LoweredExpr
         return (getFlavor() == Flavor::VaryingTuple) ? getVaryingTupleExpr() : nullptr;
     }
 
+    bool operator!()
+    {
+        return !value;
+    }
+
 private:
     RefPtr<RefObject>   value;
     Flavor              flavor;
@@ -1820,6 +1825,12 @@ struct LoweringVisitor
                 baseTuple->primaryExpr = loweredPrimary;
                 return baseTuple;
             }
+            else
+            {
+                // No primary expression? Then there is nothing
+                // to dereference.
+                return baseTuple;
+            }
         }
         else if (auto baseVaryingTuple = loweredBase.asVaryingTuple())
         {
@@ -1847,8 +1858,16 @@ struct LoweringVisitor
     LoweredExpr visitMemberExpr(
         MemberExpr* expr)
     {
+        assert(expr->BaseExpression);
         auto loweredBase = lowerExprOrTuple(expr->BaseExpression);
 
+        if( !loweredBase )
+        {
+            loweredBase = lowerExprOrTuple(expr->BaseExpression);
+        }
+
+        assert(loweredBase);
+
         auto loweredDeclRef = translateDeclRef(expr->declRef);
 
 
@@ -1872,7 +1891,11 @@ struct LoweringVisitor
                 }
 
                 if (!tupleFieldMod->hasAnyNonTupleFields)
+                {
+                    // We need to have found something!
+                    assert(tupleFieldExpr);
                     return tupleFieldExpr;
+                }
 
                 auto tupleFieldTupleExpr = tupleFieldExpr.asTuple();
                 SLANG_RELEASE_ASSERT(tupleFieldTupleExpr);
@@ -1885,6 +1908,8 @@ struct LoweringVisitor
                 loweredPrimaryExpr->declRef = loweredDeclRef.As<Decl>();
                 loweredPrimaryExpr->name = expr->name;
 
+                assert(loweredPrimaryExpr->BaseExpression);
+
                 tupleFieldTupleExpr->primaryExpr = loweredPrimaryExpr;
                 return tupleFieldTupleExpr;
             }
@@ -1892,6 +1917,7 @@ struct LoweringVisitor
             // If the field was a non-tuple field, then we can
             // simply fall through to the ordinary case below.
             loweredBase = LoweredExpr(baseTuple->primaryExpr);
+            assert(baseTuple->primaryExpr);
         }
         else if (auto baseVaryingTuple = loweredBase.asVaryingTuple())
         {
@@ -1901,6 +1927,7 @@ struct LoweringVisitor
                 if (expr->declRef.getDecl() == elem.originalFieldDeclRef.getDecl())
                 {
                     // We found the field!
+                    assert(elem.expr);
                     return elem.expr;
                 }
             }
@@ -1916,6 +1943,8 @@ struct LoweringVisitor
         loweredExpr->declRef = loweredDeclRef.As<Decl>();
         loweredExpr->name = expr->name;
 
+        assert(loweredExpr->BaseExpression);
+
         return LoweredExpr(loweredExpr);
     }
 
diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp
index 7ea3fe864..e232880e9 100644
--- a/source/slang/parser.cpp
+++ b/source/slang/parser.cpp
@@ -549,8 +549,39 @@ namespace Slang
     {
         RefPtr<Modifier>*& modifierLink = *ioModifierLink;
 
-        while(*modifierLink)
+        // We'd like to add the modifier to the end of the list,
+        // but we need to be careful, in case there is a "shared"
+        // section of modifiers for multiple declarations.
+        //
+        // TODO: This whole approach is a mess because we are "accidentally quadratic"
+        // when adding many modifiers.
+        for(;;)
+        {
+            // At end of the chain? Done.
+            if(!*modifierLink)
+                break;
+
+            // About to look at shared modifiers? Done.
+            RefPtr<Modifier> linkMod = *modifierLink;
+            if(linkMod.As<SharedModifiers>())
+                break;
+
+            // Otherwise: keep traversing the modifier list.
             modifierLink = &(*modifierLink)->next;
+        }
+
+        // Splice the modifier into the linked list
+
+        // We need to deal with the case where the modifeir to
+        // be spliced in might actually be a modifier *list*,
+        // so that we actually want to splice in at the
+        // end of the new list...
+        auto spliceLink = &modifier->next;
+        while(*spliceLink)
+            spliceLink = &(*spliceLink)->next;
+
+        // Do the splice.
+        *spliceLink = *modifierLink;
 
         *modifierLink = modifier;
         modifierLink = &modifier->next;
diff --git a/tests/bugs/gh-171.slang b/tests/bugs/gh-171.slang
new file mode 100644
index 000000000..b37f77ce2
--- /dev/null
+++ b/tests/bugs/gh-171.slang
@@ -0,0 +1,30 @@
+//TEST:COMPARE_HLSL: -profile ps_5_0 -entry main -target dxbc-assembly -split-mixed-types
+// Make sure we don't crash when desugaring resources
+// in structs when a `cbuffer` only contains resources.
+
+#ifdef __SLANG__
+
+cbuffer C
+{
+	Texture2D t;
+	SamplerState s;
+};
+
+float4 main(float2 uv: UV) : SV_Target
+{
+	return t.Sample(s, uv);
+}
+
+#else
+
+Texture2D SLANG_parameterBlock_C_t : register(t0);
+SamplerState SLANG_parameterBlock_C_s : register(s0);
+
+float4 main(float2 uv: UV) : SV_Target
+{
+	return SLANG_parameterBlock_C_t.Sample(SLANG_parameterBlock_C_s, uv);
+}
+
+#endif
+
+
diff --git a/tests/bugs/gh-172.slang b/tests/bugs/gh-172.slang
new file mode 100644
index 000000000..f898b5f4b
--- /dev/null
+++ b/tests/bugs/gh-172.slang
@@ -0,0 +1,41 @@
+//TEST:COMPARE_HLSL: -profile ps_5_0 -entry main -target dxbc-assembly -split-mixed-types
+
+// Make sure we don't crash when desugaring resource in structs,
+// when the user also declares multiple variables with a
+// single declaration.
+
+#ifdef __SLANG__
+
+cbuffer C
+{
+	Texture2D t0, t1;
+	SamplerState s;
+	float2 uv;
+};
+
+float4 main() : SV_Target
+{
+	return t0.Sample(s, uv)
+         + t1.Sample(s, uv);
+}
+
+#else
+
+cbuffer C : register(b0)
+{
+	float2 uv;
+};
+
+Texture2D SLANG_parameterBlock_C_t0 : register(t0);
+Texture2D SLANG_parameterBlock_C_t1 : register(t1);
+SamplerState SLANG_parameterBlock_C_s : register(s0);
+
+float4 main() : SV_Target
+{
+	return SLANG_parameterBlock_C_t0.Sample(SLANG_parameterBlock_C_s, uv)
+	     + SLANG_parameterBlock_C_t1.Sample(SLANG_parameterBlock_C_s, uv);
+}
+
+#endif
+
+
diff --git a/tools/slang-test/main.cpp b/tools/slang-test/main.cpp
index 6b7da5844..6ee255571 100644
--- a/tools/slang-test/main.cpp
+++ b/tools/slang-test/main.cpp
@@ -873,6 +873,16 @@ TestResult runHLSLComparisonTest(TestInput& input)
     {
         String actualOutputPath = outputStem + ".actual";
         Slang::File::WriteAllText(actualOutputPath, actualOutput);
+
+        if (options.outputMode == kOutputMode_AppVeyor)
+        {
+            fprintf(stderr, "ERROR:\n"
+                "EXPECTED{{{\n%s}}}\n"
+                "ACTUAL{{{\n%s}}}\n",
+                expectedOutput.Buffer(),
+                actualOutput.Buffer());
+            fflush(stderr);
+        }
     }
 
     return result;