From a305166ea427ba0618cde54ea03e647d73ab0044 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Fri, 2 Sep 2016 19:13:36 -0600 Subject: [PATCH] HLSL: Error if funcion with return type doesn't return a value. --- Test/baseResults/hlsl.attribute.frag.out | 4 +- Test/baseResults/hlsl.discard.frag.out | 4 +- Test/baseResults/hlsl.matType.frag.out | 62 ++++++++++++---------- Test/baseResults/hlsl.scope.frag.out | 4 +- Test/baseResults/hlsl.void.frag.out | 8 +-- Test/hlsl.attribute.frag | 2 +- Test/hlsl.discard.frag | 2 +- Test/hlsl.matType.frag | 1 + Test/hlsl.scope.frag | 2 +- Test/hlsl.void.frag | 3 +- glslang/Include/revision.h | 2 +- glslang/MachineIndependent/ParseHelper.cpp | 2 +- hlsl/hlslGrammar.cpp | 10 ++-- hlsl/hlslParseHelper.cpp | 17 +++++- hlsl/hlslParseHelper.h | 3 +- 15 files changed, 73 insertions(+), 53 deletions(-) diff --git a/Test/baseResults/hlsl.attribute.frag.out b/Test/baseResults/hlsl.attribute.frag.out index fd99cf511..efcc643fb 100755 --- a/Test/baseResults/hlsl.attribute.frag.out +++ b/Test/baseResults/hlsl.attribute.frag.out @@ -2,7 +2,7 @@ hlsl.attribute.frag Shader version: 450 gl_FragCoord origin is upper left 0:? Sequence -0:2 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:2 Function Definition: PixelShaderFunction(vf4; (global void) 0:2 Function Parameters: 0:2 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence @@ -20,7 +20,7 @@ Linked fragment stage: Shader version: 450 gl_FragCoord origin is upper left 0:? Sequence -0:2 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:2 Function Definition: PixelShaderFunction(vf4; (global void) 0:2 Function Parameters: 0:2 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence diff --git a/Test/baseResults/hlsl.discard.frag.out b/Test/baseResults/hlsl.discard.frag.out index 807799abd..463ae5bbc 100755 --- a/Test/baseResults/hlsl.discard.frag.out +++ b/Test/baseResults/hlsl.discard.frag.out @@ -14,7 +14,7 @@ gl_FragCoord origin is upper left 0:3 1.000000 0:3 true case 0:4 Branch: Kill -0:8 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:8 Function Definition: PixelShaderFunction(vf4; (global void) 0:8 Function Parameters: 0:8 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence @@ -60,7 +60,7 @@ gl_FragCoord origin is upper left 0:3 1.000000 0:3 true case 0:4 Branch: Kill -0:8 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:8 Function Definition: PixelShaderFunction(vf4; (global void) 0:8 Function Parameters: 0:8 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence diff --git a/Test/baseResults/hlsl.matType.frag.out b/Test/baseResults/hlsl.matType.frag.out index 2159a814e..20fdcc982 100755 --- a/Test/baseResults/hlsl.matType.frag.out +++ b/Test/baseResults/hlsl.matType.frag.out @@ -11,6 +11,9 @@ gl_FragCoord origin is upper left 0:9 Function Parameters: 0:9 'inFloat1' (in 1-component vector of float) 0:9 'inScalar' (in float) +0:? Sequence +0:10 Branch: Return with expression +0:10 'inFloat1' (in 1-component vector of float) 0:? Linker Objects 0:? 'f1' (global 1-component vector of float) 0:? 'fmat11' (global 1X1 matrix of float) @@ -35,6 +38,9 @@ gl_FragCoord origin is upper left 0:9 Function Parameters: 0:9 'inFloat1' (in 1-component vector of float) 0:9 'inScalar' (in float) +0:? Sequence +0:10 Branch: Return with expression +0:10 'inFloat1' (in 1-component vector of float) 0:? Linker Objects 0:? 'f1' (global 1-component vector of float) 0:? 'fmat11' (global 1X1 matrix of float) @@ -45,7 +51,7 @@ gl_FragCoord origin is upper left // Module Version 10000 // Generated by (magic number): 80001 -// Id's are bound by 38 +// Id's are bound by 40 Capability Shader Capability Float64 @@ -58,11 +64,11 @@ gl_FragCoord origin is upper left Name 9 "inFloat1" Name 10 "inScalar" Name 14 "f1" - Name 20 "fmat11" - Name 24 "fmat41" - Name 27 "fmat12" - Name 32 "dmat23" - Name 37 "int44" + Name 22 "fmat11" + Name 26 "fmat41" + Name 29 "fmat12" + Name 34 "dmat23" + Name 39 "int44" 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 @@ -71,27 +77,27 @@ gl_FragCoord origin is upper left 13: TypePointer Private 6(float) 14(f1): 13(ptr) Variable Private 15: 6(float) Constant 1065353216 - 17: TypeVector 6(float) 1 - 18: TypeMatrix 17(fvec) 1 - 19: TypePointer Private 18 - 20(fmat11): 19(ptr) Variable Private - 21: TypeVector 6(float) 4 - 22: TypeMatrix 21(fvec4) 1 - 23: TypePointer Private 22 - 24(fmat41): 23(ptr) Variable Private - 25: TypeMatrix 17(fvec) 2 - 26: TypePointer Private 25 - 27(fmat12): 26(ptr) Variable Private - 28: TypeFloat 64 - 29: TypeVector 28(float) 2 - 30: TypeMatrix 29(fvec2) 3 - 31: TypePointer Private 30 - 32(dmat23): 31(ptr) Variable Private - 33: TypeInt 32 1 - 34: TypeVector 33(int) 4 - 35: TypeMatrix 34(ivec4) 4 - 36: TypePointer Private 35 - 37(int44): 36(ptr) Variable Private + 19: TypeVector 6(float) 1 + 20: TypeMatrix 19(fvec) 1 + 21: TypePointer Private 20 + 22(fmat11): 21(ptr) Variable Private + 23: TypeVector 6(float) 4 + 24: TypeMatrix 23(fvec4) 1 + 25: TypePointer Private 24 + 26(fmat41): 25(ptr) Variable Private + 27: TypeMatrix 19(fvec) 2 + 28: TypePointer Private 27 + 29(fmat12): 28(ptr) Variable Private + 30: TypeFloat 64 + 31: TypeVector 30(float) 2 + 32: TypeMatrix 31(fvec2) 3 + 33: TypePointer Private 32 + 34(dmat23): 33(ptr) Variable Private + 35: TypeInt 32 1 + 36: TypeVector 35(int) 4 + 37: TypeMatrix 36(ivec4) 4 + 38: TypePointer Private 37 + 39(int44): 38(ptr) Variable Private 4(PixelShaderFunction): 2 Function None 3 5: Label Store 14(f1) 15 @@ -100,6 +106,6 @@ gl_FragCoord origin is upper left 9(inFloat1): 7(ptr) FunctionParameter 10(inScalar): 7(ptr) FunctionParameter 12: Label - 16: 6(float) Undef + 16: 6(float) Load 9(inFloat1) ReturnValue 16 FunctionEnd diff --git a/Test/baseResults/hlsl.scope.frag.out b/Test/baseResults/hlsl.scope.frag.out index 49dd577d7..2bc1dd523 100755 --- a/Test/baseResults/hlsl.scope.frag.out +++ b/Test/baseResults/hlsl.scope.frag.out @@ -2,7 +2,7 @@ hlsl.scope.frag Shader version: 450 gl_FragCoord origin is upper left 0:? Sequence -0:2 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:2 Function Definition: PixelShaderFunction(vf4; (global void) 0:2 Function Parameters: 0:2 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence @@ -46,7 +46,7 @@ Linked fragment stage: Shader version: 450 gl_FragCoord origin is upper left 0:? Sequence -0:2 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:2 Function Definition: PixelShaderFunction(vf4; (global void) 0:2 Function Parameters: 0:2 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence diff --git a/Test/baseResults/hlsl.void.frag.out b/Test/baseResults/hlsl.void.frag.out index f9e0eaf1a..40444c02f 100755 --- a/Test/baseResults/hlsl.void.frag.out +++ b/Test/baseResults/hlsl.void.frag.out @@ -6,12 +6,13 @@ gl_FragCoord origin is upper left 0:1 Function Parameters: 0:2 Function Definition: foo2( (global void) 0:2 Function Parameters: -0:5 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:5 Function Definition: PixelShaderFunction(vf4; (global void) 0:5 Function Parameters: 0:5 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence 0:6 Function Call: foo1( (global void) 0:7 Function Call: foo2( (global void) +0:8 Branch: Return 0:? Linker Objects @@ -25,17 +26,18 @@ gl_FragCoord origin is upper left 0:1 Function Parameters: 0:2 Function Definition: foo2( (global void) 0:2 Function Parameters: -0:5 Function Definition: PixelShaderFunction(vf4; (global 4-component vector of float) +0:5 Function Definition: PixelShaderFunction(vf4; (global void) 0:5 Function Parameters: 0:5 'input' (layout(location=0 ) in 4-component vector of float) 0:? Sequence 0:6 Function Call: foo1( (global void) 0:7 Function Call: foo2( (global void) +0:8 Branch: Return 0:? Linker Objects // Module Version 10000 // Generated by (magic number): 80001 -// Id's are bound by 12 +// Id's are bound by 13 Capability Shader 1: ExtInstImport "GLSL.std.450" diff --git a/Test/hlsl.attribute.frag b/Test/hlsl.attribute.frag index 25c72d46e..9ff8541c1 100644 --- a/Test/hlsl.attribute.frag +++ b/Test/hlsl.attribute.frag @@ -1,4 +1,4 @@ -float4 PixelShaderFunction(float4 input) : COLOR0 +void PixelShaderFunction(float4 input) : COLOR0 { [unroll]; []; diff --git a/Test/hlsl.discard.frag b/Test/hlsl.discard.frag index e8b526794..7d9271c9c 100644 --- a/Test/hlsl.discard.frag +++ b/Test/hlsl.discard.frag @@ -4,7 +4,7 @@ void foo(float f) discard; } -float4 PixelShaderFunction(float4 input) : COLOR0 +void PixelShaderFunction(float4 input) : COLOR0 { foo(input.z); if (input.x) diff --git a/Test/hlsl.matType.frag b/Test/hlsl.matType.frag index 36d71e3ba..ee7c04763 100644 --- a/Test/hlsl.matType.frag +++ b/Test/hlsl.matType.frag @@ -7,4 +7,5 @@ int4x4 int44; float1 ShaderFunction(float1 inFloat1, float inScalar) : COLOR0 { + return inFloat1; } diff --git a/Test/hlsl.scope.frag b/Test/hlsl.scope.frag index 0d8cc1ad9..a5309ca2a 100644 --- a/Test/hlsl.scope.frag +++ b/Test/hlsl.scope.frag @@ -1,4 +1,4 @@ -float4 PixelShaderFunction(float4 input) : COLOR0 +void PixelShaderFunction(float4 input) : COLOR0 { int x; x; diff --git a/Test/hlsl.void.frag b/Test/hlsl.void.frag index 9bf06b726..950bbd752 100644 --- a/Test/hlsl.void.frag +++ b/Test/hlsl.void.frag @@ -1,8 +1,9 @@ void foo1() {} void foo2(void) {} -float4 PixelShaderFunction(float4 input) : COLOR0 +void PixelShaderFunction(float4 input) : COLOR0 { foo1(); foo2(); + return; } \ No newline at end of file diff --git a/glslang/Include/revision.h b/glslang/Include/revision.h index eac03933e..864613e93 100644 --- a/glslang/Include/revision.h +++ b/glslang/Include/revision.h @@ -2,5 +2,5 @@ // For the version, it uses the latest git tag followed by the number of commits. // For the date, it uses the current date (when then script is run). -#define GLSLANG_REVISION "Overload400-PrecQual.1460" +#define GLSLANG_REVISION "Overload400-PrecQual.1461" #define GLSLANG_DATE "02-Sep-2016" diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 24f106a45..efb1baffb 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -1036,7 +1036,7 @@ TIntermAggregate* TParseContext::handleFunctionDefinition(const TSourceLoc& loc, error(loc, "can't find function", function.getName().c_str(), ""); // Note: 'prevDec' could be 'function' if this is the first time we've seen function // as it would have just been put in the symbol table. Otherwise, we're looking up - // an earlier occurance. + // an earlier occurrence. if (prevDec && prevDec->isDefined()) { // Then this function already has a body. diff --git a/hlsl/hlslGrammar.cpp b/hlsl/hlslGrammar.cpp index 192e28ae6..e548e5d29 100755 --- a/hlsl/hlslGrammar.cpp +++ b/hlsl/hlslGrammar.cpp @@ -1496,20 +1496,16 @@ bool HlslGrammar::acceptParameterDeclaration(TFunction& function) // parsing the body (compound_statement). bool HlslGrammar::acceptFunctionDefinition(TFunction& function, TIntermNode*& node) { - TFunction* functionDeclarator = parseContext.handleFunctionDeclarator(token.loc, function, false /* not prototype */); + TFunction& functionDeclarator = parseContext.handleFunctionDeclarator(token.loc, function, false /* not prototype */); TSourceLoc loc = token.loc; // This does a pushScope() - node = parseContext.handleFunctionDefinition(loc, *functionDeclarator); + node = parseContext.handleFunctionDefinition(loc, functionDeclarator); // compound_statement TIntermNode* functionBody = nullptr; if (acceptCompoundStatement(functionBody)) { - node = intermediate.growAggregate(node, functionBody); - intermediate.setAggregateOperator(node, EOpFunction, functionDeclarator->getType(), loc); - node->getAsAggregate()->setName(functionDeclarator->getMangledName().c_str()); - parseContext.popScope(); - + parseContext.handleFunctionBody(loc, functionDeclarator, functionBody, node); return true; } diff --git a/hlsl/hlslParseHelper.cpp b/hlsl/hlslParseHelper.cpp index a214df96b..96270c894 100755 --- a/hlsl/hlslParseHelper.cpp +++ b/hlsl/hlslParseHelper.cpp @@ -686,7 +686,7 @@ TIntermTyped* HlslParseContext::handleDotDereference(const TSourceLoc& loc, TInt // Handle seeing a function declarator in the grammar. This is the precursor // to recognizing a function prototype or function definition. // -TFunction* HlslParseContext::handleFunctionDeclarator(const TSourceLoc& loc, TFunction& function, bool prototype) +TFunction& HlslParseContext::handleFunctionDeclarator(const TSourceLoc& loc, TFunction& function, bool prototype) { // // Multiple declarations of the same function name are allowed. @@ -720,7 +720,7 @@ TFunction* HlslParseContext::handleFunctionDeclarator(const TSourceLoc& loc, TFu // in which case, we need to use the parameter names from this one, and not the one that's // being redeclared. So, pass back this declaration, not the one in the symbol table. // - return &function; + return function; } // @@ -798,6 +798,18 @@ TIntermAggregate* HlslParseContext::handleFunctionDefinition(const TSourceLoc& l return paramNodes; } +void HlslParseContext::handleFunctionBody(const TSourceLoc& loc, TFunction& function, TIntermNode* functionBody, TIntermNode*& node) +{ + node = intermediate.growAggregate(node, functionBody); + intermediate.setAggregateOperator(node, EOpFunction, function.getType(), loc); + node->getAsAggregate()->setName(function.getMangledName().c_str()); + + popScope(); + + if (function.getType().getBasicType() != EbtVoid && ! functionReturnsValue) + error(loc, "function does not return a value:", "", function.getName().c_str()); +} + // AST I/O is done through shader globals declared in the 'in' or 'out' // storage class. An HLSL entry point has a return value, input parameters // and output parameters. These need to get remapped to the AST I/O. @@ -839,6 +851,7 @@ void HlslParseContext::remapEntrypointIO(TFunction& function) // if necessary. TIntermNode* HlslParseContext::handleReturnValue(const TSourceLoc& loc, TIntermTyped* value) { + functionReturnsValue = true; TIntermTyped* converted = value; if (currentFunctionType->getBasicType() == EbtVoid) { diff --git a/hlsl/hlslParseHelper.h b/hlsl/hlslParseHelper.h index 2f5ee5704..77d8157ac 100755 --- a/hlsl/hlslParseHelper.h +++ b/hlsl/hlslParseHelper.h @@ -83,8 +83,9 @@ public: TIntermTyped* handleBinaryMath(const TSourceLoc&, const char* str, TOperator op, TIntermTyped* left, TIntermTyped* right); TIntermTyped* handleUnaryMath(const TSourceLoc&, const char* str, TOperator op, TIntermTyped* childNode); TIntermTyped* handleDotDereference(const TSourceLoc&, TIntermTyped* base, const TString& field); - TFunction* handleFunctionDeclarator(const TSourceLoc&, TFunction& function, bool prototype); + TFunction& handleFunctionDeclarator(const TSourceLoc&, TFunction& function, bool prototype); TIntermAggregate* handleFunctionDefinition(const TSourceLoc&, TFunction&); + void handleFunctionBody(const TSourceLoc&, TFunction&, TIntermNode* functionBody, TIntermNode*& node); void remapEntrypointIO(TFunction& function); TIntermNode* handleReturnValue(const TSourceLoc&, TIntermTyped*); void handleFunctionArgument(TFunction*, TIntermTyped*& arguments, TIntermTyped* newArg);