Ensure that if-else branches are always sequence nodes
This mainly affects RewriteElseBlocks, which was the only piece of
code still adding TIntermIfElse nodes directly as children of other
TIntermIfElse nodes.
BUG=angleproject:1490
TEST=angle_unittests
Change-Id: I5b25c2fb9c642424417cd6c29e37c20482c6ffaf
Reviewed-on: https://chromium-review.googlesource.com/392847
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index e17ac56..09bc972 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -310,8 +310,8 @@
bool TIntermIfElse::replaceChildNode(TIntermNode *original, TIntermNode *replacement)
{
REPLACE_IF_IS(mCondition, TIntermTyped, original, replacement);
- REPLACE_IF_IS(mTrueBlock, TIntermNode, original, replacement);
- REPLACE_IF_IS(mFalseBlock, TIntermNode, original, replacement);
+ REPLACE_IF_IS(mTrueBlock, TIntermAggregate, original, replacement);
+ REPLACE_IF_IS(mFalseBlock, TIntermAggregate, original, replacement);
return false;
}
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 18140c6..f261e76 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -641,7 +641,7 @@
class TIntermIfElse : public TIntermNode
{
public:
- TIntermIfElse(TIntermTyped *cond, TIntermNode *trueB, TIntermNode *falseB)
+ TIntermIfElse(TIntermTyped *cond, TIntermAggregate *trueB, TIntermAggregate *falseB)
: TIntermNode(), mCondition(cond), mTrueBlock(trueB), mFalseBlock(falseB)
{
}
@@ -650,14 +650,14 @@
bool replaceChildNode(TIntermNode *original, TIntermNode *replacement) override;
TIntermTyped *getCondition() const { return mCondition; }
- TIntermNode *getTrueBlock() const { return mTrueBlock; }
- TIntermNode *getFalseBlock() const { return mFalseBlock; }
+ TIntermAggregate *getTrueBlock() const { return mTrueBlock; }
+ TIntermAggregate *getFalseBlock() const { return mFalseBlock; }
TIntermIfElse *getAsIfElseNode() override { return this; }
protected:
TIntermTyped *mCondition;
- TIntermNode *mTrueBlock;
- TIntermNode *mFalseBlock;
+ TIntermAggregate *mTrueBlock;
+ TIntermAggregate *mFalseBlock;
};
//
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index cd9712e..57d53bf 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -142,11 +142,10 @@
//
// Returns an aggregate, unless NULL was passed in for the existing node.
//
-TIntermAggregate *TIntermediate::makeAggregate(
- TIntermNode *node, const TSourceLoc &line)
+TIntermAggregate *TIntermediate::MakeAggregate(TIntermNode *node, const TSourceLoc &line)
{
- if (node == NULL)
- return NULL;
+ if (node == nullptr)
+ return nullptr;
TIntermAggregate *aggNode = new TIntermAggregate;
aggNode->getSequence()->push_back(node);
@@ -159,7 +158,7 @@
// If the input node is nullptr, return nullptr.
// If the input node is a sequence (block) node, return it.
// If the input node is not a sequence node, put it inside a sequence node and return that.
-TIntermAggregate *TIntermediate::ensureSequence(TIntermNode *node)
+TIntermAggregate *TIntermediate::EnsureSequence(TIntermNode *node)
{
if (node == nullptr)
return nullptr;
@@ -167,7 +166,7 @@
if (aggNode != nullptr && aggNode->getOp() == EOpSequence)
return aggNode;
- aggNode = makeAggregate(node, node->getLine());
+ aggNode = MakeAggregate(node, node->getLine());
aggNode->setOp(EOpSequence);
return aggNode;
}
@@ -200,7 +199,7 @@
}
TIntermIfElse *node =
- new TIntermIfElse(cond, ensureSequence(nodePair.node1), ensureSequence(nodePair.node2));
+ new TIntermIfElse(cond, EnsureSequence(nodePair.node1), EnsureSequence(nodePair.node2));
node->setLine(line);
return node;
@@ -332,7 +331,7 @@
TLoopType type, TIntermNode *init, TIntermTyped *cond, TIntermTyped *expr,
TIntermNode *body, const TSourceLoc &line)
{
- TIntermNode *node = new TIntermLoop(type, init, cond, expr, ensureSequence(body));
+ TIntermNode *node = new TIntermLoop(type, init, cond, expr, EnsureSequence(body));
node->setLine(line);
return node;
diff --git a/src/compiler/translator/Intermediate.h b/src/compiler/translator/Intermediate.h
index bf78b32..c4a21d8 100644
--- a/src/compiler/translator/Intermediate.h
+++ b/src/compiler/translator/Intermediate.h
@@ -35,8 +35,8 @@
TOperator op, TIntermTyped *child, const TSourceLoc &line, const TType *funcReturnType);
TIntermAggregate *growAggregate(
TIntermNode *left, TIntermNode *right, const TSourceLoc &);
- TIntermAggregate *makeAggregate(TIntermNode *node, const TSourceLoc &);
- TIntermAggregate *ensureSequence(TIntermNode *node);
+ static TIntermAggregate *MakeAggregate(TIntermNode *node, const TSourceLoc &line);
+ static TIntermAggregate *EnsureSequence(TIntermNode *node);
TIntermAggregate *setAggregateOperator(TIntermNode *, TOperator, const TSourceLoc &);
TIntermNode *addIfElse(TIntermTyped *cond, TIntermNodePair code, const TSourceLoc &line);
static TIntermTyped *AddTernarySelection(TIntermTyped *cond,
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index 5762a29..4ed4b42 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -1944,9 +1944,8 @@
outputLineDirective(out, node->getFalseBlock()->getLine().first_line);
- // Either this is "else if" or the falseBlock child node will output braces.
- ASSERT(IsSequence(node->getFalseBlock()) ||
- node->getFalseBlock()->getAsIfElseNode() != nullptr);
+ // The falseBlock child node will output braces.
+ ASSERT(IsSequence(node->getFalseBlock()));
node->getFalseBlock()->traverse(this);
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index f852937..99d6968 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -1588,7 +1588,7 @@
symbol->setId(variable->getUniqueId());
}
- return intermediate.makeAggregate(symbol, identifierOrTypeLocation);
+ return TIntermediate::MakeAggregate(symbol, identifierOrTypeLocation);
}
TIntermAggregate *TParseContext::parseSingleArrayDeclaration(TPublicType &publicType,
@@ -1619,7 +1619,7 @@
if (variable && symbol)
symbol->setId(variable->getUniqueId());
- return intermediate.makeAggregate(symbol, identifierLocation);
+ return TIntermediate::MakeAggregate(symbol, identifierLocation);
}
TIntermAggregate *TParseContext::parseSingleInitDeclaration(const TPublicType &publicType,
@@ -1638,7 +1638,7 @@
//
// Build intermediate representation
//
- return intermNode ? intermediate.makeAggregate(intermNode, initLocation) : nullptr;
+ return intermNode ? TIntermediate::MakeAggregate(intermNode, initLocation) : nullptr;
}
else
{
@@ -1678,7 +1678,7 @@
TIntermNode *initNode = nullptr;
if (!executeInitializer(identifierLocation, identifier, arrayType, initializer, &initNode))
{
- return initNode ? intermediate.makeAggregate(initNode, initLocation) : nullptr;
+ return initNode ? TIntermediate::MakeAggregate(initNode, initLocation) : nullptr;
}
else
{
@@ -1735,7 +1735,7 @@
TIntermSymbol *intermSymbol =
intermediate.addSymbol(variable->getUniqueId(), *identifier, type, identifierLoc);
- TIntermAggregate *aggregate = intermediate.makeAggregate(intermSymbol, identifierLoc);
+ TIntermAggregate *aggregate = TIntermediate::MakeAggregate(intermSymbol, identifierLoc);
aggregate->setOp(EOpInvariantDeclaration);
return aggregate;
}
@@ -2537,7 +2537,7 @@
symbolName = instanceTypeDef->getName();
}
- TIntermAggregate *aggregate = intermediate.makeAggregate(
+ TIntermAggregate *aggregate = TIntermediate::MakeAggregate(
intermediate.addSymbol(symbolId, symbolName, interfaceBlockType, typeQualifier.line),
nameLine);
aggregate->setOp(EOpDeclaration);
diff --git a/src/compiler/translator/RewriteElseBlocks.cpp b/src/compiler/translator/RewriteElseBlocks.cpp
index bbffc79..13d71c7 100644
--- a/src/compiler/translator/RewriteElseBlocks.cpp
+++ b/src/compiler/translator/RewriteElseBlocks.cpp
@@ -8,6 +8,8 @@
//
#include "compiler/translator/RewriteElseBlocks.h"
+
+#include "compiler/translator/Intermediate.h"
#include "compiler/translator/NodeSearch.h"
#include "compiler/translator/SymbolTable.h"
@@ -49,16 +51,7 @@
TIntermIfElse *ifElse = statement->getAsIfElseNode();
if (ifElse && ifElse->getFalseBlock() != nullptr)
{
- // Check for if / else if
- TIntermIfElse *elseIfBranch = ifElse->getFalseBlock()->getAsIfElseNode();
- if (elseIfBranch)
- {
- ifElse->replaceChildNode(elseIfBranch, rewriteIfElse(elseIfBranch));
- delete elseIfBranch;
- }
-
(*node->getSequence())[statementIndex] = rewriteIfElse(ifElse);
- delete ifElse;
}
}
}
@@ -84,7 +77,7 @@
TIntermTyped *typedCondition = ifElse->getCondition()->getAsTyped();
TIntermAggregate *storeCondition = createTempInitDeclaration(typedCondition);
- TIntermIfElse *falseBlock = nullptr;
+ TIntermAggregate *falseBlock = nullptr;
TType boolType(EbtBool, EbpUndefined, EvqTemporary);
@@ -107,7 +100,9 @@
TIntermSymbol *conditionSymbolElse = createTempSymbol(boolType);
TIntermUnary *negatedCondition = new TIntermUnary(EOpLogicalNot, conditionSymbolElse);
- falseBlock = new TIntermIfElse(negatedCondition, ifElse->getFalseBlock(), negatedElse);
+ TIntermIfElse *falseIfElse =
+ new TIntermIfElse(negatedCondition, ifElse->getFalseBlock(), negatedElse);
+ falseBlock = TIntermediate::EnsureSequence(falseIfElse);
}
TIntermSymbol *conditionSymbolSel = createTempSymbol(boolType);
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index c068c05..ef2dcaa 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -351,7 +351,7 @@
const TType *type = new TType($2->getType());
$1->addParameter(TConstParameter(type));
$$.function = $1;
- $$.nodePair.node1 = context->intermediate.makeAggregate($2, @2);
+ $$.nodePair.node1 = TIntermediate::MakeAggregate($2, @2);
}
| function_call_header_with_parameters COMMA assignment_expression {
const TType *type = new TType($3->getType());
@@ -1319,7 +1319,7 @@
statement_list
: statement {
- $$ = context->intermediate.makeAggregate($1, @$);
+ $$ = TIntermediate::MakeAggregate($1, @$);
}
| statement_list statement {
$$ = context->intermediate.growAggregate($1, $2, @$);
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index a34001d..2d3f170 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -2546,7 +2546,7 @@
const TType *type = new TType((yyvsp[0].interm.intermTypedNode)->getType());
(yyvsp[-1].interm.function)->addParameter(TConstParameter(type));
(yyval.interm).function = (yyvsp[-1].interm.function);
- (yyval.interm).nodePair.node1 = context->intermediate.makeAggregate((yyvsp[0].interm.intermTypedNode), (yylsp[0]));
+ (yyval.interm).nodePair.node1 = TIntermediate::MakeAggregate((yyvsp[0].interm.intermTypedNode), (yylsp[0]));
}
break;
@@ -4371,7 +4371,7 @@
case 234:
{
- (yyval.interm.intermAggregate) = context->intermediate.makeAggregate((yyvsp[0].interm.intermNode), (yyloc));
+ (yyval.interm.intermAggregate) = TIntermediate::MakeAggregate((yyvsp[0].interm.intermNode), (yyloc));
}
break;