Add support for arrays of arrays in AST processing
Data concerning arrays of arrays is added in TType.
Parsing arrays of arrays and support for arrays of arrays in
TPublicType are still left to be implemented later.
ShaderVariable interface for arrays of arrays is also left to be
implemented later.
We rely on existing test coverage to make sure that arrays of arrays
are not accidentally exposed.
BUG=angleproject:2125
TEST=angle_unittests, angle_end2end_tests, angle_deqp_gles31_tests
Change-Id: Ie17d5ac9b8d33958e9126dc0fb40bf1c81ddeec9
Reviewed-on: https://chromium-review.googlesource.com/616146
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 25ee442..f422f04 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -680,7 +680,7 @@
{
// The size of an unsized constructor should already have been determined.
ASSERT(!type.isUnsizedArray());
- if (static_cast<size_t>(type.getArraySize()) != arguments->size())
+ if (static_cast<size_t>(type.getOutermostArraySize()) != arguments->size())
{
error(line, "array constructor needs one argument per array element", "constructor");
return false;
@@ -695,7 +695,7 @@
error(line, "constructing from a non-dereferenced array", "constructor");
return false;
}
- if (!argType.sameElementType(type))
+ if (!argType.isElementTypeOf(type))
{
error(line, "Array constructor argument has an incorrect type", "constructor");
return false;
@@ -1057,7 +1057,14 @@
{
const TVariable *maxDrawBuffers = static_cast<const TVariable *>(
symbolTable.findBuiltIn("gl_MaxDrawBuffers", mShaderVersion));
- if (static_cast<int>(type.getArraySize()) == maxDrawBuffers->getConstPointer()->getIConst())
+ if (type.isArrayOfArrays())
+ {
+ error(line, "redeclaration of gl_LastFragData as an array of arrays",
+ identifier.c_str());
+ return false;
+ }
+ else if (static_cast<int>(type.getOutermostArraySize()) ==
+ maxDrawBuffers->getConstPointer()->getIConst())
{
if (TSymbol *builtInSymbol = symbolTable.findBuiltIn(identifier, mShaderVersion))
{
@@ -1389,14 +1396,21 @@
void TParseContext::checkBindingIsValid(const TSourceLoc &identifierLocation, const TType &type)
{
TLayoutQualifier layoutQualifier = type.getLayoutQualifier();
- int arraySize = type.isArray() ? type.getArraySize() : 1;
+ // Note that the ESSL 3.10 section 4.4.5 is not particularly clear on how the binding qualifier
+ // on arrays of arrays should be handled. We interpret the spec so that the binding value is
+ // incremented for each element of the innermost nested arrays. This is in line with how arrays
+ // of arrays of blocks are specified to behave in GLSL 4.50 and a conservative interpretation
+ // when it comes to which shaders are accepted by the compiler.
+ int arrayTotalElementCount = type.getArraySizeProduct();
if (IsImage(type.getBasicType()))
{
- checkImageBindingIsValid(identifierLocation, layoutQualifier.binding, arraySize);
+ checkImageBindingIsValid(identifierLocation, layoutQualifier.binding,
+ arrayTotalElementCount);
}
else if (IsSampler(type.getBasicType()))
{
- checkSamplerBindingIsValid(identifierLocation, layoutQualifier.binding, arraySize);
+ checkSamplerBindingIsValid(identifierLocation, layoutQualifier.binding,
+ arrayTotalElementCount);
}
else if (IsAtomicCounter(type.getBasicType()))
{
@@ -1468,10 +1482,12 @@
}
}
-void TParseContext::checkImageBindingIsValid(const TSourceLoc &location, int binding, int arraySize)
+void TParseContext::checkImageBindingIsValid(const TSourceLoc &location,
+ int binding,
+ int arrayTotalElementCount)
{
// Expects arraySize to be 1 when setting binding for only a single variable.
- if (binding >= 0 && binding + arraySize > mMaxImageUnits)
+ if (binding >= 0 && binding + arrayTotalElementCount > mMaxImageUnits)
{
error(location, "image binding greater than gl_MaxImageUnits", "binding");
}
@@ -1479,10 +1495,10 @@
void TParseContext::checkSamplerBindingIsValid(const TSourceLoc &location,
int binding,
- int arraySize)
+ int arrayTotalElementCount)
{
// Expects arraySize to be 1 when setting binding for only a single variable.
- if (binding >= 0 && binding + arraySize > mMaxCombinedTextureImageUnits)
+ if (binding >= 0 && binding + arrayTotalElementCount > mMaxCombinedTextureImageUnits)
{
error(location, "sampler binding greater than maximum texture units", "binding");
}
@@ -1777,7 +1793,7 @@
else if (variable->getType().getQualifier() == EvqPerVertexIn)
{
TType type(variable->getType());
- type.setArraySize(mGeometryShaderInputArraySize);
+ type.setArraySize(0, mGeometryShaderInputArraySize);
node = new TIntermSymbol(variable->getUniqueId(), variable->getName(), type);
}
else
@@ -1806,17 +1822,11 @@
TVariable *variable = nullptr;
if (type.isUnsizedArray())
{
- // We have not checked yet whether the initializer actually is an array or not.
- if (initializer->isArray())
- {
- type.setArraySize(initializer->getArraySize());
- }
- else
- {
- // Having a non-array initializer for an unsized array will result in an error later,
- // so we don't generate an error message here.
- type.setArraySize(1u);
- }
+ // In case initializer is not an array or type has more dimensions than initializer, this
+ // will default to setting array sizes to 1. We have not checked yet whether the initializer
+ // actually is an array or not. Having a non-array initializer for an unsized array will
+ // result in an error later, so we don't generate an error message here.
+ type.sizeUnsizedArrays(initializer->getType().getArraySizes());
}
if (!declareVariable(line, identifier, type, &variable))
{
@@ -2353,7 +2363,7 @@
unsigned int size = checkIsValidArraySize(identifierLocation, indexExpression);
// Make the type an array even if size check failed.
// This ensures useless error messages regarding the variable's non-arrayness won't follow.
- arrayType.setArraySize(size);
+ arrayType.makeArray(size);
if (IsAtomicCounter(publicType.getBasicType()))
{
@@ -2564,7 +2574,7 @@
{
TType arrayType = TType(publicType);
unsigned int size = checkIsValidArraySize(arrayLocation, indexExpression);
- arrayType.setArraySize(size);
+ arrayType.makeArray(size);
if (IsAtomicCounter(publicType.getBasicType()))
{
@@ -3341,6 +3351,45 @@
return parseParameterDeclarator(*type, identifier, identifierLoc);
}
+bool TParseContext::checkUnsizedArrayConstructorArgumentDimensionality(TIntermSequence *arguments,
+ TType type,
+ const TSourceLoc &line)
+{
+ if (arguments->empty())
+ {
+ error(line, "implicitly sized array constructor must have at least one argument", "[]");
+ return false;
+ }
+ for (TIntermNode *arg : *arguments)
+ {
+ TIntermTyped *element = arg->getAsTyped();
+ ASSERT(element);
+ size_t dimensionalityFromElement = element->getType().getArraySizes().size() + 1u;
+ if (dimensionalityFromElement > type.getArraySizes().size())
+ {
+ error(line, "constructing from a non-dereferenced array", "constructor");
+ return false;
+ }
+ else if (dimensionalityFromElement < type.getArraySizes().size())
+ {
+ if (dimensionalityFromElement == 1u)
+ {
+ error(line, "implicitly sized array of arrays constructor argument is not an array",
+ "constructor");
+ }
+ else
+ {
+ error(line,
+ "implicitly sized array of arrays constructor argument dimensionality is too "
+ "low",
+ "constructor");
+ }
+ return false;
+ }
+ }
+ return true;
+}
+
// This function is used to test for the correctness of the parameters passed to various constructor
// functions and also convert them to the right datatype if it is allowed and required.
//
@@ -3352,13 +3401,23 @@
{
if (type.isUnsizedArray())
{
- if (arguments->empty())
+ if (!checkUnsizedArrayConstructorArgumentDimensionality(arguments, type, line))
{
- error(line, "implicitly sized array constructor must have at least one argument", "[]");
- type.setArraySize(1u);
+ type.sizeUnsizedArrays(TVector<unsigned int>());
return CreateZeroNode(type);
}
- type.setArraySize(static_cast<unsigned int>(arguments->size()));
+ TIntermTyped *firstElement = arguments->at(0)->getAsTyped();
+ ASSERT(firstElement);
+ type.setArraySize(type.getArraySizes().size() - 1u,
+ static_cast<unsigned int>(arguments->size()));
+ for (size_t i = 0; i < firstElement->getType().getArraySizes().size(); ++i)
+ {
+ if (type.getArraySizes()[i] == 0u)
+ {
+ type.setArraySize(i, firstElement->getType().getArraySizes().at(i));
+ }
+ }
+ ASSERT(!type.isUnsizedArray());
}
if (!checkConstructorArguments(line, arguments, type))
@@ -3561,8 +3620,11 @@
TInterfaceBlock *interfaceBlock =
new TInterfaceBlock(&blockName, fieldList, instanceName, blockLayoutQualifier);
- TType interfaceBlockType(interfaceBlock, typeQualifier.qualifier, blockLayoutQualifier,
- arraySize);
+ TType interfaceBlockType(interfaceBlock, typeQualifier.qualifier, blockLayoutQualifier);
+ if (arrayIndex != nullptr)
+ {
+ interfaceBlockType.makeArray(arraySize);
+ }
TString symbolName = "";
int symbolId = 0;
@@ -3768,7 +3830,7 @@
if (safeIndex < 0)
{
safeIndex = checkIndexOutOfRange(outOfRangeIndexIsError, location, index,
- baseExpression->getArraySize(),
+ baseExpression->getOutermostArraySize(),
"array index out of range");
}
}
@@ -4408,7 +4470,7 @@
TType *type = new TType(EbtVoid, EbpUndefined);
unsigned int size = checkIsValidArraySize(arraySizeLoc, arraySize);
- type->setArraySize(size);
+ type->makeArray(size);
return new TField(type, identifier, loc);
}
@@ -4450,46 +4512,36 @@
}
TFieldList *TParseContext::addStructDeclaratorList(const TPublicType &typeSpecifier,
- TFieldList *fieldList)
+ TFieldList *declaratorList)
{
checkPrecisionSpecified(typeSpecifier.getLine(), typeSpecifier.precision,
typeSpecifier.getBasicType());
- checkIsNonVoid(typeSpecifier.getLine(), (*fieldList)[0]->name(), typeSpecifier.getBasicType());
+ checkIsNonVoid(typeSpecifier.getLine(), (*declaratorList)[0]->name(),
+ typeSpecifier.getBasicType());
checkWorkGroupSizeIsNotSpecified(typeSpecifier.getLine(), typeSpecifier.layoutQualifier);
- for (unsigned int i = 0; i < fieldList->size(); ++i)
+ for (unsigned int i = 0; i < declaratorList->size(); ++i)
{
- //
- // Careful not to replace already known aspects of type, like array-ness
- //
- TType *type = (*fieldList)[i]->type();
- type->setBasicType(typeSpecifier.getBasicType());
- type->setPrimarySize(typeSpecifier.getPrimarySize());
- type->setSecondarySize(typeSpecifier.getSecondarySize());
- type->setPrecision(typeSpecifier.precision);
- type->setQualifier(typeSpecifier.qualifier);
- type->setLayoutQualifier(typeSpecifier.layoutQualifier);
- type->setMemoryQualifier(typeSpecifier.memoryQualifier);
- type->setInvariant(typeSpecifier.invariant);
-
+ auto declaratorArraySizes = (*declaratorList)[i]->type()->getArraySizes();
// don't allow arrays of arrays
- if (type->isArray())
+ if (!declaratorArraySizes.empty())
{
checkIsValidTypeForArray(typeSpecifier.getLine(), typeSpecifier);
}
- if (typeSpecifier.array)
- type->setArraySize(static_cast<unsigned int>(typeSpecifier.arraySize));
- if (typeSpecifier.getUserDef())
+
+ TType *type = (*declaratorList)[i]->type();
+ *type = TType(typeSpecifier);
+ for (unsigned int arraySize : declaratorArraySizes)
{
- type->setStruct(typeSpecifier.getUserDef());
+ type->makeArray(arraySize);
}
- checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *(*fieldList)[i]);
+ checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *(*declaratorList)[i]);
}
- return fieldList;
+ return declaratorList;
}
TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
@@ -4800,7 +4852,7 @@
return false;
}
// At this point, size of implicitly sized arrays should be resolved.
- if (left->getArraySize() != right->getArraySize())
+ if (left->getType().getArraySizes() != right->getType().getArraySizes())
{
error(loc, "array size mismatch", GetOperatorString(op));
return false;
@@ -5398,7 +5450,7 @@
}
else
{
- arraySize = typedThis->getArraySize();
+ arraySize = typedThis->getOutermostArraySize();
if (typedThis->getAsSymbolNode() == nullptr)
{
// This code path can be hit with expressions like these: