summaryrefslogtreecommitdiffstats
path: root/source
diff options
context:
space:
mode:
authorTim Foley <tfoley@nvidia.com>2017-08-25 15:18:10 -0700
committerTim Foley <tfoley@nvidia.com>2017-08-25 15:48:49 -0700
commit8c68434d6fa6ff1b2e54586637869cceace9d1bb (patch)
tree69f257a9357fa1962ec4a76b258c06abdafef1bc /source
parent7e05e062a0b7c39dbce6e850227d2038aca2f38e (diff)
Fix some resources-in-structs bugs
Fixes #171 Fixes #172 These two bugs related to bad logic in handling of splitting resource-containing `cbuffer` declarations. - Issue #171 was the case where a `cbuffer` *only* had resource fields, in which case we crashed whenever referencing any field (some code was assuming there had to be non-resource fields) - Issue #172 was a case where two fields were declared with a single declaration (`Texture2D a, b;`), and the logic we had for tracking resource-type fields was accidentally tagging *both* fields with a single modifier so that field `b` would get confused for `a` in some contexts, and attempts to access `b` would crash. Both issues are now fixed, and regression tests have been added.
Diffstat (limited to 'source')
-rw-r--r--source/slang/lower.cpp29
-rw-r--r--source/slang/parser.cpp23
2 files changed, 51 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..c3969be42 100644
--- a/source/slang/parser.cpp
+++ b/source/slang/parser.cpp
@@ -549,9 +549,30 @@ 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
+ modifier->next = *modifierLink;
*modifierLink = modifier;
modifierLink = &modifier->next;
}