AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Simplify parsing of for init-statements.

Browse Source 
The existing logic was written before `varDeclarationsOrExpression-
Statement` existed. The new logic mirrors how this->statement() would
behave, except that it doesn't even try to parse keywords that can't be
used in an init-stmt (like do, if, switch, while, etc.)

Separately, I've tried just calling `this->statement()` directly, and it
does work, but it is too permissive--statements that can compile away
to a no-op (like `switch(0) {}`) suddenly become valid inside a for
init-statement. This is probably solvable but there's not much of a
reason to investigate further when the parser can catch these already.
DSL doesn't go through the parser, but we're willing to accept this
weak spot for DSL since it is always safe; either we catch and report
the error, or the code was meaningless and optimizes away to nothing.

Change-Id: I44779ec39b30cf958255534d3d5c5eb3d71d4023
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398227
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
John Stiles 2021-04-19 14:57:27 -04:00 committed by Skia Commit-Bot
parent 6475638790
commit 89b484aee3
1 changed files with 18 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/sksl/SkSLParser.cpp
View File

@ -512,7 +512,14 @@ ASTNode::ID Parser::declaration() {
/* (varDeclarations | expressionStatement) */
ASTNode::ID Parser::varDeclarationsOrExpressionStatement() {
if (this->isType(this->text(this->peek()))) {
Token nextToken = this->peek();
if (nextToken.fKind == Token::Kind::TK_CONST) {
// Statements that begin with `const` might be variable declarations, but can't be legal
// SkSL expression-statements. (SkSL constructors don't take a `const` modifier.)
return this->varDeclarations();
}
if (this->isType(this->text(nextToken))) {
// Statements that begin with a typename are most often variable declarations, but
// occasionally the type is part of a constructor, and these are actually expression-
// statements in disguise. First, attempt the common case: parse it as a vardecl.
@ -1074,7 +1081,6 @@ ASTNode::ID Parser::statement() {
this->nextToken();
return this->createNode(start.fOffset, ASTNode::Kind::kBlock);
case Token::Kind::TK_CONST:
return this->varDeclarations();
case Token::Kind::TK_IDENTIFIER:
return this->varDeclarationsOrExpressionStatement();
default:
@ -1362,38 +1368,18 @@ ASTNode::ID Parser::forStatement() {
return ASTNode::ID::Invalid();
}
ASTNode::ID result = this->createNode(start.fOffset, ASTNode::Kind::kFor);
ASTNode::ID initializer;
Token nextToken = this->peek();
switch (nextToken.fKind) {
case Token::Kind::TK_SEMICOLON:
this->nextToken();
this->createEmptyChild(result);
break;
case Token::Kind::TK_CONST: {
initializer = this->varDeclarations();
if (!initializer) {
return ASTNode::ID::Invalid();
}
getNode(result).addChild(initializer);
break;
if (nextToken.fKind == Token::Kind::TK_SEMICOLON) {
// An empty init-statement.
this->nextToken();
this->createEmptyChild(result);
} else {
// The init-statement must be an expression or variable declaration.
ASTNode::ID initializer = this->varDeclarationsOrExpressionStatement();
if (!initializer) {
return ASTNode::ID::Invalid();
}
case Token::Kind::TK_IDENTIFIER: {
if (this->isType(this->text(nextToken))) {
initializer = this->varDeclarations();
if (!initializer) {
return ASTNode::ID::Invalid();
}
getNode(result).addChild(initializer);
break;
}
[[fallthrough]];
}
default:
initializer = this->expressionStatement();
if (!initializer) {
return ASTNode::ID::Invalid();
}
getNode(result).addChild(initializer);
getNode(result).addChild(initializer);
}
ASTNode::ID test;
if (this->peek().fKind != Token::Kind::TK_SEMICOLON) {