From e369bfccfeaa7e86b3fe44186ff42af5e71535a3 Mon Sep 17 00:00:00 2001
From: John Kessenich <cepheus@frii.com>
Date: Wed, 26 Jun 2013 05:54:40 +0000
Subject: [PATCH] Semantic checks for .length(), switch/case/default, and
multidimensional arrays.
git-svn-id: https://cvs.khronos.org/svn/repos/ogl/trunk/ecosystem/public/sdk/tools/glslang@22175 e7fa87d3-cd2b-0410-9028-fcbf551c1848
---
Test/120.vert | 30 ++++++++---
Test/300.frag | 6 +++
Test/switch.frag | 42 +++++++++++++---
glslang/MachineIndependent/ParseHelper.cpp | 50 +++++++++++++++++--
glslang/MachineIndependent/ParseHelper.h | 5 +-
glslang/MachineIndependent/ShaderLang.cpp | 2 +-
glslang/MachineIndependent/glslang.l | 37 +-------------
glslang/MachineIndependent/glslang.y | 41 ++++++++++-----
glslang/MachineIndependent/preprocessor/cpp.c | 2 -
9 files changed, 144 insertions(+), 71 deletions(-)
diff --git a/Test/120.vert b/Test/120.vert
index d0f64e402..be02d66b1 100644
--- a/Test/120.vert
+++ b/Test/120.vert
@@ -1,15 +1,15 @@
#version 120
-in vec4 i;
-out vec4 o;
+in vec4 i; // ERROR
+out vec4 o; // ERROR
attribute vec2 attv2;
attribute vec4 attv4;
uniform sampler2D s2D;
invariant varying vec2 centTexCoord;
invariant gl_Position;
-centroid gl_Position;
-centroid centroid foo;
+centroid gl_Position; // ERROR
+centroid centroid foo; // ERROR
invariant gl_Position, gl_PointSize;
void main()
@@ -22,11 +22,29 @@ void main()
gl_Position = b[b.length()-1];
float f[];
- int a = f.length();
+ int a1 = f.length(); // ERROR
+ float f[7];
+ int aa = f.length();
+ int a2 = f.length; // ERROR
+ int a3 = f.length(a); // ERROR
+ int a4 = f.flizbit; // ERROR
+ int a4 = f.flizbit(); // ERROR
+ float md[2][4]; // ERROR
+ float[2] md2[4]; // ERROR
+ float[2][4] md3; // ERROR
+ float md5, md6[2][3]; // ERROR
+ float[2] md4, md7[4]; // ERROR
+ float md9[2][3] = float[2][3](1, 2, 3, 4, 5, 6); // ERROR
+ float md10, md11[2][3] = float[2][3](1, 2, 3, 4, 5, 6); // ERROR
gl_PointSize = 3.8;
}
-uniform float initted = 3.4;
+uniform float initted = 3.4; // okay
const float concall = sin(0.3);
+
+int[2][3] foo( // ERROR
+ float[2][3] a, // ERROR
+ float[2] b[3], // ERROR
+ float c[2][3]); // ERROR
diff --git a/Test/300.frag b/Test/300.frag
index 8a185ba48..88539cb59 100644
--- a/Test/300.frag
+++ b/Test/300.frag
@@ -97,5 +97,11 @@ void main()
atanh(c3D);
}
+uniform multi {
+ int[2] a[3]; // ERROR
+ int[2][3] b; // ERROR
+ int c[2][3]; // ERROR
+} multiInst[2][3]; // ERROR
+
float imageBuffer; // ERROR, reserved
float uimage2DRect; // ERROR, reserved
diff --git a/Test/switch.frag b/Test/switch.frag
index d83eac63e..6924b3e91 100644
--- a/Test/switch.frag
+++ b/Test/switch.frag
@@ -8,41 +8,67 @@ void main()
float f;
int a[2];
- switch(f) { // ERROR
+ switch(f) { // ERROR
}
- switch(a) { // ERROR
+ switch(a) { // ERROR
}
switch(c)
{
}
- switch(c)
+ switch(c) // ERROR, not enough stuff after last label
{
- case 2: // ERROR, not enough stuff
+ case 2:
}
switch(c)
{
- f = sin(x); // ERRROR
- case 2: // ERROR, not enough stuff
+ f = sin(x); // ERRROR
+ case 2:
f = cos(x);
break;
}
switch (c) {
+ default:
+ break;
case 1:
f = sin(x);
break;
case 2:
f = cos(x);
break;
- default:
+ default: // ERROR, 2nd default
f = tan(x);
}
switch (c) {
+ case 1:
+ f = sin(x);
+ break;
+ case 2:
+ switch (d) {
+ case 1:
+ f = x * x * x;
+ break;
+ case 2:
+ f = x * x;
+ break;
+ }
+ break;
+ default:
+ f = tan(x);
+ case 1: // ERROR, 2nd 'case 1'
+ break;
+ case 3.8: // ERROR, non-int
+ break;
+ case c: // ERROR, non-constant
+ break;
+ }
+
+ switch (c) { // a no-error normal switch
case 1:
f = sin(x);
break;
@@ -60,5 +86,5 @@ void main()
f = tan(x);
}
- break; // ERROR
+ break; // ERROR
}
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
index 3a3b31af1..51696198e 100644
--- a/glslang/MachineIndependent/ParseHelper.cpp
+++ b/glslang/MachineIndependent/ParseHelper.cpp
@@ -461,10 +461,10 @@ bool TParseContext::lValueErrorCheck(int line, const char* op, TIntermTyped* nod
// Both test, and if necessary spit out an error, to see if the node is really
// a constant.
//
-void TParseContext::constCheck(TIntermTyped* node)
+void TParseContext::constCheck(TIntermTyped* node, const char* token)
{
if (node->getQualifier().storage != EvqConst)
- error(node->getLine(), "constant expression required", "", "");
+ error(node->getLine(), "constant expression required", token, "");
}
//
@@ -996,6 +996,28 @@ void TParseContext::arraySizeRequiredCheck(int line, int& size)
}
}
+void TParseContext::arrayDimError(int line)
+{
+ requireProfile(line, (EProfileMask)(ECoreProfileMask | ECompatibilityProfileMask), "arrays of arrays");
+ profileRequires(line, ECoreProfile, 430, 0, "arrays of arrays");
+ profileRequires(line, ECompatibilityProfile, 430, 0, "arrays of arrays");
+}
+
+void TParseContext::arrayDimCheck(int line, TArraySizes sizes1, TArraySizes sizes2)
+{
+ if (sizes1 && sizes2 ||
+ sizes1 && sizes1->size() > 1 ||
+ sizes2 && sizes2->size() > 1)
+ arrayDimError(line);
+}
+
+void TParseContext::arrayDimCheck(int line, const TType* type, TArraySizes sizes2)
+{
+ if (type && type->isArray() && sizes2 ||
+ sizes2 && sizes2->size() > 1)
+ arrayDimError(line);
+}
+
//
// Do all the semantic checking for declaring an array, with and
// without a size, and make the right changes to the symbol table.
@@ -1582,6 +1604,8 @@ void TParseContext::addBlock(int line, TTypeList& typeList, const TString* insta
return;
}
+ arrayDimCheck(line, arraySizes, 0);
+
// check for qualifiers and types that don't belong within a block
for (unsigned int member = 0; member < typeList.size(); ++member) {
TQualifier memberQualifier = typeList[member].type->getQualifier();
@@ -1774,10 +1798,26 @@ void TParseContext::wrapupSwitchSubsequence(TIntermAggregate* statements, TInter
statements->setOperator(EOpSequence);
switchSequence->push_back(statements);
}
- if (branchNode)
+ if (branchNode) {
+ // check all previous cases for the same label (or both are 'default')
+ for (unsigned int s = 0; s < switchSequence->size(); ++s) {
+ TIntermBranch* prevBranch = (*switchSequence)[s]->getAsBranchNode();
+ if (prevBranch) {
+ TIntermTyped* prevExpression = prevBranch->getExpression();
+ TIntermTyped* newExpression = branchNode->getAsBranchNode()->getExpression();
+ if (prevExpression == 0 && newExpression == 0)
+ error(branchNode->getLine(), "duplicate label", "default", "");
+ else if (prevExpression != 0 &&
+ newExpression != 0 &&
+ prevExpression->getAsConstantUnion() &&
+ newExpression->getAsConstantUnion() &&
+ prevExpression->getAsConstantUnion()->getUnionArrayPointer()->getIConst() ==
+ newExpression->getAsConstantUnion()->getUnionArrayPointer()->getIConst())
+ error(branchNode->getLine(), "duplicated value", "case", "");
+ }
+ }
switchSequence->push_back(branchNode);
-
- // TODO: semantics: verify no duplicated case values
+ }
}
//
diff --git a/glslang/MachineIndependent/ParseHelper.h b/glslang/MachineIndependent/ParseHelper.h
index efaa6c6ae..bdb866d53 100644
--- a/glslang/MachineIndependent/ParseHelper.h
+++ b/glslang/MachineIndependent/ParseHelper.h
@@ -113,13 +113,16 @@ struct TParseContext {
void binaryOpError(int line, const char* op, TString left, TString right);
void variableCheck(TIntermTyped*& nodePtr);
bool lValueErrorCheck(int line, const char* op, TIntermTyped*);
- void constCheck(TIntermTyped* node);
+ void constCheck(TIntermTyped* node, const char* token);
void integerCheck(TIntermTyped* node, const char* token);
void globalCheck(int line, bool global, const char* token);
bool constructorError(int line, TIntermNode*, TFunction&, TOperator, TType&);
void arraySizeCheck(int line, TIntermTyped* expr, int& size);
bool arrayQualifierError(int line, const TPublicType&);
void arraySizeRequiredCheck(int line, int& size);
+ void arrayDimError(int line);
+ void arrayDimCheck(int line, TArraySizes sizes1, TArraySizes sizes2);
+ void arrayDimCheck(int line, const TType*, TArraySizes);
void arrayCheck(int line, TString& identifier, const TPublicType&, TVariable*& variable);
bool insertBuiltInArrayAtGlobalLevel();
bool voidErrorCheck(int, const TString&, const TPublicType&);
diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp
index 0c1bd9a35..bf5a00f2e 100644
--- a/glslang/MachineIndependent/ShaderLang.cpp
+++ b/glslang/MachineIndependent/ShaderLang.cpp
@@ -82,7 +82,7 @@ const int VersionCount = 12;
// Each has a different set of built-ins, and we want to preserve that from
// compile to compile.
//
-// TODO: quality: thread safety: ensure the built-in symbol table levels are reado only.
+// TODO: quality: thread safety: ensure the built-in symbol table levels are read only.
TSymbolTable* SharedSymbolTables[VersionCount][EProfileCount][EShLangCount] = {};
TPoolAllocator* PerProcessGPA = 0;
diff --git a/glslang/MachineIndependent/glslang.l b/glslang/MachineIndependent/glslang.l
index 14a7ab934..0845f8ae0 100644
--- a/glslang/MachineIndependent/glslang.l
+++ b/glslang/MachineIndependent/glslang.l
@@ -554,8 +554,6 @@ int yy_input(char* buf, int max_size);
{D}+"."{D}*({E})?{LF} { pyylval->lex.line = yylineno; pyylval->lex.d = atof(yytext); return DOUBLECONSTANT; }
"."{D}+({E})?{LF} { pyylval->lex.line = yylineno; pyylval->lex.d = atof(yytext); return DOUBLECONSTANT; }
-"/*" { int ret = PaParseComment(pyylval->lex.line, parseContext); if (!ret) return ret; }
-
"+=" { pyylval->lex.line = yylineno; return ADD_ASSIGN; }
"-=" { pyylval->lex.line = yylineno; return SUB_ASSIGN; }
"*=" { pyylval->lex.line = yylineno; return MUL_ASSIGN; }
@@ -667,7 +665,7 @@ int PaParseStrings(char* argv[], int strLen[], int argc, TParseContext& parseCon
char *writeablePreamble = 0;
if (preamble) {
// preAmble could be a hard-coded string; make writable copy
- // TODO: CPP: make it not need writable strings
+ // TODO: efficiency PP: make it not need writable strings
int size = strlen(preamble) + 1;
writeablePreamble = new char[size];
memcpy(writeablePreamble, preamble, size);
@@ -689,7 +687,7 @@ int PaParseStrings(char* argv[], int strLen[], int argc, TParseContext& parseCon
cpp->notAVersionToken = 0;
yylineno = 1;
- // TODO: CPP: a shader containing nothing but white space and comments is valid, even though it has no parse tokens
+ // TODO: desktop PP: a shader containing nothing but white space and comments is valid, even though it has no parse tokens
int len = 0;
while (argv[0][len] == ' ' ||
argv[0][len] == '\t' ||
@@ -886,37 +884,6 @@ int Pa2ndGenerationImage(TParseContext& pc, int line, const char* text, YYSTYPE*
return PaIdentOrType(text, pc, pyylval);
}
-// TODO: CPP: is this dead code? Combine with other comment parsers.
-int PaParseComment(int& lineno, TParseContext& parseContextLocal)
-{
- int transitionFlag = 0;
- int nextChar;
-
- while (transitionFlag != 2) {
- nextChar = yyinput();
- if (nextChar == '\n')
- lineno++;
- switch (nextChar) {
- case '*' :
- transitionFlag = 1;
- break;
- case '/' : /* if star is the previous character, then it is the end of comment */
- if (transitionFlag == 1) {
- return 1 ;
- }
- break;
- case EOF :
- parseContextLocal.error(yylineno, "End of shader found before end of comment.", "", "", "");
-
- return YY_NULL;
- default : /* Any other character will be a part of the comment */
- transitionFlag = 0;
- }
- }
-
- return 1;
-}
-
extern "C" {
void ShPpDebugLogMsg(const char *msg)
diff --git a/glslang/MachineIndependent/glslang.y b/glslang/MachineIndependent/glslang.y
index 3164ced0d..cad4fbdd5 100644
--- a/glslang/MachineIndependent/glslang.y
+++ b/glslang/MachineIndependent/glslang.y
@@ -326,7 +326,6 @@ postfix_expression
TType newType($1->getType());
newType.dereference();
$$->setType(newType);
- // TODO: testing: write a set of dereference tests
}
}
| function_call {
@@ -340,8 +339,6 @@ postfix_expression
// we later see the function calling syntax. Save away the name for now.
//
- // TODO: semantics: if next token is not "(", then this is an error
-
if (*$3.string == "length") {
parseContext.profileRequires($3.line, ENoProfile, 120, "GL_3DL_array_objects", ".length");
$$ = parseContext.intermediate.addMethod($1, TType(EbtInt), $3.string, $2.line);
@@ -453,7 +450,8 @@ function_call
TFunction* fnCall = $1.function;
TOperator op = fnCall->getBuiltInOp();
if (op == EOpArrayLength) {
- // TODO: semantics: check for no arguments to .length()
+ if (fnCall->getParamCount() > 0)
+ parseContext.error($1.line, "method does not accept any arguments", fnCall->getName().c_str(), "");
int length;
if ($1.intermNode->getAsTyped() == 0 || ! $1.intermNode->getAsTyped()->getType().isArray() || $1.intermNode->getAsTyped()->getType().getArraySize() == 0) {
parseContext.error($1.line, "", fnCall->getName().c_str(), "array must be declared with a size before using this method");
@@ -502,8 +500,8 @@ function_call
$$ = parseContext.intermediate.addBuiltInFunctionCall(op, fnCandidate->getParamCount() == 1, $1.intermNode, fnCandidate->getReturnType());
if ($$ == 0) {
parseContext.error($1.intermNode->getLine(), " wrong operand type", "Internal Error",
- "built in unary operator function. Type: %s",
- static_cast<TIntermTyped*>($1.intermNode)->getCompleteString().c_str());
+ "built in unary operator function. Type: %s",
+ static_cast<TIntermTyped*>($1.intermNode)->getCompleteString().c_str());
YYERROR;
}
} else {
@@ -754,7 +752,7 @@ function_identifier
TFunction *function = new TFunction(&symbol->getName(), TType(EbtVoid));
$$.function = function;
} else
- parseContext.error($1->getLine(), "function call, method or subroutine call expected", "", "");
+ parseContext.error($1->getLine(), "function call, method, or subroutine call expected", "", "");
}
if ($$.function == 0) {
@@ -769,6 +767,8 @@ unary_expression
: postfix_expression {
parseContext.variableCheck($1);
$$ = $1;
+ if (TIntermMethod* method = $1->getAsMethodNode())
+ parseContext.error($1->getLine(), "incomplete method syntax", method->getMethodName().c_str(), "");
}
| INC_OP unary_expression {
parseContext.lValueErrorCheck($1.line, "++", $2);
@@ -1086,7 +1086,7 @@ expression
constant_expression
: conditional_expression {
- parseContext.constCheck($1);
+ parseContext.constCheck($1, "");
$$ = $1;
}
;
@@ -1277,6 +1277,7 @@ parameter_declarator
parseContext.profileRequires($1.line, EEsProfile, 300, 0, "arrayed type");
parseContext.arraySizeRequiredCheck($1.line, $1.arraySizes->front());
}
+ parseContext.arrayDimCheck($2.line, $1.arraySizes, $3.arraySizes);
parseContext.arraySizeRequiredCheck($3.line, $3.arraySizes->front());
parseContext.reservedErrorCheck($2.line, *$2.string);
@@ -1346,7 +1347,8 @@ init_declarator_list
parseContext.nonInitConstCheck($3.line, *$3.string, $1.type);
if (parseContext.profile == EEsProfile)
parseContext.arraySizeRequiredCheck($4.line, $4.arraySizes->front());
-
+ parseContext.arrayDimCheck($3.line, $1.type.arraySizes, $4.arraySizes);
+
$$ = $1;
if (! parseContext.arrayQualifierError($4.line, $1.type)) {
@@ -1363,6 +1365,7 @@ init_declarator_list
$1.type.arraySizes = $4.arraySizes;
parseContext.arrayCheck($4.line, *$3.string, $1.type, variable);
}
+ parseContext.arrayDimCheck($3.line, $1.type.arraySizes, $4.arraySizes);
parseContext.profileRequires($5.line, ENoProfile, 120, "GL_3DL_array_objects", "initializer");
@@ -1414,7 +1417,8 @@ single_declaration
$$.intermAggregate = 0;
parseContext.nonInitConstCheck($2.line, *$2.string, $1);
if (parseContext.profile == EEsProfile)
- parseContext.arraySizeRequiredCheck($3.line, $3.arraySizes->front());
+ parseContext.arraySizeRequiredCheck($3.line, $3.arraySizes->front());
+ parseContext.arrayDimCheck($2.line, $1.arraySizes, $3.arraySizes);
$$.type = $1;
@@ -1425,7 +1429,9 @@ single_declaration
}
parseContext.updateTypedDefaults($2.line, $$.type.qualifier, $2.string);
}
- | fully_specified_type IDENTIFIER array_specifier EQUAL initializer {
+ | fully_specified_type IDENTIFIER array_specifier EQUAL initializer {
+ parseContext.arrayDimCheck($3.line, $1.arraySizes, $3.arraySizes);
+
$$.intermAggregate = 0;
$$.type = $1;
@@ -1736,6 +1742,7 @@ type_specifier
$$.qualifier.precision = parseContext.getDefaultPrecision($$);
}
| type_specifier_nonarray array_specifier {
+ parseContext.arrayDimCheck($2.line, $2.arraySizes, 0);
$$ = $1;
$$.qualifier.precision = parseContext.getDefaultPrecision($$);
$$.arraySizes = $2.arraySizes;
@@ -2477,8 +2484,10 @@ struct_declaration
parseContext.voidErrorCheck($1.line, (*$2)[0].type->getFieldName(), $1);
parseContext.precisionQualifierCheck($1.line, $1);
- for (unsigned int i = 0; i < $$->size(); ++i)
+ for (unsigned int i = 0; i < $$->size(); ++i) {
+ parseContext.arrayDimCheck($1.line, (*$$)[i].type, $1.arraySizes);
(*$$)[i].type->mergeType($1);
+ }
}
| type_qualifier type_specifier struct_declarator_list SEMICOLON {
if ($2.arraySizes) {
@@ -2494,8 +2503,10 @@ struct_declaration
parseContext.mergeQualifiers($2.line, $2.qualifier, $1.qualifier, true);
parseContext.precisionQualifierCheck($2.line, $2);
- for (unsigned int i = 0; i < $$->size(); ++i)
+ for (unsigned int i = 0; i < $$->size(); ++i) {
+ parseContext.arrayDimCheck($1.line, (*$$)[i].type, $2.arraySizes);
(*$$)[i].type->mergeType($2);
+ }
}
;
@@ -2518,6 +2529,8 @@ struct_declarator
| IDENTIFIER array_specifier {
if (parseContext.profile == EEsProfile)
parseContext.arraySizeRequiredCheck($2.line, $2.arraySizes->front());
+ parseContext.arrayDimCheck($1.line, $2.arraySizes, 0);
+
$$.type = new TType(EbtVoid);
$$.line = $1.line;
$$.type->setFieldName(*$1.string);
@@ -2678,6 +2691,8 @@ switch_statement_list
case_label
: CASE expression COLON {
+ parseContext.constCheck($2, "case");
+ parseContext.integerCheck($2, "case");
$$ = parseContext.intermediate.addBranch(EOpCase, $2, $1.line);
}
| DEFAULT COLON {
diff --git a/glslang/MachineIndependent/preprocessor/cpp.c b/glslang/MachineIndependent/preprocessor/cpp.c
index 51872a01e..86a28fb57 100644
--- a/glslang/MachineIndependent/preprocessor/cpp.c
+++ b/glslang/MachineIndependent/preprocessor/cpp.c
@@ -78,8 +78,6 @@ NVIDIA HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// cpp.c
//
-// TODO: CPP handle escaped newlines in a // style comment, correctly done in ConsumeWhitespaceComment()
-
#define _CRT_SECURE_NO_WARNINGS
#include <stdarg.h>
--
GitLab