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

Moved DetectVarDeclarationWithoutScope out of IRGenerator

Browse Source 
As these checks were in the AST processing side of things, they were not
catching errors in DSL code. SkSLAnalysis is also a better home for this
function than SkSLIRGenerator.

Change-Id: I8149825047570300cc09425deba1339ac0edb7ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444656
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This commit is contained in:
Ethan Nicholas 2021-09-01 14:37:29 -04:00 committed by SkCQ
parent facffbe093
commit 98eae1ec2e
7 changed files with 59 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/sksl/SkSLAnalysis.cpp
View File

@ -722,6 +722,38 @@ bool Analysis::DetectStaticRecursion(SkSpan<std::unique_ptr<ProgramElement>> pro
return false;
}
bool Analysis::DetectVarDeclarationWithoutScope(const Statement& stmt, ErrorReporter* errors) {
// A variable declaration can create either a lone VarDeclaration or an unscoped Block
// containing multiple VarDeclaration statements. We need to detect either case.
const Variable* var;
if (stmt.is<VarDeclaration>()) {
// The single-variable case. No blocks at all.
var = &stmt.as<VarDeclaration>().var();
} else if (stmt.is<Block>()) {
// The multiple-variable case: an unscoped, non-empty block...
const Block& block = stmt.as<Block>();
if (block.isScope() || block.children().empty()) {
return false;
}
// ... holding a variable declaration.
const Statement& innerStmt = *block.children().front();
if (!innerStmt.is<VarDeclaration>()) {
return false;
}
var = &innerStmt.as<VarDeclaration>().var();
} else {
// This statement wasn't a variable declaration. No problem.
return false;
}
// Report an error.
SkASSERT(var);
if (errors) {
errors->error(stmt.fOffset, "variable '" + var->name() + "' must be created in a scope");
}
return true;
}
int Analysis::NodeCountUpToLimit(const FunctionDefinition& function, int limit) {
return NodeCountVisitor{limit}.visit(*function.body());
}

7
src/sksl/SkSLAnalysis.h
View File

