Unify opaque type validation in GLSL parsing
Refactor separate sampler and image validations into unified opaque
type handling. This paves way for adding atomic counter, another
new opaque type.
BUG=angleproject:1729
Change-Id: I201d28e31c84534db43e656d518650e378bab76c
Reviewed-on: https://chromium-review.googlesource.com/493618
Reviewed-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index d30c0f9..81a8370 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -35,7 +35,7 @@
if (IsSampler(type.getBasicType()))
return true;
- if (type.getBasicType() == EbtStruct || type.isInterfaceBlock())
+ if (type.getBasicType() == EbtStruct)
{
const TFieldList &fields = type.getStruct()->fields();
for (unsigned int i = 0; i < fields.size(); ++i)
@@ -48,24 +48,6 @@
return false;
}
-bool ContainsImage(const TType &type)
-{
- if (IsImage(type.getBasicType()))
- return true;
-
- if (type.getBasicType() == EbtStruct || type.isInterfaceBlock())
- {
- const TFieldList &fields = type.getStruct()->fields();
- for (unsigned int i = 0; i < fields.size(); ++i)
- {
- if (ContainsImage(*fields[i]->type()))
- return true;
- }
- }
-
- return false;
-}
-
// Get a token from an image argument to use as an error message token.
const char *GetImageArgumentToken(TIntermTyped *imageNode)
{
@@ -340,14 +322,9 @@
error(line, "No precision specified (int)", "");
return;
default:
- if (IsSampler(type))
+ if (IsOpaqueType(type))
{
- error(line, "No precision specified (sampler)", "");
- return;
- }
- if (IsImage(type))
- {
- error(line, "No precision specified (image)", "");
+ error(line, "No precision specified", getBasicString(type));
return;
}
}
@@ -389,7 +366,7 @@
return false;
}
- const char *message = 0;
+ std::string message;
switch (node->getQualifier())
{
case EvqConst:
@@ -454,17 +431,14 @@
{
message = "can't modify void";
}
- if (IsSampler(node->getBasicType()))
+ if (IsOpaqueType(node->getBasicType()))
{
- message = "can't modify a sampler";
- }
- if (IsImage(node->getBasicType()))
- {
- message = "can't modify an image";
+ message = "can't modify a variable with type ";
+ message += getBasicString(node->getBasicType());
}
}
- if (message == 0 && binaryNode == 0 && symNode == 0)
+ if (message.empty() && binaryNode == 0 && symNode == 0)
{
error(line, "l-value required", op);
@@ -474,7 +448,7 @@
//
// Everything else is okay, no error.
//
- if (message == 0)
+ if (message.empty())
return true;
//
@@ -686,14 +660,11 @@
{
TIntermTyped *argTyped = argNode->getAsTyped();
ASSERT(argTyped != nullptr);
- if (op != EOpConstructStruct && IsSampler(argTyped->getBasicType()))
+ if (op != EOpConstructStruct && IsOpaqueType(argTyped->getBasicType()))
{
- error(line, "cannot convert a sampler", "constructor");
- return false;
- }
- if (op != EOpConstructStruct && IsImage(argTyped->getBasicType()))
- {
- error(line, "cannot convert an image", "constructor");
+ std::string reason("cannot convert a variable with type ");
+ reason += getBasicString(argTyped->getBasicType());
+ error(line, reason.c_str(), "constructor");
return false;
}
if (argTyped->getBasicType() == EbtVoid)
@@ -780,9 +751,9 @@
}
}
-bool TParseContext::checkIsNotSampler(const TSourceLoc &line,
- const TTypeSpecifierNonArray &pType,
- const char *reason)
+bool TParseContext::checkIsNotOpaqueType(const TSourceLoc &line,
+ const TTypeSpecifierNonArray &pType,
+ const char *reason)
{
if (pType.type == EbtStruct)
{
@@ -794,10 +765,11 @@
error(line, reasonStr.c_str(), getBasicString(pType.type));
return false;
}
-
+ // only samplers need to be checked from structs, since other opaque types can't be struct
+ // members.
return true;
}
- else if (IsSampler(pType.type))
+ else if (IsOpaqueType(pType.type))
{
error(line, reason, getBasicString(pType.type));
return false;
@@ -806,34 +778,6 @@
return true;
}
-bool TParseContext::checkIsNotImage(const TSourceLoc &line,
- const TTypeSpecifierNonArray &pType,
- const char *reason)
-{
- if (pType.type == EbtStruct)
- {
- if (ContainsImage(*pType.userDef))
- {
- std::stringstream reasonStream;
- reasonStream << reason << " (structure contains an image)";
- std::string reasonStr = reasonStream.str();
- error(line, reasonStr.c_str(), getBasicString(pType.type));
-
- return false;
- }
-
- return true;
- }
- else if (IsImage(pType.type))
- {
- error(line, reason, getBasicString(pType.type));
-
- return false;
- }
-
- return true;
-}
-
void TParseContext::checkDeclaratorLocationIsNotSpecified(const TSourceLoc &line,
const TPublicType &pType)
{
@@ -863,29 +807,10 @@
TQualifier qualifier,
const TType &type)
{
- checkOutParameterIsNotSampler(line, qualifier, type);
- checkOutParameterIsNotImage(line, qualifier, type);
-}
-
-void TParseContext::checkOutParameterIsNotSampler(const TSourceLoc &line,
- TQualifier qualifier,
- const TType &type)
-{
ASSERT(qualifier == EvqOut || qualifier == EvqInOut);
- if (IsSampler(type.getBasicType()))
+ if (IsOpaqueType(type.getBasicType()))
{
- error(line, "samplers cannot be output parameters", type.getBasicString());
- }
-}
-
-void TParseContext::checkOutParameterIsNotImage(const TSourceLoc &line,
- TQualifier qualifier,
- const TType &type)
-{
- ASSERT(qualifier == EvqOut || qualifier == EvqInOut);
- if (IsImage(type.getBasicType()))
- {
- error(line, "images cannot be output parameters", type.getBasicString());
+ error(line, "opaque types cannot be output parameters", type.getBasicString());
}
}
@@ -1227,16 +1152,10 @@
default:
break;
}
-
+ std::string reason(getBasicString(publicType.getBasicType()));
+ reason += "s must be uniform";
if (publicType.qualifier != EvqUniform &&
- !checkIsNotSampler(identifierLocation, publicType.typeSpecifierNonArray,
- "samplers must be uniform"))
- {
- return;
- }
- if (publicType.qualifier != EvqUniform &&
- !checkIsNotImage(identifierLocation, publicType.typeSpecifierNonArray,
- "images must be uniform"))
+ !checkIsNotOpaqueType(identifierLocation, publicType.typeSpecifierNonArray, reason.c_str()))
{
return;
}
@@ -2757,10 +2676,10 @@
{
error(location, "no qualifiers allowed for function return", "layout");
}
- // make sure a sampler or an image is not involved as well...
- checkIsNotSampler(location, type.typeSpecifierNonArray,
- "samplers can't be function return values");
- checkIsNotImage(location, type.typeSpecifierNonArray, "images can't be function return values");
+ // make sure an opaque type is not involved as well...
+ std::string reason(getBasicString(type.getBasicType()));
+ reason += "s can't be function return values";
+ checkIsNotOpaqueType(location, type.typeSpecifierNonArray, reason.c_str());
if (mShaderVersion < 300)
{
// Array return values are forbidden, but there's also no valid syntax for declaring array
@@ -2911,18 +2830,12 @@
{
TField *field = (*fieldList)[memberIndex];
TType *fieldType = field->type();
- if (IsSampler(fieldType->getBasicType()))
+ if (IsOpaqueType(fieldType->getBasicType()))
{
- error(field->line(),
- "unsupported type - sampler types are not allowed in interface blocks",
- fieldType->getBasicString());
- }
-
- if (IsImage(fieldType->getBasicType()))
- {
- error(field->line(),
- "unsupported type - image types are not allowed in interface blocks",
- fieldType->getBasicString());
+ std::string reason("unsupported type - ");
+ reason += fieldType->getBasicString();
+ reason += " types are not allowed in interface blocks";
+ error(field->line(), reason.c_str(), fieldType->getBasicString());
}
const TQualifier qualifier = fieldType->getQualifier();
@@ -3856,6 +3769,9 @@
TIntermTyped *right,
const TSourceLoc &loc)
{
+ // TODO(jie.a.chen@intel.com): Validate opaque type variables can only be operands in array
+ // indexing, structure member selection, and parentheses expressions.
+
if (left->getType().getStruct() || right->getType().getStruct())
{
switch (op)
@@ -3979,13 +3895,6 @@
return false;
}
- if ((op == EOpAssign || op == EOpInitialize) &&
- left->getType().isStructureContainingImages())
- {
- error(loc, "undefined operation for structs containing images",
- GetOperatorString(op));
- return false;
- }
if ((left->getNominalSize() != right->getNominalSize()) ||
(left->getSecondarySize() != right->getSecondarySize()))
{