Fix issues in comma operator parsing

Always qualify comma operator result with EvqTemporary in ESSL3, as specified. Also, it is possible that in the future some expressions are qualified as EvqConst but they'd still have side effects, in which case discarding them when they're the left operand of the comma operator would be wrong. This would be the case if ANGLE allowed "(a = b).length()" for example. For this reason it is better to check whether the left node has side effects, rather than check its const qualification, and only discard it if it doesn't. Also, Intermediate::addComma() never returns null, so there's no need to check the result. BUG=angleproject:1201 TEST=WebGL conformance tests conformance/glsl/misc/sequence-operator-returns-constant.html conformance2/glsl3/sequence-operator-returns-non-constant.html Change-Id: Ibfbd92baa4910b14c0dc8f8a3c3008440d191cd6 Reviewed-on: https://chromium-review.googlesource.com/311171Reviewed-by: Jamie Madill <jmadill@chromium.org> Tryjob-Request: Jamie Madill <jmadill@chromium.org> Reviewed-by: Corentin Wallez <cwallez@chromium.org> Tested-by: Olli Etuaho <oetuaho@nvidia.com>

Fix issues in comma operator parsing
15200047 · Olli Etuaho · 0b2d2dcf · 15200047 · 15200047 · 15200047
Commit 15200047 authored Nov 04, 2015 by Olli Etuaho
Showing with 25 additions and 20 deletions

Intermediate.cpp src/compiler/translator/Intermediate.cpp +20 -10

Intermediate.h src/compiler/translator/Intermediate.h +4 -2

ParseContext.cpp src/compiler/translator/ParseContext.cpp +1 -8

No files found.
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -281,22 +281,32 @@ TIntermNode *TIntermediate::addSelection(
    return node;
 }

-TIntermTyped *TIntermediate::addComma(
-    TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
+TIntermTyped *TIntermediate::addComma(TIntermTyped *left,
+                                      TIntermTyped *right,
+                                      const TSourceLoc &line,
+                                      int shaderVersion)
 {
-    if (left->getType().getQualifier() == EvqConst &&
-        right->getType().getQualifier() == EvqConst)
+    TQualifier resultQualifier = EvqConst;
+    // ESSL3.00 section 12.43: The result of a sequence operator is not a constant-expression.
+    if (shaderVersion >= 300 || left->getQualifier() != EvqConst ||
+        right->getQualifier() != EvqConst)
    {
-        return right;
+        resultQualifier = EvqTemporary;
+    }
+
+    TIntermTyped *commaNode = nullptr;
+    if (!left->hasSideEffects())
+    {
+        commaNode = right;
    }
    else
    {
-        TIntermTyped *commaAggregate = growAggregate(left, right, line);
-        commaAggregate->getAsAggregate()->setOp(EOpComma);
-        commaAggregate->setType(right->getType());
-        commaAggregate->getTypePointer()->setQualifier(EvqTemporary);
-        return commaAggregate;
+        commaNode = growAggregate(left, right, line);
+        commaNode->getAsAggregate()->setOp(EOpComma);
+        commaNode->setType(right->getType());
    }
+    commaNode->getTypePointer()->setQualifier(resultQualifier);
+    return commaNode;
 }

 //

--- a/src/compiler/translator/Intermediate.h
+++ b/src/compiler/translator/Intermediate.h
@@ -48,8 +48,10 @@ class TIntermediate
        TIntermTyped *init, TIntermAggregate *statementList, const TSourceLoc &line);
    TIntermCase *addCase(
        TIntermTyped *condition, const TSourceLoc &line);
-    TIntermTyped *addComma(
-        TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
+    TIntermTyped *addComma(TIntermTyped *left,
+                           TIntermTyped *right,
+                           const TSourceLoc &line,
+                           int shaderVersion);
    TIntermConstantUnion *addConstantUnion(
        TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line);
    // TODO(zmo): Get rid of default value.

--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3750,14 +3750,7 @@ TIntermTyped *TParseContext::addComma(TIntermTyped *left,
                                      TIntermTyped *right,
                                      const TSourceLoc &loc)
 {
-    TIntermTyped *node = intermediate.addComma(left, right, loc);
-    if (node == nullptr)
-    {
-        binaryOpError(loc, ",", left->getCompleteString(), right->getCompleteString());
-        recover();
-        return right;
-    }
-    return node;
+    return intermediate.addComma(left, right, loc, mShaderVersion);
 }

 TIntermBranch *TParseContext::addBranch(TOperator op, const TSourceLoc &loc)