@ -60,6 +60,13 @@ struct Analysis {
static bool DetectStaticRecursion(SkSpan<std::unique_ptr<ProgramElement>> programElements,
ErrorReporter& errors);
/**
* Detect an orphaned variable declaration outside of a scope, e.g. if (true) int a;. Returns
* true if an error was reported.
*/
static bool DetectVarDeclarationWithoutScope(const Statement& stmt,
ErrorReporter* errors = nullptr);
static int NodeCountUpToLimit(const FunctionDefinition& function, int limit);
/**

47
src/sksl/SkSLIRGenerator.cpp
View File

@ -91,38 +91,6 @@ void IRGenerator::popSymbolTable() {
fSymbolTable = fSymbolTable->fParent;
}
bool IRGenerator::detectVarDeclarationWithoutScope(const Statement& stmt) {
// Parsing an AST node containing a single variable declaration creates a lone VarDeclaration
// statement. An AST with multiple variable declarations creates an unscoped Block containing
// multiple VarDeclaration statements. We need to detect either case.
const Variable* var;
if (stmt.is<VarDeclaration>()) {
// The single-variable case. No blocks at all.
var = &stmt.as<VarDeclaration>().var();
} else if (stmt.is<Block>()) {
// The multiple-variable case: an unscoped, non-empty block...
const Block& block = stmt.as<Block>();
if (block.isScope() || block.children().empty()) {
return false;
}
// ... holding a variable declaration.
const Statement& innerStmt = *block.children().front();
if (!innerStmt.is<VarDeclaration>()) {
return false;
}
var = &innerStmt.as<VarDeclaration>().var();
} else {
// This statement wasn't a variable declaration. No problem.
return false;
}
// Report an error.
SkASSERT(var);
this->errorReporter().error(stmt.fOffset,
"variable '" + var->name() + "' must be created in a scope");
return true;
}
std::unique_ptr<Extension> IRGenerator::convertExtension(int offset, skstd::string_view name) {
if (this->programKind() != ProgramKind::kFragment &&
this->programKind() != ProgramKind::kVertex) {
@ -416,18 +384,12 @@ std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
if (!ifTrue) {
return nullptr;
}
if (this->detectVarDeclarationWithoutScope(*ifTrue)) {
return nullptr;
}
std::unique_ptr<Statement> ifFalse;
if (iter != n.end()) {
ifFalse = this->convertStatement(*(iter++));
if (!ifFalse) {
return nullptr;
}
if (this->detectVarDeclarationWithoutScope(*ifFalse)) {
return nullptr;
}
}
bool isStatic = n.getBool();
return IfStatement::Convert(fContext, n.fOffset, isStatic, std::move(test),
@ -466,9 +428,6 @@ std::unique_ptr<Statement> IRGenerator::convertFor(const ASTNode& f) {
if (!statement) {
return nullptr;
}
if (this->detectVarDeclarationWithoutScope(*statement)) {
return nullptr;
}
return ForStatement::Convert(fContext, f.fOffset, std::move(initializer), std::move(test),
std::move(next), std::move(statement), fSymbolTable);
@ -485,9 +444,6 @@ std::unique_ptr<Statement> IRGenerator::convertWhile(const ASTNode& w) {
if (!statement) {
return nullptr;
}
if (this->detectVarDeclarationWithoutScope(*statement)) {
return nullptr;
}
return ForStatement::ConvertWhile(fContext, w.fOffset, std::move(test), std::move(statement),
fSymbolTable);
}
@ -503,9 +459,6 @@ std::unique_ptr<Statement> IRGenerator::convertDo(const ASTNode& d) {
if (!test) {
return nullptr;
}
if (this->detectVarDeclarationWithoutScope(*statement)) {
return nullptr;
}
return DoStatement::Convert(fContext, std::move(statement), std::move(test));
}

1
src/sksl/SkSLIRGenerator.h
View File

@ -233,7 +233,6 @@ private:
bool setRefKind(Expression& expr, VariableReference::RefKind kind);
void copyIntrinsicIfNeeded(const FunctionDeclaration& function);
void findAndDeclareBuiltinVariables();
bool detectVarDeclarationWithoutScope(const Statement& stmt);
// Runtime effects (and the interpreter, which uses the same CPU runtime) require adherence to
// the strict rules from The OpenGL ES Shading Language Version 1.00. (Including Appendix A).

5
src/sksl/ir/SkSLDoStatement.cpp
View File

@ -5,6 +5,7 @@
* found in the LICENSE file.
*/
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLContext.h"
#include "src/sksl/SkSLProgramSettings.h"
#include "src/sksl/ir/SkSLDoStatement.h"
@ -22,6 +23,9 @@ std::unique_ptr<Statement> DoStatement::Convert(const Context& context,
if (!test) {
return nullptr;
}
if (Analysis::DetectVarDeclarationWithoutScope(*stmt, context.fErrors)) {
return nullptr;
}
return DoStatement::Make(context, std::move(stmt), std::move(test));
}
@ -30,6 +34,7 @@ std::unique_ptr<Statement> DoStatement::Make(const Context& context,
std::unique_ptr<Expression> test) {
SkASSERT(!context.fConfig->strictES2Mode());
SkASSERT(test->type() == *context.fTypes.fBool);
SkASSERT(!Analysis::DetectVarDeclarationWithoutScope(*stmt));
return std::make_unique<DoStatement>(stmt->fOffset, std::move(stmt), std::move(test));
}

5
src/sksl/ir/SkSLForStatement.cpp
View File

@ -132,6 +132,10 @@ std::unique_ptr<Statement> ForStatement::Convert(const Context& context, int off
return Block::Make(offset, std::move(scope));
}
if (Analysis::DetectVarDeclarationWithoutScope(*statement, context.fErrors)) {
return nullptr;
}
return ForStatement::Make(context, offset, std::move(initializer), std::move(test),
std::move(next), std::move(statement), std::move(unrollInfo),
std::move(symbolTable));
@ -159,6 +163,7 @@ std::unique_ptr<Statement> ForStatement::Make(const Context& context, int offset
SkASSERT(is_simple_initializer(initializer.get()) ||
is_vardecl_block_initializer(initializer.get()));
SkASSERT(!test || test->type() == *context.fTypes.fBool);
SkASSERT(!Analysis::DetectVarDeclarationWithoutScope(*statement));
// If the caller didn't provide us with unroll info, we can compute it here if needed.
if (!unrollInfo && context.fConfig->strictES2Mode()) {

10
src/sksl/ir/SkSLIfStatement.cpp
View File

@ -5,6 +5,7 @@
* found in the LICENSE file.
*/
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLConstantFolder.h"
#include "src/sksl/SkSLContext.h"
#include "src/sksl/SkSLProgramSettings.h"
@ -42,6 +43,13 @@ std::unique_ptr<Statement> IfStatement::Convert(const Context& context, int offs
if (!test) {
return nullptr;
}
SkASSERT(ifTrue);
if (Analysis::DetectVarDeclarationWithoutScope(*ifTrue, context.fErrors)) {
return nullptr;
}
if (ifFalse && Analysis::DetectVarDeclarationWithoutScope(*ifFalse, context.fErrors)) {
return nullptr;
}
return IfStatement::Make(context, offset, isStatic, std::move(test),
std::move(ifTrue), std::move(ifFalse));
}
@ -57,6 +65,8 @@ std::unique_ptr<Statement> IfStatement::Make(const Context& context, int offset,
std::unique_ptr<Statement> ifTrue,
std::unique_ptr<Statement> ifFalse) {
SkASSERT(test->type() == *context.fTypes.fBool);
SkASSERT(!Analysis::DetectVarDeclarationWithoutScope(*ifTrue));
SkASSERT(!ifFalse || !Analysis::DetectVarDeclarationWithoutScope(*ifFalse));
const bool optimize = context.fConfig->fSettings.fOptimize;
bool trueIsEmpty = false;