Fixed DSLParser code rot
Where DSLParser is #ifdef'ed out, it had experienced some code rot and
was no longer building. This fixes it to be able to successfully build
when SKSL_DSL_PARSER is set to 1.
Change-Id: I5846b05ba3342b5b1502663577663e332c55529b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442976
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/include/sksl/DSLType.h b/include/sksl/DSLType.h
index 40e93bf..73c601c 100644
--- a/include/sksl/DSLType.h
+++ b/include/sksl/DSLType.h
@@ -75,6 +75,7 @@
kUShort3_Type,
kUShort4_Type,
kVoid_Type,
+ kPoison_Type,
};
class DSLType {
@@ -163,7 +164,7 @@
const SkSL::Type* fSkSLType = nullptr;
- TypeConstant fTypeConstant;
+ TypeConstant fTypeConstant = kPoison_Type;
friend DSLType Array(const DSLType& base, int count);
friend DSLType Struct(skstd::string_view name, SkSpan<DSLField> fields);
diff --git a/include/sksl/DSLVar.h b/include/sksl/DSLVar.h
index 4f75eef..1b9250b 100644
--- a/include/sksl/DSLVar.h
+++ b/include/sksl/DSLVar.h
@@ -128,6 +128,8 @@
skstd::string_view fRawName; // for error reporting
skstd::string_view fName;
DSLExpression fInitialValue;
+ // true if we have attempted to create the SkSL var
+ bool fInitialized = false;
bool fDeclared = false;
friend class DSLCore;
diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp
index 4596119..366735d 100644
--- a/src/sksl/SkSLDSLParser.cpp
+++ b/src/sksl/SkSLDSLParser.cpp
@@ -193,43 +193,21 @@
}
void DSLParser::error(int offset, String msg) {
- fErrorReporter->error(offset, msg);
+ GetErrorReporter().error(msg.c_str(), PositionInfo());
}
-class DSLParserErrorReporter final : public ErrorReporter {
-public:
- DSLParserErrorReporter(ErrorReporter* reporter)
- : fReporter(*reporter) {}
-
- ~DSLParserErrorReporter() override {
- for (const String& s : fErrors) {
- fReporter.error(/*offset=*/-1, s.c_str());
- }
- }
-
- void handleError(const char* msg, PositionInfo position) override {
- fErrors.push_back(msg);
- }
-
-private:
- SkTArray<String> fErrors;
- ErrorReporter& fReporter;
-
- friend class DSLParser;
-};
-
/* declaration* END_OF_FILE */
std::unique_ptr<Program> DSLParser::program() {
+ ErrorReporter* errorReporter = &fCompiler.errorReporter();
Start(&fCompiler, fKind, fSettings);
- DSLParserErrorReporter errorReporter(&fCompiler.errorReporter());
- SetErrorReporter(&errorReporter);
+ SetErrorReporter(errorReporter);
std::unique_ptr<Program> result;
bool done = false;
while (!done) {
switch (this->peek().fKind) {
case Token::Kind::TK_END_OF_FILE:
done = true;
- if (errorReporter.fErrors.empty()) {
+ if (!errorReporter->errorCount()) {
result = dsl::ReleaseProgram(std::make_unique<String>(std::move(fText)));
}
break;
@@ -535,7 +513,7 @@
"struct '" + this->text(name) + "' must contain at least one field");
return skstd::nullopt;
}
- return dsl::Struct(this->text(name), SkMakeSpan(fields.size()));
+ return dsl::Struct(this->text(name), SkMakeSpan(fields));
}
/* structDeclaration ((IDENTIFIER varDeclarationEnd) | SEMICOLON) */
@@ -802,8 +780,8 @@
this->nextToken();
SkTArray<dsl::Field> fields;
while (!this->checkNext(Token::Kind::TK_RBRACE)) {
- DSLModifiers modifiers = this->modifiers();
- skstd::optional<dsl::DSLType> type = this->type(modifiers);
+ DSLModifiers fieldModifiers = this->modifiers();
+ skstd::optional<dsl::DSLType> type = this->type(fieldModifiers);
if (!type) {
return false;
}
diff --git a/src/sksl/SkSLDSLParser.h b/src/sksl/SkSLDSLParser.h
index a291bf6..efaa621 100644
--- a/src/sksl/SkSLDSLParser.h
+++ b/src/sksl/SkSLDSLParser.h
@@ -313,6 +313,7 @@
void restoreErrorReporter() {
SkASSERT(fOldErrorReporter);
+ fErrorReporter.reportPendingErrors(PositionInfo());
dsl::SetErrorReporter(fOldErrorReporter);
fOldErrorReporter = nullptr;
}
diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp
index 16e5656..e6a5326 100644
--- a/src/sksl/dsl/DSLCore.cpp
+++ b/src/sksl/dsl/DSLCore.cpp
@@ -163,6 +163,7 @@
const SkSL::Symbol* alreadyDeclared = (*DSLWriter::SymbolTable())[var.fName];
if (alreadyDeclared && alreadyDeclared->is<Variable>()) {
var.fVar = &alreadyDeclared->as<Variable>();
+ var.fInitialized = true;
}
}
}
diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp
index 18a8892..78bafb0 100644
--- a/src/sksl/dsl/DSLFunction.cpp
+++ b/src/sksl/dsl/DSLFunction.cpp
@@ -60,6 +60,7 @@
if (fDecl) {
for (size_t i = 0; i < params.size(); ++i) {
params[i]->fVar = fDecl->parameters()[i];
+ params[i]->fInitialized = true;
}
// We don't know when this function is going to be defined; go ahead and add a prototype in
// case the definition is delayed. If we end up defining the function immediately, we'll
diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp
index 8e84881..75e16d8 100644
--- a/src/sksl/dsl/DSLType.cpp
+++ b/src/sksl/dsl/DSLType.cpp
@@ -193,6 +193,8 @@
return *context.fTypes.fUShort4;
case kVoid_Type:
return *context.fTypes.fVoid;
+ case kPoison_Type:
+ return *context.fTypes.fPoison;
default:
SkUNREACHABLE;
}
diff --git a/src/sksl/dsl/DSLVar.cpp b/src/sksl/dsl/DSLVar.cpp
index ae370f7..188c41a 100644
--- a/src/sksl/dsl/DSLVar.cpp
+++ b/src/sksl/dsl/DSLVar.cpp
@@ -90,6 +90,7 @@
std::swap(fName, other.fName);
std::swap(fInitialValue.fExpression, other.fInitialValue.fExpression);
std::swap(fDeclared, other.fDeclared);
+ std::swap(fInitialized, other.fInitialized);
}
void DSLVar::swap(DSLVar& other) {
@@ -125,12 +126,14 @@
DSLWriter::Context().fTypes.fFloat2.get(),
/*builtin=*/true,
SkSL::VariableStorage::kGlobal));
+ fInitialized = true;
return;
}
#endif
const SkSL::Symbol* result = (*DSLWriter::SymbolTable())[fName];
SkASSERTF(result, "could not find '%.*s' in symbol table", (int)fName.length(), fName.data());
fVar = &result->as<SkSL::Variable>();
+ fInitialized = true;
}
void DSLGlobalVar::swap(DSLGlobalVar& other) {
diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp
index a49d59d..d37afcb 100644
--- a/src/sksl/dsl/priv/DSLWriter.cpp
+++ b/src/sksl/dsl/priv/DSLWriter.cpp
@@ -231,10 +231,18 @@
}
const SkSL::Variable* DSLWriter::Var(DSLVarBase& var) {
- if (!var.fVar) {
+ // fInitialized is true if we have attempted to create a var, whether or not we actually
+ // succeeded. If it's true, we don't want to try again, to avoid reporting the same error
+ // multiple times.
+ if (!var.fInitialized) {
+ // We haven't even attempted to create a var yet, so fVar ought to be null
+ SkASSERT(!var.fVar);
+ var.fInitialized = true;
if (var.storage() != SkSL::VariableStorage::kParameter) {
- DSLWriter::IRGenerator().checkVarDeclaration(/*offset=*/-1, var.fModifiers.fModifiers,
- &var.fType.skslType(), var.storage());
+ DSLWriter::IRGenerator().checkVarDeclaration(/*offset=*/-1,
+ var.fModifiers.fModifiers,
+ &var.fType.skslType(),
+ var.storage());
}
std::unique_ptr<SkSL::Variable> skslvar = DSLWriter::IRGenerator().convertVar(
/*offset=*/-1,
@@ -245,19 +253,21 @@
/*arraySize=*/nullptr,
var.storage());
SkSL::Variable* varPtr = skslvar.get();
- // We can't call VarDeclaration::Convert directly here, because the IRGenerator has special
- // treatment for sk_FragColor that we want to preserve in DSL. We also do not want the
- // variable added to the symbol table for several reasons - DSLParser handles the symbol
- // table itself, parameters don't go into the symbol table until after the
- // FunctionDeclaration is created which makes this the wrong spot for them, and outside of
- // DSLParser we don't even need DSL variables to show up in the symbol table in the first
- // place.
- var.fDeclaration = DSLWriter::IRGenerator().convertVarDeclaration(
- std::move(skslvar),
- var.fInitialValue.releaseIfValid(),
- /*addToSymbolTable=*/false);
- if (var.fDeclaration) {
- var.fVar = varPtr;
+ if (var.storage() != SkSL::VariableStorage::kParameter) {
+ // We can't call VarDeclaration::Convert directly here, because the IRGenerator has
+ // special treatment for sk_FragColor that we want to preserve in DSL. We also do not
+ // want the variable added to the symbol table for several reasons - DSLParser handles
+ // the symbol table itself, parameters don't go into the symbol table until after the
+ // FunctionDeclaration is created which makes this the wrong spot for them, and outside
+ // of DSLParser we don't even need DSL variables to show up in the symbol table in the
+ // first place.
+ var.fDeclaration = DSLWriter::IRGenerator().convertVarDeclaration(
+ std::move(skslvar), var.fInitialValue.releaseIfValid(),
+ /*addToSymbolTable=*/false);
+ if (var.fDeclaration) {
+ var.fVar = varPtr;
+ var.fInitialized = true;
+ }
}
ReportErrors();
}