Improve error reporting for invalid var-decls.

We now interpret any statement of the form `Type identifier...` as a
var-declaration and report errors as such. Previously, if a var-decl
statement generated an error during parse, we'd report errors as if it
were an expression-statement, which meant that slightly-invalid code
could return out-of-context, misleading errors.

Bug: skia:11287
Change-Id: I2c6cf2984760eb34593c80cb30f8c4e007d42027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370036
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
John Stiles 2021-02-12 16:05:00 -05:00
parent 6e73404a78
commit 8dabeac58c
5 changed files with 33 additions and 12 deletions

View File

@ -131,6 +131,7 @@ sksl_error_tests = [
"/sksl/errors/OverflowIntLiteral.sksl",
"/sksl/errors/OverflowParamArraySize.sksl",
"/sksl/errors/OverflowUintLiteral.sksl",
"/sksl/errors/PrototypeInFuncBody.sksl",
"/sksl/errors/PrivateTypes.sksl",
"/sksl/errors/RedeclareBasicType.sksl",
"/sksl/errors/RedeclareEnum.sksl",

View File

@ -0,0 +1,3 @@
void main() {
float x();
}

View File

@ -534,9 +534,9 @@ ASTNode::ID Parser::varDeclarationsOrExpressionStatement() {
// 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.
Checkpoint checkpoint(this);
ASTNode::ID node = this->varDeclarations();
if (node) {
return node;
VarDeclarationsPrefix prefix;
if (this->varDeclarationsPrefix(&prefix)) {
return this->varDeclarationEnd(prefix.modifiers, prefix.type, this->text(prefix.name));
}
// If this statement wasn't actually a vardecl after all, rewind and try parsing it as an
@ -547,19 +547,24 @@ ASTNode::ID Parser::varDeclarationsOrExpressionStatement() {
return this->expressionStatement();
}
// Helper function for varDeclarations(). If this function succeeds, we assume that the rest of the
// statement is a variable-declaration statement, not an expression-statement.
bool Parser::varDeclarationsPrefix(VarDeclarationsPrefix* prefixData) {
prefixData->modifiers = this->modifiers();
prefixData->type = this->type();
if (!prefixData->type) {
return false;
}
return this->expectIdentifier(&prefixData->name);
}
/* modifiers type IDENTIFIER varDeclarationEnd */
ASTNode::ID Parser::varDeclarations() {
Checkpoint checkpoint(this);
Modifiers modifiers = this->modifiers();
ASTNode::ID type = this->type();
if (!type) {
VarDeclarationsPrefix prefix;
if (!this->varDeclarationsPrefix(&prefix)) {
return ASTNode::ID::Invalid();
}
Token name;
if (!this->expectIdentifier(&name)) {
return ASTNode::ID::Invalid();
}
return this->varDeclarationEnd(modifiers, type, this->text(name));
return this->varDeclarationEnd(prefix.modifiers, prefix.type, this->text(prefix.name));
}
/* STRUCT IDENTIFIER LBRACE varDeclaration* RBRACE */

View File

@ -172,6 +172,14 @@ private:
ASTNode::ID declaration();
struct VarDeclarationsPrefix {
Modifiers modifiers;
ASTNode::ID type;
Token name;
};
bool varDeclarationsPrefix(VarDeclarationsPrefix* prefixData);
ASTNode::ID varDeclarationsOrExpressionStatement();
ASTNode::ID varDeclarations();

View File

@ -0,0 +1,4 @@
### Compilation failed:
error: 2: expected ';', but found '('
1 error