From 2f782d403ae5729b6c93fbe92577ee01f7a8d608 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Fri, 20 Apr 2018 16:56:33 -0700 Subject: Diagnose use of an implicit cast as an argument for an `out` parameter (#516) Work on #499 Two big fixes here: * The logic for checking constraints on `out` arguments wasn't actually triggering because it relied on function parameters being given an `OutType` if they are marked `out`, but the code wasn't actually doing that. Fixing the computation of types for functions resolved that issue. * Next, I added a specific diagnostic to follow up the "expected an l-value" error to let the user know that their argument was implicitly converted, and that is why it doesn't count as an l-value in Slang's rules. I've added a test case to ensure that we retain this diagnostic until we can do a true fix for the issue. The right long-term fix is to have an AST representation of all the implicit casts involved (e.g., in both directions for an `inout` parameter), and then have the IR generate explicit code for the conversions in each direction (the `LoweredVal` representation can handle this sort of thing). --- source/slang/check.cpp | 36 ++++++++++++++++++---- source/slang/diagnostic-defs.h | 2 ++ source/slang/syntax.cpp | 17 ++++++++-- tests/diagnostics/implicit-cast-lvalue.slang | 15 +++++++++ .../implicit-cast-lvalue.slang.expected | 7 +++++ 5 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 tests/diagnostics/implicit-cast-lvalue.slang create mode 100644 tests/diagnostics/implicit-cast-lvalue.slang.expected diff --git a/source/slang/check.cpp b/source/slang/check.cpp index 67b628596..acf473998 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -6738,13 +6738,37 @@ namespace Slang auto paramType = funcType->getParamType(pp); if (auto outParamType = paramType->As()) { - if (pp < expr->Arguments.Count() - && !expr->Arguments[pp]->type.IsLeftValue) + if( pp < expr->Arguments.Count() ) { - getSink()->diagnose( - expr->Arguments[pp], - Diagnostics::argumentExpectedLValue, - pp); + auto argExpr = expr->Arguments[pp]; + if( !argExpr->type.IsLeftValue ) + { + getSink()->diagnose( + argExpr, + Diagnostics::argumentExpectedLValue, + pp); + + if( auto implicitCastExpr = argExpr.As() ) + { + getSink()->diagnose( + argExpr, + Diagnostics::implicitCastUsedAsLValue, + implicitCastExpr->Arguments[0]->type, + implicitCastExpr->type); + } + } + } + else + { + // This implies that the function had an `out` + // or `inout` parameter and they gave it a default + // argument expression. I'm not even sure what + // that would mean. + // + // TODO: make sure this gets validated on the + // declaring side. + // + SLANG_DIAGNOSE_UNEXPECTED(getSink(), invoke, "default argument expression for out/inout paameter"); } } } diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h index f1aee496c..a3c51d67e 100644 --- a/source/slang/diagnostic-defs.h +++ b/source/slang/diagnostic-defs.h @@ -192,6 +192,8 @@ DIAGNOSTIC(30033, Error, invalidTypeForLocalVariable, "cannot declare a local va DIAGNOSTIC(30035, Error, componentOverloadTypeMismatch, "'$0': type of overloaded component mismatches previous definition.") DIAGNOSTIC(30041, Error, bitOperationNonIntegral, "bit operation: operand must be integral type.") DIAGNOSTIC(30047, Error, argumentExpectedLValue, "argument passed to parameter '$0' must be l-value.") +DIAGNOSTIC(30048, Note, implicitCastUsedAsLValue, "argument was implicitly cast from '$0' to '$1', and Slang does not support using an implicit cast as an l-value") + DIAGNOSTIC(30051, Error, invalidValueForArgument, "invalid value for argument '$0'") DIAGNOSTIC(30052, Error, invalidSwizzleExpr, "invalid swizzle pattern '$0' on type '$1'") DIAGNOSTIC(33070, Error, expectedFunction, "expression preceding parenthesis of apparent call must have function type.") diff --git a/source/slang/syntax.cpp b/source/slang/syntax.cpp index 9d29e7d21..5e855de29 100644 --- a/source/slang/syntax.cpp +++ b/source/slang/syntax.cpp @@ -2081,9 +2081,22 @@ void Type::accept(IValVisitor* visitor, void* extra) funcType->setSession(session); funcType->resultType = GetResultType(declRef); - for (auto pp : GetParameters(declRef)) + for (auto paramDeclRef : GetParameters(declRef)) { - funcType->paramTypes.Add(GetType(pp)); + auto paramDecl = paramDeclRef.getDecl(); + auto paramType = GetType(paramDeclRef); + if( paramDecl->FindModifier() ) + { + if(paramDecl->FindModifier() || paramDecl->FindModifier()) + { + paramType = session->getInOutType(paramType); + } + else + { + paramType = session->getOutType(paramType); + } + } + funcType->paramTypes.Add(paramType); } return funcType; diff --git a/tests/diagnostics/implicit-cast-lvalue.slang b/tests/diagnostics/implicit-cast-lvalue.slang new file mode 100644 index 000000000..4bf3c13b0 --- /dev/null +++ b/tests/diagnostics/implicit-cast-lvalue.slang @@ -0,0 +1,15 @@ +//TEST:SIMPLE: + +// Passing an argument for an `out` parameter such +// that implicit conversion would be required in +// both directions. + +void a(out uint x) +{ + x = 0; +} + +void b(int y) +{ + a(y); +} diff --git a/tests/diagnostics/implicit-cast-lvalue.slang.expected b/tests/diagnostics/implicit-cast-lvalue.slang.expected new file mode 100644 index 000000000..e5685fc51 --- /dev/null +++ b/tests/diagnostics/implicit-cast-lvalue.slang.expected @@ -0,0 +1,7 @@ +result code = -1 +standard error = { +tests/diagnostics/implicit-cast-lvalue.slang(14): error 30047: argument passed to parameter '0' must be l-value. +tests/diagnostics/implicit-cast-lvalue.slang(14): note 30048: argument was implicitly cast from 'int' to 'uint', and Slang does not support using an implicit cast as an l-value +} +standard output = { +} -- cgit v1.2.3