From b8617af2888db01f80efba9e0a103e6a61989c9c Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Thu, 24 Mar 2022 11:42:56 -0400 Subject: Fix for default initialization with generic field (#2168) * #include an absolute path didn't work - because paths were taken to always be relative. * Fix for = {} initialization with a field that is generic type parameter. * Handling for if a non type is passed to a generic parameter which requires a type. * Small comment improvements. Fix some tab issues. * This fixes the matrix.slang issue. Move the matrix.slang test into bugs as generic-default-matrix.slang --- source/slang/slang-check-type.cpp | 30 +++++++++++++++++++--- source/slang/slang-lower-to-ir.cpp | 8 +++--- tests/bugs/generic-default-matrix.slang | 20 +++++++++++++++ .../bugs/generic-default-matrix.slang.expected.txt | 4 +++ tests/bugs/generic-default-value.slang | 23 +++++++++++++++++ .../bugs/generic-default-value.slang.expected.txt | 4 +++ .../generic-invalid-type-specialization.slang | 20 +++++++++++++++ ...eric-invalid-type-specialization.slang.expected | 8 ++++++ tests/experiments/generic/matrix.slang | 25 ------------------ 9 files changed, 111 insertions(+), 31 deletions(-) create mode 100644 tests/bugs/generic-default-matrix.slang create mode 100644 tests/bugs/generic-default-matrix.slang.expected.txt create mode 100644 tests/bugs/generic-default-value.slang create mode 100644 tests/bugs/generic-default-value.slang.expected.txt create mode 100644 tests/diagnostics/generic-invalid-type-specialization.slang create mode 100644 tests/diagnostics/generic-invalid-type-specialization.slang.expected delete mode 100644 tests/experiments/generic/matrix.slang diff --git a/source/slang/slang-check-type.cpp b/source/slang/slang-check-type.cpp index 6c9b0c8ca..bd7b89e16 100644 --- a/source/slang/slang-check-type.cpp +++ b/source/slang/slang-check-type.cpp @@ -172,10 +172,11 @@ namespace Slang DiagnosticSink* diagSink) { Type* type = typeExp.type; - if(!type && typeExp.exp) - { - auto expr = typeExp.exp; + auto originalExpr = typeExp.exp; + auto expr = originalExpr; + if(!type && expr) + { expr = maybeResolveOverloadedExpr(expr, LookupMask::type, diagSink); if(auto typeType = as(expr->type)) @@ -186,6 +187,29 @@ namespace Slang if (!type) { + // Only output diagnostic if we have a sink. + if (diagSink) + { + // This function *can* be called with typeExp with both exp and type = nullptr. + // Previous behavior didn't output a diagnostic if originalExpr was null, so this keeps that behavior. + // + // Additional we check for ErrorType on expr, because if it's set a diagnostic has already been output via + // previous code or via maybeResolveOverloadedExpr. + if (originalExpr && (expr == nullptr || as(expr->type) == nullptr)) + { + // The diagnostic for expectedAType wants to say what it 'got'. + // The solution given here, currently is to just use the node name. + // How useful that might be could depend, and perhaps some other mechanism + // that catagorized 'what' the wrong thing was is. For now this seems sufficient. + // + // Note that use originalExpr (not expr) because we want original expr for diagnostic. + + // Get the AST node type info, so we can output a 'got' name + auto info = ASTClassInfo::getInfo(originalExpr->astNodeType); + diagSink->diagnose(originalExpr, Diagnostics::expectedAType, info->m_name); + } + } + if (outProperType) { *outProperType = nullptr; diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index b2a71a2e0..9ec8c1c71 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -3016,15 +3016,17 @@ struct ExprLoweringVisitorBase : ExprVisitor UNREACHABLE_RETURN(LoweredValInfo()); } - LoweredValInfo getDefaultVal(VarDeclBase* decl) + LoweredValInfo getDefaultVal(DeclRef decl) { - if(auto initExpr = decl->initExpr) + if(auto initExpr = decl.getDecl()->initExpr) { return lowerRValueExpr(context, initExpr); } else { - return getDefaultVal(decl->type); + Type* type = decl.substitute(getASTBuilder(), decl.getDecl()->type); + SLANG_ASSERT(type); + return getDefaultVal(type); } } diff --git a/tests/bugs/generic-default-matrix.slang b/tests/bugs/generic-default-matrix.slang new file mode 100644 index 000000000..47fbe520e --- /dev/null +++ b/tests/bugs/generic-default-matrix.slang @@ -0,0 +1,20 @@ +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -shaderobj + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer +RWStructuredBuffer outputBuffer; + +struct Another +{ + matrix values; +}; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + int index = dispatchThreadID.x; + + Another<2, 4> a = {}; + + outputBuffer[index] = index + a.values[0].x; +} + diff --git a/tests/bugs/generic-default-matrix.slang.expected.txt b/tests/bugs/generic-default-matrix.slang.expected.txt new file mode 100644 index 000000000..bc856dafa --- /dev/null +++ b/tests/bugs/generic-default-matrix.slang.expected.txt @@ -0,0 +1,4 @@ +0 +1 +2 +3 diff --git a/tests/bugs/generic-default-value.slang b/tests/bugs/generic-default-value.slang new file mode 100644 index 000000000..b3805e317 --- /dev/null +++ b/tests/bugs/generic-default-value.slang @@ -0,0 +1,23 @@ +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -shaderobj + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer +RWStructuredBuffer outputBuffer; + +/* Tests purpose is to confirm that use of `= {}` initialization +works with a generic */ + +struct Check +{ + T v; +}; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + int index = dispatchThreadID.x; + + Check v = {}; + + outputBuffer[index] = index + v.v; +} + diff --git a/tests/bugs/generic-default-value.slang.expected.txt b/tests/bugs/generic-default-value.slang.expected.txt new file mode 100644 index 000000000..bc856dafa --- /dev/null +++ b/tests/bugs/generic-default-value.slang.expected.txt @@ -0,0 +1,4 @@ +0 +1 +2 +3 diff --git a/tests/diagnostics/generic-invalid-type-specialization.slang b/tests/diagnostics/generic-invalid-type-specialization.slang new file mode 100644 index 000000000..28d155643 --- /dev/null +++ b/tests/diagnostics/generic-invalid-type-specialization.slang @@ -0,0 +1,20 @@ +//DIAGNOSTIC_TEST:SIMPLE: + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer +RWStructuredBuffer outputBuffer; + +struct Check +{ + T v; +}; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + int index = dispatchThreadID.x; + + // Invalid as should only accept a type + Check<2 + 2> v; + + outputBuffer[index] = index; +} diff --git a/tests/diagnostics/generic-invalid-type-specialization.slang.expected b/tests/diagnostics/generic-invalid-type-specialization.slang.expected new file mode 100644 index 000000000..b3c75c760 --- /dev/null +++ b/tests/diagnostics/generic-invalid-type-specialization.slang.expected @@ -0,0 +1,8 @@ +result code = -1 +standard error = { +tests/diagnostics/generic-invalid-type-specialization.slang(17): error 30060: expected a type, got a 'InfixExpr' + Check<2 + 2> v; + ^ +} +standard output = { +} diff --git a/tests/experiments/generic/matrix.slang b/tests/experiments/generic/matrix.slang deleted file mode 100644 index c55c7be33..000000000 --- a/tests/experiments/generic/matrix.slang +++ /dev/null @@ -1,25 +0,0 @@ -//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -shaderobj - -/* A test to use generics around resource/built in types. - -CRASHES the compiler. - */ - -//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer -RWStructuredBuffer outputBuffer; - -struct Another -{ - matrix values; -}; - -[numthreads(4, 1, 1)] -void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) -{ - int index = dispatchThreadID.x; - - Another<2, 4> a = {}; - - outputBuffer[index] = index; -} - -- cgit v1.2.3