Refactor ternary operator parsing
Refactor ternary operator parsing so that validation is done in
ParseContext and Intermediate's role is simply to create the node added
to the tree.
Remove partially bugged checks for null nodes as a part of this - in
error cases the parser doesn't typically add null nodes to the tree, but
rather always has a fallback to add a dummy node if parsing fails as a
method of recovery. When parsing ternary operators it should be
guaranteed that none of the parameter nodes is null.
Includes a better explanation of why ternary operators are not always
folded when only the condition is constant, and a test to make sure this
doesn't regress.
BUG=angleproject:952
TEST=WebGL conformance tests, angle_unittests
Change-Id: Icbcb721b5ab36cf314a16e79f9814aef1f355fa0
Reviewed-on: https://chromium-review.googlesource.com/265643
Reviewed-by: Olli Etuaho <oetuaho@nvidia.com>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index 320056f..ecc6915 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -305,22 +305,17 @@
// a true path, and a false path. The two paths are specified
// as separate parameters.
//
-// Returns the selection node created, or 0 if one could not be.
+// Returns the selection node created, or one of trueBlock and falseBlock if the expression could be folded.
//
-TIntermTyped *TIntermediate::addSelection(
- TIntermTyped *cond, TIntermTyped *trueBlock, TIntermTyped *falseBlock,
- const TSourceLoc &line)
+TIntermTyped *TIntermediate::addSelection(TIntermTyped *cond, TIntermTyped *trueBlock, TIntermTyped *falseBlock,
+ const TSourceLoc &line)
{
- if (!cond || !trueBlock || !falseBlock ||
- trueBlock->getType() != falseBlock->getType())
- {
- return NULL;
- }
-
- //
- // See if all the operands are constant, then fold it otherwise not.
- //
-
+ // Right now it's safe to fold ternary operators only when all operands
+ // are constant. If only the condition is constant, it's theoretically
+ // possible to fold the ternary operator, but that requires making sure
+ // that the node returned from here won't be treated as a constant
+ // expression in case the node that gets eliminated was not a constant
+ // expression.
if (cond->getAsConstantUnion() &&
trueBlock->getAsConstantUnion() &&
falseBlock->getAsConstantUnion())
@@ -334,8 +329,7 @@
//
// Make a selection node.
//
- TIntermSelection *node = new TIntermSelection(
- cond, trueBlock, falseBlock, trueBlock->getType());
+ TIntermSelection *node = new TIntermSelection(cond, trueBlock, falseBlock, trueBlock->getType());
node->getTypePointer()->setQualifier(EvqTemporary);
node->setLine(line);