Rewrite some loops in CFXA_FMParser.
Resolve some TODO() about whether the loops are necessary. From looking
at the code, we've got a for-loop with a break as its last statement,
which looks unnecessary until one spots the continue statements lurking
within. So remove the continues and the final break to make this
clearer.
OTOH, the TODO() might have been referring to whether the sequence of
tokens required to loop the loop could actually occur, which does seem
possible.
Change-Id: I64778892ae175b7db6a6455c92e666c48ec82ab5
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73630
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fxfa/fm2js/cxfa_fmparser.cpp b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
index 28f3bcc..2f1cb7e 100644
--- a/xfa/fxfa/fm2js/cxfa_fmparser.cpp
+++ b/xfa/fxfa/fm2js/cxfa_fmparser.cpp
@@ -286,8 +286,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -296,22 +295,18 @@
case TOKksor: {
if (!NextToken())
return nullptr;
-
std::unique_ptr<CXFA_FMSimpleExpression> e2(
ParseLogicalAndExpression());
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMLogicalOrExpression>(TOKor, std::move(e1),
std::move(e2));
- continue;
+ break;
}
default:
- break;
+ return e1;
}
- break;
}
- return e1;
}
// LogicalAnd := EqualityExpression |
@@ -326,8 +321,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -336,21 +330,17 @@
case TOKksand: {
if (!NextToken())
return nullptr;
-
std::unique_ptr<CXFA_FMSimpleExpression> e2 = ParseEqualityExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMLogicalAndExpression>(
TOKand, std::move(e1), std::move(e2));
- continue;
+ break;
}
default:
- break;
+ return e1;
}
- break;
}
- return e1;
}
// Equality := RelationExpression |
@@ -365,8 +355,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -375,36 +364,30 @@
case TOKkseq: {
if (!NextToken())
return nullptr;
-
std::unique_ptr<CXFA_FMSimpleExpression> e2 =
ParseRelationalExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMEqualExpression>(TOKeq, std::move(e1),
std::move(e2));
- continue;
+ break;
}
case TOKne:
case TOKksne: {
if (!NextToken())
return nullptr;
-
std::unique_ptr<CXFA_FMSimpleExpression> e2 =
ParseRelationalExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMNotEqualExpression>(TOKne, std::move(e1),
std::move(e2));
- continue;
+ break;
}
default:
- break;
+ return e1;
}
- break;
}
- return e1;
}
// Relational := AdditiveExpression |
@@ -419,8 +402,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -430,56 +412,46 @@
case TOKkslt:
if (!NextToken())
return nullptr;
-
e2 = ParseAdditiveExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMLtExpression>(TOKlt, std::move(e1),
std::move(e2));
- continue;
+ break;
case TOKgt:
case TOKksgt:
if (!NextToken())
return nullptr;
-
e2 = ParseAdditiveExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMGtExpression>(TOKgt, std::move(e1),
std::move(e2));
- continue;
+ break;
case TOKle:
case TOKksle:
if (!NextToken())
return nullptr;
-
e2 = ParseAdditiveExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMLeExpression>(TOKle, std::move(e1),
std::move(e2));
- continue;
+ break;
case TOKge:
case TOKksge:
if (!NextToken())
return nullptr;
-
e2 = ParseAdditiveExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMGeExpression>(TOKge, std::move(e1),
std::move(e2));
- continue;
- default:
break;
+ default:
+ return e1;
}
- break;
}
- return e1;
}
// Additive := MultiplicativeExpression |
@@ -494,8 +466,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -504,14 +475,13 @@
case TOKplus:
if (!NextToken())
return nullptr;
-
e2 = ParseMultiplicativeExpression();
if (!e2)
return nullptr;
e1 = std::make_unique<CXFA_FMPlusExpression>(TOKplus, std::move(e1),
std::move(e2));
- continue;
+ break;
case TOKminus:
if (!NextToken())
return nullptr;
@@ -522,13 +492,11 @@
e1 = std::make_unique<CXFA_FMMinusExpression>(TOKminus, std::move(e1),
std::move(e2));
- continue;
- default:
break;
+ default:
+ return e1;
}
- break;
}
- return e1;
}
// Multiplicative := UnaryExpression |
@@ -543,8 +511,7 @@
if (!e1)
return nullptr;
- // TODO(dsinclair): Is this for() needed?
- for (;;) {
+ while (1) {
if (!IncrementParseDepthAndCheck())
return nullptr;
@@ -553,31 +520,25 @@
case TOKmul:
if (!NextToken())
return nullptr;
-
e2 = ParseUnaryExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMMulExpression>(TOKmul, std::move(e1),
std::move(e2));
- continue;
+ break;
case TOKdiv:
if (!NextToken())
return nullptr;
-
e2 = ParseUnaryExpression();
if (!e2)
return nullptr;
-
e1 = std::make_unique<CXFA_FMDivExpression>(TOKdiv, std::move(e1),
std::move(e2));
- continue;
- default:
break;
+ default:
+ return e1;
}
- break;
}
- return e1;
}
// Unary := PrimaryExpression | UnaryOperator UnaryExpression
