summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2020-02-25 08:12:16 -0800
committerGitHub <noreply@github.com>2020-02-25 08:12:16 -0800
commit6308a1224672944220a1fee34ae22f70212703a0 (patch)
tree25d2011e23e64d2794dc1b07e2fa70785e13700f
parentc4a32e34f68c5d202b8a4a867963a1ecbb03b96e (diff)
Fix a crash when a generic value argument isn't constant (#1241)
This arose when a user tried to specialize the DXR 1.1 `RayQuery` type to a local variable: ```hlsl RAY_FLAG rayFlags = RAY_FLAG_CULL_FRONT_FACING_TRIANGLES | RAY_FLAG_CULL_NON_OPAQUE; RayQuery<rayFlags> query; ``` In this case, we issued an error around `rayFlags` not being a constant as expected, but then we also crashes later on in checking because the `DeclRef` that was being used for the type had a null pointer for the generic argument corresponding to `rayFlags`. The main fix here was thus to add an `ErrorIntVal` case that can be used to represent something that should be an `IntVal` but where there was some kind of error in the input code so that the actual value isn't known to the compiler. A secondary fix here is that we were issuing error messages about expecting a constant for a parameter like `rayFlags` there *twice*, and one of those times was during the `JustChecking` part of overload resolution (when we are not supposed to emit any diagnostics). I fixed that up by allowing the `DiagnosticSink` to be used to be passed down explicitly (and allowing it to be null), while also leaving behind overloaded functions with the old signatures so that all the existing logic can continue to work unmodified.
-rw-r--r--source/slang/slang-check-expr.cpp11
-rw-r--r--source/slang/slang-check-impl.h2
-rw-r--r--source/slang/slang-check-overload.cpp2
-rw-r--r--source/slang/slang-check-type.cpp15
-rw-r--r--source/slang/slang-syntax.cpp28
-rw-r--r--source/slang/slang-val-defs.h18
-rw-r--r--tests/bugs/ray-flags-non-constant.slang12
-rw-r--r--tests/bugs/ray-flags-non-constant.slang.expected6
8 files changed, 89 insertions, 5 deletions
diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp
index 524bab4e4..adfc36641 100644
--- a/source/slang/slang-check-expr.cpp
+++ b/source/slang/slang-check-expr.cpp
@@ -828,7 +828,7 @@ namespace Slang
return TryConstantFoldExpr(exp);
}
- RefPtr<IntVal> SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr)
+ RefPtr<IntVal> SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr, DiagnosticSink* sink)
{
// No need to issue further errors if the expression didn't even type-check.
if(IsErrorExpr(inExpr)) return nullptr;
@@ -840,13 +840,18 @@ namespace Slang
if(IsErrorExpr(expr)) return nullptr;
auto result = TryCheckIntegerConstantExpression(expr.Ptr());
- if (!result)
+ if (!result && sink)
{
- getSink()->diagnose(expr, Diagnostics::expectedIntegerConstantNotConstant);
+ sink->diagnose(expr, Diagnostics::expectedIntegerConstantNotConstant);
}
return result;
}
+ RefPtr<IntVal> SemanticsVisitor::CheckIntegerConstantExpression(Expr* inExpr)
+ {
+ return CheckIntegerConstantExpression(inExpr, getSink());
+ }
+
RefPtr<IntVal> SemanticsVisitor::CheckEnumConstantExpression(Expr* expr)
{
// No need to issue further errors if the expression didn't even type-check.
diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h
index 9363066d1..f51e62699 100644
--- a/source/slang/slang-check-impl.h
+++ b/source/slang/slang-check-impl.h
@@ -392,6 +392,7 @@ namespace Slang
RefPtr<Type> ExtractGenericArgType(RefPtr<Expr> exp);
+ RefPtr<IntVal> ExtractGenericArgInteger(RefPtr<Expr> exp, DiagnosticSink* sink);
RefPtr<IntVal> ExtractGenericArgInteger(RefPtr<Expr> exp);
RefPtr<Val> ExtractGenericArgVal(RefPtr<Expr> exp);
@@ -834,6 +835,7 @@ namespace Slang
// Enforce that an expression resolves to an integer constant, and get its value
RefPtr<IntVal> CheckIntegerConstantExpression(Expr* inExpr);
+ RefPtr<IntVal> CheckIntegerConstantExpression(Expr* inExpr, DiagnosticSink* sink);
RefPtr<IntVal> CheckEnumConstantExpression(Expr* expr);
diff --git a/source/slang/slang-check-overload.cpp b/source/slang/slang-check-overload.cpp
index 20d1af09a..ace495512 100644
--- a/source/slang/slang-check-overload.cpp
+++ b/source/slang/slang-check-overload.cpp
@@ -196,7 +196,7 @@ namespace Slang
}
arg = coerce(GetType(valParamRef), arg);
- auto val = ExtractGenericArgInteger(arg);
+ auto val = ExtractGenericArgInteger(arg, context.mode == OverloadResolveContext::Mode::JustTrying ? nullptr : getSink());
checkedArgs.add(val);
}
else
diff --git a/source/slang/slang-check-type.cpp b/source/slang/slang-check-type.cpp
index b92f9a8f7..68e14c462 100644
--- a/source/slang/slang-check-type.cpp
+++ b/source/slang/slang-check-type.cpp
@@ -105,9 +105,22 @@ namespace Slang
return ExpectAType(exp);
}
+ RefPtr<IntVal> SemanticsVisitor::ExtractGenericArgInteger(RefPtr<Expr> exp, DiagnosticSink* sink)
+ {
+ RefPtr<IntVal> val = CheckIntegerConstantExpression(exp.Ptr(), sink);
+ if(val) return val;
+
+ // If the argument expression could not be coerced to an integer
+ // constant expression in context, then we will instead construct
+ // a dummy "error" value to represent the result.
+ //
+ val = new ErrorIntVal();
+ return val;
+ }
+
RefPtr<IntVal> SemanticsVisitor::ExtractGenericArgInteger(RefPtr<Expr> exp)
{
- return CheckIntegerConstantExpression(exp.Ptr());
+ return ExtractGenericArgInteger(exp, getSink());
}
RefPtr<Val> SemanticsVisitor::ExtractGenericArgVal(RefPtr<Expr> exp)
diff --git a/source/slang/slang-syntax.cpp b/source/slang/slang-syntax.cpp
index 502425da8..c05f9aa0f 100644
--- a/source/slang/slang-syntax.cpp
+++ b/source/slang/slang-syntax.cpp
@@ -1346,6 +1346,34 @@ void Type::accept(IValVisitor* visitor, void* extra)
return this;
}
+ // ErrorIntVal
+
+ bool ErrorIntVal::EqualsVal(Val* val)
+ {
+ if( auto errorIntVal = as<ErrorIntVal>(val) )
+ {
+ return true;
+ }
+ return false;
+ }
+
+ String ErrorIntVal::ToString()
+ {
+ return "<error>";
+ }
+
+ int ErrorIntVal::GetHashCode()
+ {
+ return int(typeid(this).hash_code());
+ }
+
+ RefPtr<Val> ErrorIntVal::SubstituteImpl(SubstitutionSet subst, int* ioDiff)
+ {
+ SLANG_UNUSED(subst);
+ SLANG_UNUSED(ioDiff);
+ return this;
+ }
+
// Substitutions
RefPtr<Substitutions> GenericSubstitution::applySubstitutionsShallow(SubstitutionSet substSet, RefPtr<Substitutions> substOuter, int* ioDiff)
diff --git a/source/slang/slang-val-defs.h b/source/slang/slang-val-defs.h
index b9d5188ed..5024554d4 100644
--- a/source/slang/slang-val-defs.h
+++ b/source/slang/slang-val-defs.h
@@ -41,6 +41,24 @@ SYNTAX_CLASS(GenericParamIntVal, IntVal)
)
END_SYNTAX_CLASS()
+ /// An unknown integer value indicating an erroneous sub-expression
+SYNTAX_CLASS(ErrorIntVal, IntVal)
+
+ // TODO: We should probably eventually just have an `ErrorVal` here
+ // and have all `Val`s that represent ordinary values hold their
+ // `Type` so that we can have an `ErrorVal` of any type.
+
+ RAW(
+ ErrorIntVal()
+ {}
+
+ virtual bool EqualsVal(Val* val) override;
+ virtual String ToString() override;
+ virtual int GetHashCode() override;
+ virtual RefPtr<Val> SubstituteImpl(SubstitutionSet subst, int* ioDiff) override;
+)
+END_SYNTAX_CLASS()
+
// A witness to the fact that some proposition is true, encoded
// at the level of the type system.
//
diff --git a/tests/bugs/ray-flags-non-constant.slang b/tests/bugs/ray-flags-non-constant.slang
new file mode 100644
index 000000000..e2197c6cd
--- /dev/null
+++ b/tests/bugs/ray-flags-non-constant.slang
@@ -0,0 +1,12 @@
+// ray-flags-non-constant.slang
+
+// Regression test for a compiler crash occuring when a generic with integer
+// value parameters is specialized to a non-constant (and hence invalid) value.
+
+//TEST:SIMPLE:-target dxil-assembly -entry main -stage compute
+
+void main()
+{
+ RAY_FLAG rayFlags = RAY_FLAG_CULL_FRONT_FACING_TRIANGLES | RAY_FLAG_CULL_NON_OPAQUE;
+ RayQuery<rayFlags> query;
+}
diff --git a/tests/bugs/ray-flags-non-constant.slang.expected b/tests/bugs/ray-flags-non-constant.slang.expected
new file mode 100644
index 000000000..3c5ef5d07
--- /dev/null
+++ b/tests/bugs/ray-flags-non-constant.slang.expected
@@ -0,0 +1,6 @@
+result code = -1
+standard error = {
+tests/bugs/ray-flags-non-constant.slang(11): error 39999: expression does not evaluate to a compile-time constant
+}
+standard output = {
+}