Fix constructor parsing issues
After this patch, ANGLE no longer accepts constructors where any of
multiple parameters is sampler or void.
Also, structure array constructors with just one parameter are now
accepted.
Error message for a constructor with no parameters is also more
informative than before.
BUG=angleproject:1193
TEST=angle_unittests
Change-Id: I6b897973448cf500096f612b3b95dcc23aebc716
Reviewed-on: https://chromium-review.googlesource.com/311590
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tryjob-Request: Olli Etuaho <oetuaho@nvidia.com>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index d537bcb..5cae37c 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -493,7 +493,7 @@
// Returns true if there was an error in construction.
//
bool TParseContext::constructorErrorCheck(const TSourceLoc &line,
- TIntermNode *node,
+ TIntermNode *argumentsNode,
TFunction &function,
TOperator op,
TType *type)
@@ -604,21 +604,27 @@
}
}
- TIntermTyped *typed = node ? node->getAsTyped() : 0;
- if (typed == 0)
+ if (argumentsNode == nullptr)
{
- error(line, "constructor argument does not have a type", "constructor");
+ error(line, "constructor does not have any arguments", "constructor");
return true;
}
- if (op != EOpConstructStruct && IsSampler(typed->getBasicType()))
+
+ TIntermAggregate *argumentsAgg = argumentsNode->getAsAggregate();
+ for (TIntermNode *&argNode : *argumentsAgg->getSequence())
{
- error(line, "cannot convert a sampler", "constructor");
- return true;
- }
- if (typed->getBasicType() == EbtVoid)
- {
- error(line, "cannot convert a void", "constructor");
- return true;
+ TIntermTyped *argTyped = argNode->getAsTyped();
+ ASSERT(argTyped != nullptr);
+ if (op != EOpConstructStruct && IsSampler(argTyped->getBasicType()))
+ {
+ error(line, "cannot convert a sampler", "constructor");
+ return true;
+ }
+ if (argTyped->getBasicType() == EbtVoid)
+ {
+ error(line, "cannot convert a void", "constructor");
+ return true;
+ }
}
return false;
@@ -2248,19 +2254,14 @@
TFunction *fnCall,
const TSourceLoc &line)
{
- TIntermAggregate *aggregateArguments = arguments->getAsAggregate();
-
- if (!aggregateArguments)
- {
- aggregateArguments = new TIntermAggregate;
- aggregateArguments->getSequence()->push_back(arguments);
- }
+ TIntermAggregate *constructor = arguments->getAsAggregate();
+ ASSERT(constructor != nullptr);
if (type->isArray())
{
// GLSL ES 3.00 section 5.4.4: Each argument must be the same type as the element type of
// the array.
- TIntermSequence *args = aggregateArguments->getSequence();
+ TIntermSequence *args = constructor->getSequence();
for (size_t i = 0; i < args->size(); i++)
{
const TType &argType = (*args)[i]->getAsTyped()->getType();
@@ -2277,7 +2278,7 @@
else if (op == EOpConstructStruct)
{
const TFieldList &fields = type->getStruct()->fields();
- TIntermSequence *args = aggregateArguments->getSequence();
+ TIntermSequence *args = constructor->getSequence();
for (size_t i = 0; i < fields.size(); i++)
{
@@ -2293,7 +2294,8 @@
}
// Turn the argument list itself into a constructor
- TIntermAggregate *constructor = intermediate.setAggregateOperator(aggregateArguments, op, line);
+ constructor->setOp(op);
+ constructor->setLine(line);
ASSERT(constructor->isConstructor());
// Need to set type before setPrecisionFromChildren() because bool doesn't have precision.
@@ -3836,6 +3838,8 @@
//
// Treat it like a built-in unary operator.
//
+ TIntermAggregate *paramAgg = paramNode->getAsAggregate();
+ paramNode = paramAgg->getSequence()->front();
callNode = createUnaryMath(op, paramNode->getAsTyped(), loc,
&fnCandidate->getReturnType());
if (callNode == nullptr)
@@ -3881,7 +3885,6 @@
else
{
// This is a real function call
-
TIntermAggregate *aggregate =
intermediate.setAggregateOperator(paramNode, EOpFunctionCall, loc);
aggregate->setType(fnCandidate->getReturnType());
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 1136bcc..1b86a6a 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -137,7 +137,11 @@
bool constErrorCheck(TIntermTyped *node);
bool integerErrorCheck(TIntermTyped *node, const char *token);
bool globalErrorCheck(const TSourceLoc &line, bool global, const char *token);
- bool constructorErrorCheck(const TSourceLoc &line, TIntermNode*, TFunction&, TOperator, TType*);
+ bool constructorErrorCheck(const TSourceLoc &line,
+ TIntermNode *argumentsNode,
+ TFunction &function,
+ TOperator op,
+ TType *type);
bool arraySizeErrorCheck(const TSourceLoc &line, TIntermTyped *expr, int &size);
bool arrayQualifierErrorCheck(const TSourceLoc &line, const TPublicType &type);
bool arrayTypeErrorCheck(const TSourceLoc &line, const TPublicType &type);
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index db0c53f..13797bd 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -328,7 +328,7 @@
const TType *type = new TType($2->getType());
$1->addParameter(TConstParameter(type));
$$.function = $1;
- $$.nodePair.node1 = $2;
+ $$.nodePair.node1 = context->intermediate.makeAggregate($2, @2);
}
| function_call_header_with_parameters COMMA assignment_expression {
const TType *type = new TType($3->getType());
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index aa37b05..3fdd8b8 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -2550,7 +2550,7 @@
const TType *type = new TType((yyvsp[0].interm.intermTypedNode)->getType());
(yyvsp[-1].interm.function)->addParameter(TConstParameter(type));
(yyval.interm).function = (yyvsp[-1].interm.function);
- (yyval.interm).nodePair.node1 = (yyvsp[0].interm.intermTypedNode);
+ (yyval.interm).nodePair.node1 = context->intermediate.makeAggregate((yyvsp[0].interm.intermTypedNode), (yylsp[0]));
}
break;
diff --git a/src/tests/compiler_tests/MalformedShader_test.cpp b/src/tests/compiler_tests/MalformedShader_test.cpp
index 24ab1c8..a71c5be 100644
--- a/src/tests/compiler_tests/MalformedShader_test.cpp
+++ b/src/tests/compiler_tests/MalformedShader_test.cpp
@@ -1085,3 +1085,76 @@
FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
}
}
+
+// Test that a sampler can't be used in constructor argument list
+TEST_F(MalformedShaderTest, SamplerInConstructorArguments)
+{
+ const std::string &shaderString =
+ "precision mediump float;\n"
+ "uniform sampler2D s;\n"
+ "void main()\n"
+ "{\n"
+ " vec2 v = vec2(0.0, s);\n"
+ " gl_FragColor = vec4(v, 0.0, 0.0);\n"
+ "}\n";
+ if (compile(shaderString))
+ {
+ FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+ }
+}
+
+// Test that void can't be used in constructor argument list
+TEST_F(MalformedShaderTest, VoidInConstructorArguments)
+{
+ const std::string &shaderString =
+ "precision mediump float;\n"
+ "void foo() {}\n"
+ "void main()\n"
+ "{\n"
+ " vec2 v = vec2(0.0, foo());\n"
+ " gl_FragColor = vec4(v, 0.0, 0.0);\n"
+ "}\n";
+ if (compile(shaderString))
+ {
+ FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+ }
+}
+
+// Test that a shader passing a struct into a constructor of array of structs with 1 element works.
+TEST_F(MalformedShaderTest, SingleStructArrayConstructor)
+{
+ const std::string &shaderString =
+ "#version 300 es\n"
+ "precision mediump float;\n"
+ "out vec4 my_FragColor;\n"
+ "uniform float u;\n"
+ "struct S { float member; };\n"
+ "void main()\n"
+ "{\n"
+ " S[1] sarr = S[1](S(u));\n"
+ " my_FragColor = vec4(sarr[0].member);\n"
+ "}\n";
+ if (!compile(shaderString))
+ {
+ FAIL() << "Shader compilation failed, expecting success " << mInfoLog;
+ }
+}
+
+// Test that a shader with empty constructor parameter list is not accepted.
+TEST_F(MalformedShaderTest, EmptyArrayConstructor)
+{
+ const std::string &shaderString =
+ "#version 300 es\n"
+ "precision mediump float;\n"
+ "out vec4 my_FragColor;\n"
+ "uniform float u;\n"
+ "const float[] f = f[]();\n"
+ "void main()\n"
+ "{\n"
+ " my_FragColor = vec4(0.0);\n"
+ "}\n";
+ if (compile(shaderString))
+ {
+ FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+ }
+}