Improved BinaryExpression position tracking

This avoids a situation where we didn't know the position of a binary
expression while we were in the middle of evaluating it, which was
leading to problems with upcoming error reporting changes.

Change-Id: Ie41fa82d077de1aa1041cd84c539ba29aaa2f5f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527500
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
Ethan Nicholas 2022-04-05 12:32:07 -04:00 committed by SkCQ
parent 59b0688e5e
commit 453e4def6f
3 changed files with 81 additions and 67 deletions

View File

@ -150,6 +150,11 @@ public:
*/
DSLExpression postfix(Operator::Kind op, Position pos);
/**
* Invokes a binary operator.
*/
DSLExpression binary(Operator::Kind op, DSLExpression right, Position pos);
/**
* Equivalent to operator[].
*/

View File

@ -1352,6 +1352,21 @@ DSLStatement DSLParser::expressionStatement() {
return {};
}
#define OPERATOR_RIGHT(op, exprType) \
do { \
this->nextToken(); \
if (!depth.increase()) { \
return {}; \
} \
DSLExpression right = this->exprType(); \
if (!right.hasValue()) { \
return {}; \
} \
Position pos = result.position().rangeThrough(right.position()); \
DSLExpression next = result.binary(Operator::Kind::op, std::move(right), pos); \
result.swap(next); \
} while (false)
/* assignmentExpression (COMMA assignmentExpression)* */
DSLExpression DSLParser::expression() {
[[maybe_unused]] Token start = this->peek();
@ -1361,18 +1376,8 @@ DSLExpression DSLParser::expression() {
}
Token t;
AutoDSLDepth depth(this);
while (this->checkNext(Token::Kind::TK_COMMA, &t)) {
if (!depth.increase()) {
return {};
}
DSLExpression right = this->assignmentExpression();
if (!right.hasValue()) {
return {};
}
Position pos = result.position().rangeThrough(right.position());
DSLExpression next = DSLExpression(dsl::operator,(std::move(result), std::move(right)),
pos);
result.swap(next);
while (this->peek().fKind == Token::Kind::TK_COMMA) {
OPERATOR_RIGHT(COMMA, assignmentExpression);
}
SkASSERTF(result.position().valid(), "Expression %s has invalid position",
result.description().c_str());
@ -1386,21 +1391,6 @@ DSLExpression DSLParser::expression() {
return result;
}
#define OPERATOR_RIGHT(op, exprType) \
do { \
this->nextToken(); \
if (!depth.increase()) { \
return {}; \
} \
DSLExpression right = this->exprType(); \
if (!right.hasValue()) { \
return {}; \
} \
Position pos = result.position().rangeThrough(right.position()); \
DSLExpression next = DSLExpression(std::move(result) op std::move(right), pos); \
result.swap(next); \
} while (false)
/* ternaryExpression ((EQEQ | STAREQ | SLASHEQ | PERCENTEQ | PLUSEQ | MINUSEQ | SHLEQ | SHREQ |
BITWISEANDEQ | BITWISEXOREQ | BITWISEOREQ | LOGICALANDEQ | LOGICALXOREQ | LOGICALOREQ)
assignmentExpression)*
@ -1413,17 +1403,39 @@ DSLExpression DSLParser::assignmentExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_EQ: OPERATOR_RIGHT(=, assignmentExpression); break;
case Token::Kind::TK_STAREQ: OPERATOR_RIGHT(*=, assignmentExpression); break;
case Token::Kind::TK_SLASHEQ: OPERATOR_RIGHT(/=, assignmentExpression); break;
case Token::Kind::TK_PERCENTEQ: OPERATOR_RIGHT(%=, assignmentExpression); break;
case Token::Kind::TK_PLUSEQ: OPERATOR_RIGHT(+=, assignmentExpression); break;
case Token::Kind::TK_MINUSEQ: OPERATOR_RIGHT(-=, assignmentExpression); break;
case Token::Kind::TK_SHLEQ: OPERATOR_RIGHT(<<=, assignmentExpression); break;
case Token::Kind::TK_SHREQ: OPERATOR_RIGHT(>>=, assignmentExpression); break;
case Token::Kind::TK_BITWISEANDEQ: OPERATOR_RIGHT(&=, assignmentExpression); break;
case Token::Kind::TK_BITWISEXOREQ: OPERATOR_RIGHT(^=, assignmentExpression); break;
case Token::Kind::TK_BITWISEOREQ: OPERATOR_RIGHT(|=, assignmentExpression); break;
case Token::Kind::TK_EQ:
OPERATOR_RIGHT(EQ, assignmentExpression);
break;
case Token::Kind::TK_STAREQ:
OPERATOR_RIGHT(STAREQ,assignmentExpression);
break;
case Token::Kind::TK_SLASHEQ:
OPERATOR_RIGHT(SLASHEQ, assignmentExpression);
break;
case Token::Kind::TK_PERCENTEQ:
OPERATOR_RIGHT(PERCENTEQ, assignmentExpression);
break;
case Token::Kind::TK_PLUSEQ:
OPERATOR_RIGHT(PLUSEQ, assignmentExpression);
break;
case Token::Kind::TK_MINUSEQ:
OPERATOR_RIGHT(MINUSEQ, assignmentExpression);
break;
case Token::Kind::TK_SHLEQ:
OPERATOR_RIGHT(SHLEQ, assignmentExpression);
break;
case Token::Kind::TK_SHREQ:
OPERATOR_RIGHT(SHREQ, assignmentExpression);
break;
case Token::Kind::TK_BITWISEANDEQ:
OPERATOR_RIGHT(BITWISEANDEQ, assignmentExpression);
break;
case Token::Kind::TK_BITWISEXOREQ:
OPERATOR_RIGHT(BITWISEXOREQ, assignmentExpression);
break;
case Token::Kind::TK_BITWISEOREQ:
OPERATOR_RIGHT(BITWISEOREQ, assignmentExpression);
break;
default:
return result;
}
@ -1466,7 +1478,7 @@ DSLExpression DSLParser::logicalOrExpression() {
return {};
}
while (this->peek().fKind == Token::Kind::TK_LOGICALOR) {
OPERATOR_RIGHT(||, logicalXorExpression);
OPERATOR_RIGHT(LOGICALOR, logicalXorExpression);
}
return result;
}
@ -1478,17 +1490,8 @@ DSLExpression DSLParser::logicalXorExpression() {
if (!result.hasValue()) {
return {};
}
while (this->checkNext(Token::Kind::TK_LOGICALXOR)) {
if (!depth.increase()) {
return {};
}
DSLExpression right = this->logicalAndExpression();
if (!right.hasValue()) {
return {};
}
Position pos = result.position().rangeThrough(right.position());
DSLExpression next(LogicalXor(std::move(result), std::move(right)), pos);
result.swap(next);
while (this->peek().fKind == Token::Kind::TK_LOGICALXOR) {
OPERATOR_RIGHT(LOGICALXOR, logicalAndExpression);
}
return result;
}
@ -1501,7 +1504,7 @@ DSLExpression DSLParser::logicalAndExpression() {
return {};
}
while (this->peek().fKind == Token::Kind::TK_LOGICALAND) {
OPERATOR_RIGHT(&&, bitwiseOrExpression);
OPERATOR_RIGHT(LOGICALAND, bitwiseOrExpression);
}
return result;
}
@ -1514,7 +1517,7 @@ DSLExpression DSLParser::bitwiseOrExpression() {
return {};
}
while (this->peek().fKind == Token::Kind::TK_BITWISEOR) {
OPERATOR_RIGHT(|, bitwiseXorExpression);
OPERATOR_RIGHT(BITWISEOR, bitwiseXorExpression);
}
return result;
}
@ -1527,7 +1530,7 @@ DSLExpression DSLParser::bitwiseXorExpression() {
return {};
}
while (this->peek().fKind == Token::Kind::TK_BITWISEXOR) {
OPERATOR_RIGHT(^, bitwiseAndExpression);
OPERATOR_RIGHT(BITWISEXOR, bitwiseAndExpression);
}
return result;
}
@ -1540,7 +1543,7 @@ DSLExpression DSLParser::bitwiseAndExpression() {
return {};
}
while (this->peek().fKind == Token::Kind::TK_BITWISEAND) {
OPERATOR_RIGHT(&, equalityExpression);
OPERATOR_RIGHT(BITWISEAND, equalityExpression);
}
return result;
}
@ -1554,8 +1557,8 @@ DSLExpression DSLParser::equalityExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_EQEQ: OPERATOR_RIGHT(==, relationalExpression); break;
case Token::Kind::TK_NEQ: OPERATOR_RIGHT(!=, relationalExpression); break;
case Token::Kind::TK_EQEQ: OPERATOR_RIGHT(EQEQ, relationalExpression); break;
case Token::Kind::TK_NEQ: OPERATOR_RIGHT(NEQ, relationalExpression); break;
default: return result;
}
}
@ -1570,10 +1573,10 @@ DSLExpression DSLParser::relationalExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_LT: OPERATOR_RIGHT(<, shiftExpression); break;
case Token::Kind::TK_GT: OPERATOR_RIGHT(>, shiftExpression); break;
case Token::Kind::TK_LTEQ: OPERATOR_RIGHT(<=, shiftExpression); break;
case Token::Kind::TK_GTEQ: OPERATOR_RIGHT(>=, shiftExpression); break;
case Token::Kind::TK_LT: OPERATOR_RIGHT(LT, shiftExpression); break;
case Token::Kind::TK_GT: OPERATOR_RIGHT(GT, shiftExpression); break;
case Token::Kind::TK_LTEQ: OPERATOR_RIGHT(LTEQ, shiftExpression); break;
case Token::Kind::TK_GTEQ: OPERATOR_RIGHT(GTEQ, shiftExpression); break;
default: return result;
}
}
@ -1588,8 +1591,8 @@ DSLExpression DSLParser::shiftExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_SHL: OPERATOR_RIGHT(<<, additiveExpression); break;
case Token::Kind::TK_SHR: OPERATOR_RIGHT(>>, additiveExpression); break;
case Token::Kind::TK_SHL: OPERATOR_RIGHT(SHL, additiveExpression); break;
case Token::Kind::TK_SHR: OPERATOR_RIGHT(SHR, additiveExpression); break;
default: return result;
}
}
@ -1604,8 +1607,8 @@ DSLExpression DSLParser::additiveExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_PLUS: OPERATOR_RIGHT(+, multiplicativeExpression); break;
case Token::Kind::TK_MINUS: OPERATOR_RIGHT(-, multiplicativeExpression); break;
case Token::Kind::TK_PLUS: OPERATOR_RIGHT(PLUS, multiplicativeExpression); break;
case Token::Kind::TK_MINUS: OPERATOR_RIGHT(MINUS, multiplicativeExpression); break;
default: return result;
}
}
@ -1620,9 +1623,9 @@ DSLExpression DSLParser::multiplicativeExpression() {
}
for (;;) {
switch (this->peek().fKind) {
case Token::Kind::TK_STAR: OPERATOR_RIGHT(*, unaryExpression); break;
case Token::Kind::TK_SLASH: OPERATOR_RIGHT(/, unaryExpression); break;
case Token::Kind::TK_PERCENT: OPERATOR_RIGHT(%, unaryExpression); break;
case Token::Kind::TK_STAR: OPERATOR_RIGHT(STAR, unaryExpression); break;
case Token::Kind::TK_SLASH: OPERATOR_RIGHT(SLASH, unaryExpression); break;
case Token::Kind::TK_PERCENT: OPERATOR_RIGHT(PERCENT, unaryExpression); break;
default: return result;
}
}

View File

@ -250,6 +250,12 @@ DSLExpression DSLExpression::postfix(Operator::Kind op, Position pos) {
return DSLExpression(std::move(result), pos);
}
DSLExpression DSLExpression::binary(Operator::Kind op, DSLExpression right, Position pos) {
std::unique_ptr<SkSL::Expression> result = BinaryExpression::Convert(ThreadContext::Context(),
pos, this->release(), op, right.release());
return DSLExpression(std::move(result), pos);
}
#define OP(op, token) \
DSLPossibleExpression operator op(DSLExpression left, DSLExpression right) { \
return BinaryExpression::Convert(ThreadContext::Context(), Position(), left.release(), \