summaryrefslogtreecommitdiffstats
path: root/source
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2020-06-15 12:05:04 -0700
committerGitHub <noreply@github.com>2020-06-15 12:05:04 -0700
commitd84cfb766b089c03f9545588a8590efea1749770 (patch)
tree3bc00b9a85e52709f682f1559a3b354a5bc77735 /source
parent90444f8366255f274993ce4699738d9ab7cf4ee1 (diff)
Remove implicit conversions to `void` (#1388)
* Remove implicit conversions to `void` Fixes #1372 The standard library code had accidentally introduced implicit-conversion `__init` operations on the `void` type that accepted each of the other basic types, so that a function written like: ```hlsl void bad() { return 1; } ``` would translate to: ```hlsl void bad() { return (void)1; } ``` The dual problesm are that the input code should have produced a diagnostic of some kind, and the output code doesn't appear to compile correctly through fxc. This change introduces several fixes aimed at this issue: * First, the problem in the stdlib code is plugged: we don't introduce implicit conversion operations *to* or *from* `void` (we'd only been banning it in one direction before) * Next, an explicit `__init` was added to `void` that accepts *any* type so that existing HLSL code that might do `(void) someExpression` to ignore a result will continue to work. This is a compatibility feature, and it might be argued that we should at least warn when it is used. Note that this function is expected to never appear in output HLSL/GLSL because its result will never be used, and it is marked `[__readNone]` allowing calls to it to be eliminated as dead code. * During IR lowering, we now take care to only emit the `IRReturnVal` instruction type if there is a non-`void` value being returned, and use `IRReturnVoid` for both the case where no expression was used in the `return` statement *and* the case where an expression of type `void` is returned. * A test case was added to confirm that returning `1` from a `void` function isn't allowed, while returning `(void) 1` *is*. The net result of these changes is that we now produce an error for the bad input code, we allow explicit casts to `void` as a compatibility feature, and we are more robust about treating `void` as if it is an ordinary type in the front-end. * fixup: missing file
Diffstat (limited to 'source')
-rw-r--r--source/slang/core.meta.slang19
-rw-r--r--source/slang/slang-lower-to-ir.cpp39
2 files changed, 52 insertions, 6 deletions
diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 0c39f2c2f..b56ca3085 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -192,7 +192,9 @@ ${{{{
// Declare initializers to convert from various other types
for (int ss = 0; ss < kBaseTypeCount; ++ss)
{
- // Don't allow conversion from `void`
+ // Don't allow conversion to or from `void`
+ if (kBaseTypes[tt].tag == BaseType::Void)
+ continue;
if (kBaseTypes[ss].tag == BaseType::Void)
continue;
@@ -250,6 +252,21 @@ ${{{{
${{{{
break;
}
+
+ // If this is the `void` type, then we want to allow
+ // explicit conversion to it from any other type, using
+ // `(void) someExpression`.
+ //
+ if( kBaseTypes[tt].tag == BaseType::Void )
+ {
+}}}}
+ __generic<T>
+ [__readNone]
+ __init(T value)
+ {}
+${{{{
+ }
+
}}}}
}
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index 42f53ef22..fa23b3307 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -3610,18 +3610,47 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
{
startBlockIfNeeded(stmt);
- // A `return` statement turns into a return
- // instruction. If the statement had an argument
- // expression, then we need to lower that to
- // a value first, and then emit the resulting value.
+ // A `return` statement turns into a `return` instruction,
+ // but we have two kinds of `return`: one for returning
+ // a (non-`void`) value, and one for returning "no value"
+ // (which effectively returns a value of type `void`).
+ //
if( auto expr = stmt->expression )
{
+ // If the AST `return` statement had an expression, then we
+ // need to lower it to the IR at this point, both to
+ // compute its value and (in case we are returning a
+ // `void`-typed expression) to execute its side effects.
+ //
auto loweredExpr = lowerRValueExpr(context, expr);
- getBuilder()->emitReturn(getSimpleVal(context, loweredExpr));
+ // If the AST `return` statement was returning a non-`void`
+ // value, then we need to emit an IR `return` of that value.
+ //
+ if(!expr->type.type->equals(context->astBuilder->getVoidType()))
+ {
+ getBuilder()->emitReturn(getSimpleVal(context, loweredExpr));
+ }
+ else
+ {
+ // If the type of the value returned was `void`, then
+ // we don't want to emit an IR-level `return` with a value,
+ // because that could trip up some of our back-end.
+ //
+ // TODO: We should eventually have only a single IR-level
+ // `return` operation that always takes a value (including
+ // values of type `void`), and then treat an AST `return;`
+ // as equivalent to something like `return void();`.
+ //
+ getBuilder()->emitReturn();
+ }
}
else
{
+ // If we hit this case, then the AST `return` was a `return;`
+ // with no value, which can only occur in a function with
+ // a `void` result type.
+ //
getBuilder()->emitReturn();
}
}