Further unified error handling between SkSL and DSL
This eliminates the SkSL ErrorReporter class and funnels everything
through the DSL ErrorHandler. Since the DSL error handler can be
changed, this required a number of updates to ensure that things work
properly in the face of custom error handlers. There is probably more
work to be done in that area, but this at least passes all existing
tests.
Change-Id: Iaee27b79fc4ed426c484ccab257c09d28619ead5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438116
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 6900509..89e5dad 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -84,12 +84,12 @@
public:
AutoSource(Compiler* compiler, const char* source)
: fCompiler(compiler) {
- SkASSERT(!fCompiler->fSource);
- fCompiler->fSource = source;
+ SkASSERT(!fCompiler->errorReporter().source());
+ fCompiler->errorReporter().setSource(source);
}
~AutoSource() {
- fCompiler->fSource = nullptr;
+ fCompiler->errorReporter().setSource(nullptr);
}
Compiler* fCompiler;
@@ -98,16 +98,17 @@
class AutoProgramConfig {
public:
AutoProgramConfig(std::shared_ptr<Context>& context, ProgramConfig* config)
- : fContext(context.get()) {
- SkASSERT(!fContext->fConfig);
+ : fContext(context.get())
+ , fOldConfig(fContext->fConfig) {
fContext->fConfig = config;
}
~AutoProgramConfig() {
- fContext->fConfig = nullptr;
+ fContext->fConfig = fOldConfig;
}
Context* fContext;
+ ProgramConfig* fOldConfig;
};
class AutoModifiersPool {
@@ -126,7 +127,8 @@
};
Compiler::Compiler(const ShaderCapsClass* caps)
- : fContext(std::make_shared<Context>(/*errors=*/*this, *caps))
+ : fErrorReporter(this)
+ , fContext(std::make_shared<Context>(fErrorReporter, *caps))
, fInliner(fContext.get()) {
SkASSERT(caps);
fRootModule.fSymbols = this->makeRootSymbolTable();
@@ -139,7 +141,7 @@
#define TYPE(t) fContext->fTypes.f ## t .get()
std::shared_ptr<SymbolTable> Compiler::makeRootSymbolTable() {
- auto rootSymbolTable = std::make_shared<SymbolTable>(this, /*builtin=*/true);
+ auto rootSymbolTable = std::make_shared<SymbolTable>(&this->errorReporter(), /*builtin=*/true);
const SkSL::Symbol* rootTypes[] = {
TYPE(Void),
@@ -340,6 +342,7 @@
settings.fReplaceSettings = !dehydrate;
#if defined(SKSL_STANDALONE)
+ SkASSERT(this->errorCount() == 0);
SkASSERT(data.fPath);
std::ifstream in(data.fPath);
String text{std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>()};
@@ -353,16 +356,17 @@
std::vector<std::unique_ptr<ProgramElement>> elements;
std::vector<const ProgramElement*> sharedElements;
dsl::StartModule(this, kind, settings, baseModule);
+ dsl::SetErrorReporter(&this->errorReporter());
AutoSource as(this, source->c_str());
IRGenerator::IRBundle ir = fIRGenerator->convertProgram(baseModule, /*isBuiltinCode=*/true,
*source);
SkASSERT(ir.fSharedElements.empty());
LoadedModule module = { kind, std::move(ir.fSymbolTable), std::move(ir.fElements) };
- dsl::End();
- if (this->fErrorCount) {
+ if (this->errorCount()) {
printf("Unexpected errors: %s\n", this->fErrorText.c_str());
SkDEBUGFAILF("%s %s\n", data.fPath, this->fErrorText.c_str());
}
+ dsl::End();
#else
ProgramConfig config;
config.fKind = kind;
@@ -474,8 +478,7 @@
settings.fAllowNarrowingConversions = true;
}
- fErrorText = "";
- fErrorCount = 0;
+ this->resetErrors();
fInliner.reset();
#if SKSL_DSL_PARSER
@@ -486,7 +489,7 @@
AutoSource as(this, textPtr->c_str());
dsl::Start(this, kind, settings);
- dsl::SetErrorHandler(this);
+ dsl::SetErrorReporter(&fErrorReporter);
IRGenerator::IRBundle ir = fIRGenerator->convertProgram(baseModule, /*isBuiltinCode=*/false,
*textPtr);
// Ideally, we would just use dsl::ReleaseProgram and not have to do any manual mucking about
@@ -501,8 +504,9 @@
std::move(ir.fSymbolTable),
std::move(dsl::DSLWriter::MemoryPool()),
ir.fInputs);
+ this->errorReporter().reportPendingErrors(PositionInfo());
bool success = false;
- if (fErrorCount) {
+ if (this->errorCount()) {
// Do not return programs that failed to compile.
} else if (!this->optimize(*program)) {
// Do not return programs that failed to optimize.
@@ -561,7 +565,7 @@
}
// Check all of the program's owned elements. (Built-in elements are assumed to be valid.)
- StaticTestVerifier visitor{this};
+ StaticTestVerifier visitor{&this->errorReporter()};
for (const std::unique_ptr<ProgramElement>& element : program.ownedElements()) {
if (element->is<FunctionDefinition>()) {
visitor.visitProgramElement(*element);
@@ -570,7 +574,7 @@
}
bool Compiler::optimize(LoadedModule& module) {
- SkASSERT(!fErrorCount);
+ SkASSERT(!this->errorCount());
// Create a temporary program configuration with default settings.
ProgramConfig config;
@@ -582,13 +586,13 @@
std::unique_ptr<ProgramUsage> usage = Analysis::GetUsage(module);
- while (fErrorCount == 0) {
+ while (this->errorCount() == 0) {
// Perform inline-candidate analysis and inline any functions deemed suitable.
if (!fInliner.analyze(module.fElements, module.fSymbols, usage.get())) {
break;
}
}
- return fErrorCount == 0;
+ return this->errorCount() == 0;
}
bool Compiler::removeDeadFunctions(Program& program, ProgramUsage* usage) {
@@ -864,10 +868,10 @@
return true;
}
- SkASSERT(!fErrorCount);
+ SkASSERT(!this->errorCount());
ProgramUsage* usage = program.fUsage.get();
- if (fErrorCount == 0) {
+ if (this->errorCount() == 0) {
// Run the inliner only once; it is expensive! Multiple passes can occasionally shake out
// more wins, but it's diminishing returns.
fInliner.analyze(program.ownedElements(), program.fSymbols, usage);
@@ -884,11 +888,11 @@
this->removeDeadGlobalVariables(program, usage);
}
- if (fErrorCount == 0) {
+ if (this->errorCount() == 0) {
this->verifyStaticTests(program);
}
- return fErrorCount == 0;
+ return this->errorCount() == 0;
}
#if defined(SKSL_STANDALONE) || SK_SUPPORT_GPU
@@ -899,6 +903,7 @@
ProgramSettings settings;
settings.fDSLUseMemoryPool = false;
dsl::Start(this, program.fConfig->fKind, settings);
+ dsl::SetErrorReporter(&fErrorReporter);
dsl::DSLWriter::IRGenerator().fSymbolTable = program.fSymbols;
#ifdef SK_ENABLE_SPIRV_VALIDATION
StringStream buffer;
@@ -927,7 +932,7 @@
if (tools.Disassemble((const uint32_t*)data.data(), data.size() / 4, &disassembly)) {
errors.append(disassembly);
}
- this->error(-1, errors);
+ this->errorReporter().error(-1, errors);
#else
SkDEBUGFAILF("%s", errors.c_str());
#endif
@@ -996,29 +1001,29 @@
#endif // defined(SKSL_STANDALONE) || SK_SUPPORT_GPU
-void Compiler::handleError(const char* msg, dsl::PositionInfo pos) {
+void Compiler::handleError(const char* msg, PositionInfo pos) {
if (strstr(msg, POISON_TAG)) {
// don't report errors on poison values
return;
}
- fErrorCount++;
fErrorText += "error: " + (pos.line() >= 1 ? to_string(pos.line()) + ": " : "") + msg + "\n";
}
String Compiler::errorText(bool showCount) {
+ this->errorReporter().reportPendingErrors(PositionInfo());
if (showCount) {
this->writeErrorCount();
}
- fErrorCount = 0;
String result = fErrorText;
- fErrorText = "";
+ this->resetErrors();
return result;
}
void Compiler::writeErrorCount() {
- if (fErrorCount) {
- fErrorText += to_string(fErrorCount) + " error";
- if (fErrorCount > 1) {
+ int count = this->errorCount();
+ if (count) {
+ fErrorText += to_string(count) + " error";
+ if (count > 1) {
fErrorText += "s";
}
fErrorText += "\n";