diff --git a/bench/SkSLBench.cpp b/bench/SkSLBench.cpp index e8f56c357a..68f2f9925c 100644 --- a/bench/SkSLBench.cpp +++ b/bench/SkSLBench.cpp @@ -140,7 +140,8 @@ protected: void onDraw(int loops, SkCanvas*) override { for (int i = 0; i < loops; i++) { fCompiler.irGenerator().pushSymbolTable(); - SkSL::Parser parser(fSrc, *fCompiler.irGenerator().symbolTable(), fCompiler); + SkSL::Parser parser(fSrc, *fCompiler.irGenerator().symbolTable(), + fCompiler.errorReporter()); parser.compilationUnit(); fCompiler.irGenerator().popSymbolTable(); if (fCompiler.errorCount()) { diff --git a/gn/sksl.gni b/gn/sksl.gni index 9c3a15b3cc..61893f6067 100644 --- a/gn/sksl.gni +++ b/gn/sksl.gni @@ -22,7 +22,6 @@ skia_sksl_sources = [ "$_include/sksl/DSLBlock.h", "$_include/sksl/DSLCase.h", "$_include/sksl/DSLCore.h", - "$_include/sksl/DSLErrorHandling.h", "$_include/sksl/DSLExpression.h", "$_include/sksl/DSLFunction.h", "$_include/sksl/DSLLayout.h", @@ -32,6 +31,7 @@ skia_sksl_sources = [ "$_include/sksl/DSLSymbols.h", "$_include/sksl/DSLType.h", "$_include/sksl/DSLVar.h", + "$_include/sksl/SkSLErrorReporter.h", "$_src/sksl/SkSLASTFile.h", "$_src/sksl/SkSLASTNode.cpp", "$_src/sksl/SkSLASTNode.h", @@ -49,6 +49,7 @@ skia_sksl_sources = [ "$_src/sksl/SkSLDSLParser.h", "$_src/sksl/SkSLDehydrator.cpp", "$_src/sksl/SkSLDehydrator.h", + "$_src/sksl/SkSLErrorReporter.cpp", "$_src/sksl/SkSLErrorReporter.h", "$_src/sksl/SkSLFileOutputStream.h", "$_src/sksl/SkSLIRGenerator.cpp", diff --git a/include/sksl/DSLCore.h b/include/sksl/DSLCore.h index 71e3dbb03e..8bbbb21698 100644 --- a/include/sksl/DSLCore.h +++ b/include/sksl/DSLCore.h @@ -12,13 +12,13 @@ #include "include/private/SkTArray.h" #include "include/sksl/DSLBlock.h" #include "include/sksl/DSLCase.h" -#include "include/sksl/DSLErrorHandling.h" #include "include/sksl/DSLExpression.h" #include "include/sksl/DSLFunction.h" #include "include/sksl/DSLStatement.h" #include "include/sksl/DSLType.h" #include "include/sksl/DSLVar.h" #include "include/sksl/DSLWrapper.h" +#include "include/sksl/SkSLErrorReporter.h" #define SKSL_DSL_PARSER 0 @@ -59,16 +59,15 @@ void End(); std::unique_ptr ReleaseProgram(std::unique_ptr source = nullptr); /** - * Returns the ErrorHandler which will be notified of any errors that occur during DSL calls. The - * default error handler is null, which means any errors encountered will be fatal. + * Returns the ErrorReporter which will be notified of any errors that occur during DSL calls. The + * default error reporter aborts on any error. */ -ErrorHandler* GetErrorHandler(); +ErrorReporter& GetErrorReporter(); /** - * Installs an ErrorHandler which will be notified of any errors that occur during DSL calls. If - * no ErrorHandler is installed, any errors will be fatal. + * Installs an ErrorReporter which will be notified of any errors that occur during DSL calls. */ -void SetErrorHandler(ErrorHandler* errorHandler); +void SetErrorReporter(ErrorReporter* errorReporter); DSLGlobalVar sk_FragColor(); diff --git a/include/sksl/DSLErrorHandling.h b/include/sksl/DSLErrorHandling.h deleted file mode 100644 index 5e42811994..0000000000 --- a/include/sksl/DSLErrorHandling.h +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright 2021 Google LLC. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SKSL_DSL_ERROR_HANDLING -#define SKSL_DSL_ERROR_HANDLING - -namespace SkSL { - -namespace dsl { - -#ifndef __has_builtin - #define __has_builtin(x) 0 -#endif - -class PositionInfo { -public: - PositionInfo(const char* file = nullptr, int line = -1) - : fFile(file) - , fLine(line) {} - -#if __has_builtin(__builtin_FILE) && __has_builtin(__builtin_LINE) - static PositionInfo Capture(const char* file = __builtin_FILE(), int line = __builtin_LINE()) { - return PositionInfo(file, line); - } -#else - static PositionInfo Capture() { return PositionInfo(); } -#endif // __has_builtin(__builtin_FILE) && __has_builtin(__builtin_LINE) - - const char* file_name() { - return fFile; - } - - int line() { - return fLine; - } - -private: - const char* fFile; - int fLine; -}; - -/** - * Class which is notified in the event of an error. - */ -class ErrorHandler { -public: - virtual ~ErrorHandler() {} - - /** - * Reports a DSL error. Position may not be available, in which case it will be null. - */ - virtual void handleError(const char* msg, PositionInfo position) = 0; -}; - -} // namespace dsl - -} // namespace SkSL - -#endif diff --git a/include/sksl/DSLExpression.h b/include/sksl/DSLExpression.h index bd0c940ba6..837c5b42ff 100644 --- a/include/sksl/DSLExpression.h +++ b/include/sksl/DSLExpression.h @@ -11,8 +11,8 @@ #include "include/core/SkStringView.h" #include "include/core/SkTypes.h" #include "include/private/SkTArray.h" -#include "include/sksl/DSLErrorHandling.h" #include "include/sksl/DSLWrapper.h" +#include "include/sksl/SkSLErrorReporter.h" #include #include diff --git a/include/sksl/DSLLayout.h b/include/sksl/DSLLayout.h index 5bc9d5064a..f0cb1b2648 100644 --- a/include/sksl/DSLLayout.h +++ b/include/sksl/DSLLayout.h @@ -11,7 +11,7 @@ #include "include/sksl/DSLLayout.h" #include "include/private/SkSLLayout.h" -#include "include/sksl/DSLErrorHandling.h" +#include "include/sksl/SkSLErrorReporter.h" namespace SkSL { diff --git a/include/sksl/DSLStatement.h b/include/sksl/DSLStatement.h index f0c1bfd6bb..4f4f80d70f 100644 --- a/include/sksl/DSLStatement.h +++ b/include/sksl/DSLStatement.h @@ -11,7 +11,7 @@ #include "include/core/SkString.h" #include "include/core/SkTypes.h" #include "include/private/SkSLStatement.h" -#include "include/sksl/DSLErrorHandling.h" +#include "include/sksl/SkSLErrorReporter.h" #include diff --git a/include/sksl/SkSLErrorReporter.h b/include/sksl/SkSLErrorReporter.h new file mode 100644 index 0000000000..5ee35e5820 --- /dev/null +++ b/include/sksl/SkSLErrorReporter.h @@ -0,0 +1,119 @@ +/* + * Copyright 2021 Google LLC. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SKSL_ERROR_REPORTER +#define SKSL_ERROR_REPORTER + +#include "include/core/SkTypes.h" + +#include +#include + +namespace SkSL { + +#ifndef __has_builtin + #define __has_builtin(x) 0 +#endif + +class PositionInfo { +public: + PositionInfo(const char* file = nullptr, int line = -1) + : fFile(file) + , fLine(line) {} + +#if __has_builtin(__builtin_FILE) && __has_builtin(__builtin_LINE) + static PositionInfo Capture(const char* file = __builtin_FILE(), int line = __builtin_LINE()) { + return PositionInfo(file, line); + } +#else + static PositionInfo Capture() { return PositionInfo(); } +#endif // __has_builtin(__builtin_FILE) && __has_builtin(__builtin_LINE) + + const char* file_name() const { + return fFile; + } + + int line() { + return fLine; + } + +private: + const char* fFile; + int fLine; +}; + +/** + * Class which is notified in the event of an error. + */ +class ErrorReporter { +public: + ErrorReporter() {} + + virtual ~ErrorReporter() { + SkASSERT(fPendingErrors.empty()); + } + + void error(const char* msg, PositionInfo position); + + /** + * Reports an error message at the given character offset of the source text. Errors reported + * with an offset of -1 will be queued until line number information can be determined. + */ + void error(int offset, std::string msg) { + this->error(offset, msg.c_str()); + } + + /** + * Reports an error message at the given character offset of the source text. Errors reported + * with an offset of -1 will be queued until line number information can be determined. + */ + void error(int offset, const char* msg); + + const char* source() const { return fSource; } + + void setSource(const char* source) { fSource = source; } + + void reportPendingErrors(PositionInfo pos) { + for (std::string& msg : fPendingErrors) { + this->handleError(msg.c_str(), pos); + } + fPendingErrors.clear(); + } + + int errorCount() const { + return fErrorCount; + } + + void resetErrorCount() { + fErrorCount = 0; + } + +protected: + /** + * Reports an error. Position may not be available, in which case it will be null. + */ + virtual void handleError(const char* msg, PositionInfo position) = 0; + +private: + PositionInfo position(int offset) const; + + const char* fSource = nullptr; + std::vector fPendingErrors; + int fErrorCount = 0; +}; + +/** + * Error reporter for tests that need an SkSL context; aborts immediately if an error is reported. + */ +class TestingOnly_AbortErrorReporter : public ErrorReporter { +public: + void handleError(const char* msg, PositionInfo pos) override { SK_ABORT("%s", msg); } +}; + +} // namespace SkSL + +#endif diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp index c71e9f666d..6b9dcc7385 100644 --- a/src/sksl/SkSLAnalysis.cpp +++ b/src/sksl/SkSLAnalysis.cpp @@ -11,8 +11,8 @@ #include "include/private/SkSLProgramElement.h" #include "include/private/SkSLSampleUsage.h" #include "include/private/SkSLStatement.h" +#include "include/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLCompiler.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/ir/SkSLExpression.h" #include "src/sksl/ir/SkSLProgram.h" @@ -304,11 +304,7 @@ private: // If a caller doesn't care about errors, we can use this trivial reporter that just counts up. class TrivialErrorReporter : public ErrorReporter { public: - void handleError(const char*, dsl::PositionInfo) override { ++fErrorCount; } - int errorCount() override { return fErrorCount; } - -private: - int fErrorCount = 0; + void handleError(const char*, PositionInfo) override {} }; // This isn't actually using ProgramVisitor, because it only considers a subset of the fields for diff --git a/src/sksl/SkSLBuiltinTypes.cpp b/src/sksl/SkSLBuiltinTypes.cpp index 28ad6378be..b4d572171f 100644 --- a/src/sksl/SkSLBuiltinTypes.cpp +++ b/src/sksl/SkSLBuiltinTypes.cpp @@ -9,7 +9,6 @@ #include "include/private/SkSLModifiers.h" #include "src/sksl/SkSLCompiler.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/ir/SkSLExpression.h" #include "src/sksl/spirv.h" diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 6900509bd3..89e5dad598 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -84,12 +84,12 @@ class AutoSource { 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 @@ public: class AutoProgramConfig { public: AutoProgramConfig(std::shared_ptr& 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 @@ public: }; Compiler::Compiler(const ShaderCapsClass* caps) - : fContext(std::make_shared(/*errors=*/*this, *caps)) + : fErrorReporter(this) + , fContext(std::make_shared(fErrorReporter, *caps)) , fInliner(fContext.get()) { SkASSERT(caps); fRootModule.fSymbols = this->makeRootSymbolTable(); @@ -139,7 +141,7 @@ Compiler::~Compiler() {} #define TYPE(t) fContext->fTypes.f ## t .get() std::shared_ptr Compiler::makeRootSymbolTable() { - auto rootSymbolTable = std::make_shared(this, /*builtin=*/true); + auto rootSymbolTable = std::make_shared(&this->errorReporter(), /*builtin=*/true); const SkSL::Symbol* rootTypes[] = { TYPE(Void), @@ -340,6 +342,7 @@ LoadedModule Compiler::loadModule(ProgramKind kind, settings.fReplaceSettings = !dehydrate; #if defined(SKSL_STANDALONE) + SkASSERT(this->errorCount() == 0); SkASSERT(data.fPath); std::ifstream in(data.fPath); String text{std::istreambuf_iterator(in), std::istreambuf_iterator()}; @@ -353,16 +356,17 @@ LoadedModule Compiler::loadModule(ProgramKind kind, std::vector> elements; std::vector 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 @@ std::unique_ptr Compiler::convertProgram( settings.fAllowNarrowingConversions = true; } - fErrorText = ""; - fErrorCount = 0; + this->resetErrors(); fInliner.reset(); #if SKSL_DSL_PARSER @@ -486,7 +489,7 @@ std::unique_ptr Compiler::convertProgram( 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::unique_ptr Compiler::convertProgram( 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 @@ void Compiler::verifyStaticTests(const Program& program) { } // 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& element : program.ownedElements()) { if (element->is()) { visitor.visitProgramElement(*element); @@ -570,7 +574,7 @@ void Compiler::verifyStaticTests(const Program& program) { } bool Compiler::optimize(LoadedModule& module) { - SkASSERT(!fErrorCount); + SkASSERT(!this->errorCount()); // Create a temporary program configuration with default settings. ProgramConfig config; @@ -582,13 +586,13 @@ bool Compiler::optimize(LoadedModule& module) { std::unique_ptr 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 @@ bool Compiler::optimize(Program& program) { 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 @@ bool Compiler::optimize(Program& program) { 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 @@ bool Compiler::toSPIRV(Program& program, OutputStream& out) { 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 @@ bool Compiler::toSPIRV(Program& program, OutputStream& out) { 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 @@ bool Compiler::toMetal(Program& program, String* out) { #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"; diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h index 66e84f6277..35b99e89d8 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -14,7 +14,6 @@ #include "src/sksl/SkSLASTFile.h" #include "src/sksl/SkSLAnalysis.h" #include "src/sksl/SkSLContext.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLInliner.h" #include "src/sksl/SkSLParsedModule.h" #include "src/sksl/ir/SkSLProgram.h" @@ -68,7 +67,7 @@ struct LoadedModule { * * See the README for information about SkSL. */ -class SK_API Compiler : public ErrorReporter { +class SK_API Compiler { public: static constexpr const char FRAGCOLOR_NAME[] = "sk_FragColor"; static constexpr const char RTADJUST_NAME[] = "sk_RTAdjust"; @@ -120,7 +119,7 @@ public: Compiler(const ShaderCapsClass* caps); - ~Compiler() override; + ~Compiler(); Compiler(const Compiler&) = delete; Compiler& operator=(const Compiler&) = delete; @@ -161,19 +160,19 @@ public: bool toMetal(Program& program, String* out); - void handleError(const char* msg, dsl::PositionInfo pos) override; + void handleError(const char* msg, PositionInfo pos); String errorText(bool showCount = true); + ErrorReporter& errorReporter() { return fContext->errors(); } + + int errorCount() const { return fContext->errors().errorCount(); } + void writeErrorCount(); void resetErrors() { fErrorText.clear(); - fErrorCount = 0; - } - - int errorCount() override { - return fErrorCount; + this->errorReporter().resetErrorCount(); } Context& context() { @@ -207,6 +206,19 @@ public: const ParsedModule& moduleForProgramKind(ProgramKind kind); private: + class CompilerErrorReporter : public ErrorReporter { + public: + CompilerErrorReporter(Compiler* compiler) + : fCompiler(*compiler) {} + + void handleError(const char* msg, PositionInfo pos) override { + fCompiler.handleError(msg, pos); + } + + private: + Compiler& fCompiler; + }; + const ParsedModule& loadGPUModule(); const ParsedModule& loadFragmentModule(); const ParsedModule& loadVertexModule(); @@ -240,6 +252,7 @@ private: Position position(int offset); + CompilerErrorReporter fErrorReporter; std::shared_ptr fContext; ParsedModule fRootModule; // Core types @@ -261,7 +274,6 @@ private: Inliner fInliner; std::unique_ptr fIRGenerator; - int fErrorCount = 0; String fErrorText; static OverrideFlag sOptimizer; diff --git a/src/sksl/SkSLConstantFolder.cpp b/src/sksl/SkSLConstantFolder.cpp index e273b5c29d..abef8d5c6c 100644 --- a/src/sksl/SkSLConstantFolder.cpp +++ b/src/sksl/SkSLConstantFolder.cpp @@ -9,8 +9,8 @@ #include +#include "include/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLContext.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/ir/SkSLBinaryExpression.h" #include "src/sksl/ir/SkSLBoolLiteral.h" #include "src/sksl/ir/SkSLConstructor.h" diff --git a/src/sksl/SkSLConstantFolder.h b/src/sksl/SkSLConstantFolder.h index 992030a7fe..7fee850c52 100644 --- a/src/sksl/SkSLConstantFolder.h +++ b/src/sksl/SkSLConstantFolder.h @@ -16,7 +16,6 @@ namespace SkSL { class Context; -class ErrorReporter; class Expression; /** diff --git a/src/sksl/SkSLContext.cpp b/src/sksl/SkSLContext.cpp index 83c9248805..4e695978f2 100644 --- a/src/sksl/SkSLContext.cpp +++ b/src/sksl/SkSLContext.cpp @@ -7,6 +7,9 @@ #include "src/sksl/SkSLContext.h" +#include "include/sksl/DSLCore.h" +#include "src/sksl/dsl/priv/DSLWriter.h" + namespace SkSL { Context::Context(ErrorReporter& errors, const ShaderCapsClass& caps) @@ -15,5 +18,12 @@ Context::Context(ErrorReporter& errors, const ShaderCapsClass& caps) SkASSERT(!Pool::IsAttached()); } +ErrorReporter& Context::errors() const { + if (dsl::DSLWriter::IsActive()) { + return dsl::GetErrorReporter(); + } + return fErrors; +} + } // namespace SkSL diff --git a/src/sksl/SkSLContext.h b/src/sksl/SkSLContext.h index aff135d89d..0d76518abe 100644 --- a/src/sksl/SkSLContext.h +++ b/src/sksl/SkSLContext.h @@ -10,8 +10,8 @@ #include +#include "include/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLBuiltinTypes.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLPool.h" #include "src/sksl/SkSLUtil.h" #include "src/sksl/ir/SkSLExpression.h" @@ -32,8 +32,8 @@ public: SkASSERT(!Pool::IsAttached()); } - // Returns the current error handler - ErrorReporter& errors() const { return fErrors; } + // Returns the current error reporter + ErrorReporter& errors() const; // The Context holds all of the built-in types. BuiltinTypes fTypes; @@ -48,7 +48,8 @@ public: ProgramConfig* fConfig = nullptr; private: - // The Context holds a reference to our error reporter. + // The default error reporter to use outside of DSL code (between Start() and End(), the DSL + // error reporter is used instead) ErrorReporter& fErrors; }; diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp index debccf6a3f..a000ecacb1 100644 --- a/src/sksl/SkSLDSLParser.cpp +++ b/src/sksl/SkSLDSLParser.cpp @@ -109,7 +109,7 @@ DSLParser::DSLParser(Compiler* compiler, const ProgramSettings& settings, Progra String text) : fCompiler(*compiler) , fSettings(settings) - , fErrorReporter(compiler) + , fErrorReporter(&compiler->errorReporter()) , fKind(kind) , fText(std::move(text)) , fPushback(Token::Kind::TK_NONE, -1, -1) { @@ -205,12 +205,12 @@ void DSLParser::error(int offset, String msg) { fErrorReporter->error(offset, msg); } -class DSLParserErrorHandler final : public ErrorHandler { +class DSLParserErrorReporter final : public ErrorReporter { public: - DSLParserErrorHandler(ErrorReporter* reporter) + DSLParserErrorReporter(ErrorReporter* reporter) : fReporter(*reporter) {} - ~DSLParserErrorHandler() override { + ~DSLParserErrorReporter() override { for (const String& s : fErrors) { fReporter.error(/*offset=*/-1, s.c_str()); } @@ -230,15 +230,15 @@ private: /* declaration* END_OF_FILE */ std::unique_ptr DSLParser::program() { Start(&fCompiler, fKind, fSettings); - DSLParserErrorHandler errorHandler(&fCompiler); - SetErrorHandler(&errorHandler); + DSLParserErrorReporter errorReporter(&fCompiler.errorReporter()); + SetErrorReporter(&errorReporter); std::unique_ptr result; bool done = false; while (!done) { switch (this->peek().fKind) { case Token::Kind::TK_END_OF_FILE: done = true; - if (errorHandler.fErrors.empty()) { + if (errorReporter.fErrors.empty()) { result = dsl::ReleaseProgram(std::make_unique(std::move(fText))); } break; @@ -544,7 +544,7 @@ skstd::optional DSLParser::structDeclaration() { "struct '" + this->text(name) + "' must contain at least one field"); return skstd::nullopt; } - return dsl::Struct(this->text(name), std::move(fields)); + return dsl::Struct(this->text(name), SkMakeSpan(fields.size())); } /* structDeclaration ((IDENTIFIER varDeclarationEnd) | SEMICOLON) */ diff --git a/src/sksl/SkSLDSLParser.h b/src/sksl/SkSLDSLParser.h index 3908bfb8ca..fc3f1be561 100644 --- a/src/sksl/SkSLDSLParser.h +++ b/src/sksl/SkSLDSLParser.h @@ -15,7 +15,6 @@ #include "include/private/SkTOptional.h" #include "include/sksl/DSL.h" #include "include/sksl/DSLSymbols.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLLexer.h" #include "src/sksl/ir/SkSLProgram.h" @@ -23,6 +22,7 @@ namespace SkSL { +class ErrorReporter; struct Modifiers; class SymbolTable; @@ -274,63 +274,63 @@ private: Checkpoint(DSLParser* p) : fParser(p) { fPushbackCheckpoint = fParser->fPushback; fLexerCheckpoint = fParser->fLexer.getCheckpoint(); - fOldErrorHandler = dsl::GetErrorHandler(); - SkASSERT(fOldErrorHandler); - dsl::SetErrorHandler(&fErrorHandler); + fOldErrorReporter = &dsl::GetErrorReporter(); + SkASSERT(fOldErrorReporter); + dsl::SetErrorReporter(&fErrorReporter); } ~Checkpoint() { - SkASSERTF(!fOldErrorHandler, + SkASSERTF(!fOldErrorReporter, "Checkpoint was not accepted or rewound before destruction"); } void accept() { - this->restoreErrorHandler(); + this->restoreErrorReporter(); // Parser errors should have been fatal, but we can encounter other errors like type // mismatches despite accepting the parse. Forward those messages to the actual error // handler now. - fErrorHandler.forwardErrors(); + fErrorReporter.forwardErrors(); } void rewind() { - this->restoreErrorHandler(); + this->restoreErrorReporter(); fParser->fPushback = fPushbackCheckpoint; fParser->fLexer.rewindToCheckpoint(fLexerCheckpoint); } private: - class ForwardingErrorHandler : public dsl::ErrorHandler { + class ForwardingErrorReporter : public ErrorReporter { public: - void handleError(const char* msg, dsl::PositionInfo pos) override { + void handleError(const char* msg, PositionInfo pos) override { fErrors.push_back({String(msg), pos}); } void forwardErrors() { for (Error& error : fErrors) { - dsl::GetErrorHandler()->handleError(error.fMsg.c_str(), error.fPos); + dsl::GetErrorReporter().error(error.fMsg.c_str(), error.fPos); } } private: struct Error { String fMsg; - dsl::PositionInfo fPos; + PositionInfo fPos; }; SkTArray fErrors; }; - void restoreErrorHandler() { - SkASSERT(fOldErrorHandler); - dsl::SetErrorHandler(fOldErrorHandler); - fOldErrorHandler = nullptr; + void restoreErrorReporter() { + SkASSERT(fOldErrorReporter); + dsl::SetErrorReporter(fOldErrorReporter); + fOldErrorReporter = nullptr; } DSLParser* fParser; Token fPushbackCheckpoint; int32_t fLexerCheckpoint; - ForwardingErrorHandler fErrorHandler; - dsl::ErrorHandler* fOldErrorHandler; + ForwardingErrorReporter fErrorReporter; + ErrorReporter* fOldErrorReporter; }; static std::unordered_map* layoutTokens; diff --git a/src/sksl/SkSLErrorReporter.cpp b/src/sksl/SkSLErrorReporter.cpp new file mode 100644 index 0000000000..49a66a4121 --- /dev/null +++ b/src/sksl/SkSLErrorReporter.cpp @@ -0,0 +1,42 @@ +/* + * Copyright 2021 Google LLC. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "include/sksl/SkSLErrorReporter.h" + +#include "src/sksl/dsl/priv/DSLWriter.h" + +namespace SkSL { + +void ErrorReporter::error(const char* msg, PositionInfo position) { + ++fErrorCount; + this->handleError(msg, position); +} + +void ErrorReporter::error(int offset, const char* msg) { + if (offset == -1) { + ++fErrorCount; + fPendingErrors.push_back(std::string(msg)); + } else { + this->error(msg, this->position(offset)); + } +} + +PositionInfo ErrorReporter::position(int offset) const { + if (fSource && offset >= 0) { + int line = 1; + for (int i = 0; i < offset; i++) { + if (fSource[i] == '\n') { + ++line; + } + } + return PositionInfo(/*file=*/nullptr, line); + } else { + return PositionInfo(); + } +} + +} // namespace SkSL diff --git a/src/sksl/SkSLErrorReporter.h b/src/sksl/SkSLErrorReporter.h deleted file mode 100644 index 278713215e..0000000000 --- a/src/sksl/SkSLErrorReporter.h +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright 2016 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SKSL_ERRORREPORTER -#define SKSL_ERRORREPORTER - -#include "include/sksl/DSLErrorHandling.h" -#include "src/sksl/SkSLPosition.h" -#include "src/sksl/SkSLPosition.h" - -namespace SkSL { - -/** - * Extends dsl::ErrorHandler to add offset-based error reporting for usage by Compiler. - */ -class ErrorReporter : public dsl::ErrorHandler { -public: - ErrorReporter() {} - - ErrorReporter(const char* filename, const char* source) - : fFilename(filename) - , fSource(source) {} - - /** Reports an error message at the given character offset of the source text. */ - void error(int offset, String msg) { - this->handleError(msg.c_str(), this->position(offset)); - } - - void error(int offset, const char* msg) { - this->handleError(msg, this->position(offset)); - } - - /** Returns the number of errors that have been reported. */ - virtual int errorCount() = 0; - - const char* fFilename = nullptr; - const char* fSource = nullptr; - -private: - dsl::PositionInfo position(int offset) const { - if (fSource && offset >= 0) { - int line = 1; - for (int i = 0; i < offset; i++) { - if (fSource[i] == '\n') { - ++line; - } - } - return dsl::PositionInfo(fFilename, line); - } else { - return dsl::PositionInfo(fFilename, -1); - } - } -}; - -/** - * Error reporter for tests that need an SkSL context; aborts immediately if an error is reported. - */ -class TestingOnly_AbortErrorReporter : public ErrorReporter { -public: - void handleError(const char* msg, dsl::PositionInfo pos) override { SK_ABORT("%s", msg); } - int errorCount() override { return 0; } -}; - -} // namespace SkSL - -#endif diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index 1e643bdb9a..40c1687a2a 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -1036,7 +1036,7 @@ std::unique_ptr IRGenerator::convertInterfaceBlock(const A if (vd.var().type().isOpaque()) { this->errorReporter().error(decl->fOffset, "opaque type '" + vd.var().type().name() + - "' is not permitted in an interface block"); + "' is not permitted in an interface block"); } if (&vd.var() == fRTAdjust) { foundRTAdjust = true; diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h index 7c79fcf95d..e0d7a3e361 100644 --- a/src/sksl/SkSLIRGenerator.h +++ b/src/sksl/SkSLIRGenerator.h @@ -15,7 +15,6 @@ #include "include/private/SkSLStatement.h" #include "src/sksl/SkSLASTFile.h" #include "src/sksl/SkSLASTNode.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLOperators.h" #include "src/sksl/ir/SkSLBlock.h" #include "src/sksl/ir/SkSLExpression.h" diff --git a/src/sksl/SkSLParser.h b/src/sksl/SkSLParser.h index d6e33ac242..c9bc418f74 100644 --- a/src/sksl/SkSLParser.h +++ b/src/sksl/SkSLParser.h @@ -16,8 +16,8 @@ #include "include/sksl/DSLCore.h" #include "src/sksl/SkSLASTFile.h" #include "src/sksl/SkSLASTNode.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLLexer.h" +#include "src/sksl/SkSLPosition.h" namespace SkSL { @@ -286,24 +286,20 @@ private: private: class ForwardingErrorReporter : public ErrorReporter { public: - void handleError(const char* msg, dsl::PositionInfo pos) override { + void handleError(const char* msg, PositionInfo pos) override { fErrors.push_back({String(msg), pos}); } - int errorCount() override { - return fErrors.count(); - } - void forwardErrors(ErrorReporter& target) { for (Error& error : fErrors) { - target.handleError(error.fMsg.c_str(), error.fPos); + target.error(error.fMsg.c_str(), error.fPos); } } private: struct Error { String fMsg; - dsl::PositionInfo fPos; + PositionInfo fPos; }; SkTArray fErrors; diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp index 9701a5c258..2b3a8487ec 100644 --- a/src/sksl/dsl/DSLCore.cpp +++ b/src/sksl/dsl/DSLCore.cpp @@ -51,12 +51,13 @@ void End() { DSLWriter::SetInstance(nullptr); } -ErrorHandler* GetErrorHandler() { - return DSLWriter::GetErrorHandler(); +ErrorReporter& GetErrorReporter() { + return *DSLWriter::GetErrorReporter(); } -void SetErrorHandler(ErrorHandler* errorHandler) { - DSLWriter::SetErrorHandler(errorHandler); +void SetErrorReporter(ErrorReporter* errorReporter) { + SkASSERT(errorReporter); + DSLWriter::SetErrorReporter(errorReporter); } class DSLCore { @@ -76,7 +77,7 @@ public: std::move(instance.fPool), bundle.fInputs); bool success = false; - if (DSLWriter::Compiler().errorCount() || DSLWriter::Instance().fEncounteredErrors) { + if (DSLWriter::Context().errors().errorCount()) { DSLWriter::ReportErrors(); // Do not return programs that failed to compile. } else if (!DSLWriter::Compiler().optimize(*result)) { @@ -86,11 +87,6 @@ public: // We have a successful program! success = true; } - // Make sure that if we encountered any compiler errors, we reported them through the - // DSL error handling side of things. (The converse is not a problem - it is ok to detect - // errors in the DSL layer, and thus have fEncounteredErrors be true, while not having the - // compiler see any errors because we caught them before they got there.) - SkASSERT(!DSLWriter::Compiler().errorCount() || DSLWriter::Instance().fEncounteredErrors); if (pool) { pool->detachFromThread(); } @@ -135,7 +131,7 @@ public: static DSLStatement Declare(DSLVar& var, PositionInfo pos) { if (var.fDeclared) { - DSLWriter::ReportError("error: variable has already been declared\n", pos); + DSLWriter::ReportError("variable has already been declared", pos); } var.fDeclared = true; return DSLWriter::Declaration(var); @@ -151,7 +147,7 @@ public: static void Declare(DSLGlobalVar& var, PositionInfo pos) { if (var.fDeclared) { - DSLWriter::ReportError("error: variable has already been declared\n", pos); + DSLWriter::ReportError("variable has already been declared", pos); } var.fDeclared = true; std::unique_ptr stmt = DSLWriter::Declaration(var); diff --git a/src/sksl/dsl/DSLExpression.cpp b/src/sksl/dsl/DSLExpression.cpp index 1aa4d00cd2..fde1ab3762 100644 --- a/src/sksl/dsl/DSLExpression.cpp +++ b/src/sksl/dsl/DSLExpression.cpp @@ -45,9 +45,9 @@ DSLExpression::DSLExpression(float value) value)) { if (!isfinite(value)) { if (isinf(value)) { - DSLWriter::ReportError("error: floating point value is infinite\n"); + DSLWriter::ReportError("floating point value is infinite"); } else if (isnan(value)) { - DSLWriter::ReportError("error: floating point value is NaN\n"); + DSLWriter::ReportError("floating point value is NaN"); } } } @@ -254,10 +254,6 @@ DSLPossibleExpression operator,(DSLPossibleExpression left, DSLPossibleExpressio } std::unique_ptr DSLExpression::coerceAndRelease(const SkSL::Type& type) { - // tripping this assert means we had an error occur somewhere else in DSL construction that - // wasn't caught where it should have been - SkASSERTF(!DSLWriter::Compiler().errorCount(), "Unexpected SkSL DSL error: %s", - DSLWriter::Compiler().errorText().c_str()); return DSLWriter::Coerce(this->release(), type).release(); } diff --git a/src/sksl/dsl/DSLFunction.cpp b/src/sksl/dsl/DSLFunction.cpp index 8bd93ad978..21305c1b44 100644 --- a/src/sksl/dsl/DSLFunction.cpp +++ b/src/sksl/dsl/DSLFunction.cpp @@ -37,7 +37,7 @@ void DSLFunction::init(DSLModifiers modifiers, const DSLType& returnType, skstd: paramVars.reserve(params.size()); for (DSLParameter* param : params) { if (param->fDeclared) { - DSLWriter::ReportError("error: parameter has already been used in another function\n"); + DSLWriter::ReportError("parameter has already been used in another function"); } SkASSERT(!param->fInitialValue.valid()); SkASSERT(!param->fDeclaration); diff --git a/src/sksl/dsl/DSLLayout.cpp b/src/sksl/dsl/DSLLayout.cpp index 001da33058..157c191729 100644 --- a/src/sksl/dsl/DSLLayout.cpp +++ b/src/sksl/dsl/DSLLayout.cpp @@ -15,8 +15,8 @@ namespace dsl { DSLLayout& DSLLayout::flag(SkSL::Layout::Flag mask, const char* name, PositionInfo pos) { if (fSkSLLayout.fFlags & mask) { - DSLWriter::ReportError(("error: layout qualifier '" + String(name) + - "' appears more than once\n").c_str(), pos); + DSLWriter::ReportError(("layout qualifier '" + String(name) + + "' appears more than once").c_str(), pos); } fSkSLLayout.fFlags |= mask; return *this; diff --git a/src/sksl/dsl/DSLType.cpp b/src/sksl/dsl/DSLType.cpp index 25501b4644..4791b2394b 100644 --- a/src/sksl/dsl/DSLType.cpp +++ b/src/sksl/dsl/DSLType.cpp @@ -204,10 +204,10 @@ DSLExpression DSLType::Construct(DSLType type, SkSpan argArray) { DSLType Array(const DSLType& base, int count) { if (count <= 0) { - DSLWriter::ReportError("error: array size must be positive\n"); + DSLWriter::ReportError("array size must be positive"); } if (base.isArray()) { - DSLWriter::ReportError("error: multidimensional arrays are not permitted\n"); + DSLWriter::ReportError("multidimensional arrays are not permitted"); return base; } return DSLWriter::SymbolTable()->addArrayDimension(&base.skslType(), count); diff --git a/src/sksl/dsl/DSLVar.cpp b/src/sksl/dsl/DSLVar.cpp index 90320dfa7b..9fb8abb59c 100644 --- a/src/sksl/dsl/DSLVar.cpp +++ b/src/sksl/dsl/DSLVar.cpp @@ -72,8 +72,8 @@ DSLVarBase::DSLVarBase(const DSLModifiers& modifiers, DSLType type, skstd::strin DSLVarBase::~DSLVarBase() { if (fDeclaration && !fDeclared) { - DSLWriter::ReportError(String::printf("error: variable '%.*s' was destroyed without being " - "declared\n", + DSLWriter::ReportError(String::printf("variable '%.*s' was destroyed without being " + "declared", (int)fRawName.length(), fRawName.data()).c_str()); } diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp index 6f03e09ae3..7e64472220 100644 --- a/src/sksl/dsl/priv/DSLWriter.cpp +++ b/src/sksl/dsl/priv/DSLWriter.cpp @@ -9,7 +9,7 @@ #include "include/private/SkSLDefines.h" #include "include/sksl/DSLCore.h" -#include "include/sksl/DSLErrorHandling.h" +#include "include/sksl/SkSLErrorReporter.h" #if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU #include "src/gpu/glsl/GrGLSLFragmentShaderBuilder.h" #include "src/gpu/mock/GrMockCaps.h" @@ -208,15 +208,18 @@ DSLPossibleStatement DSLWriter::ConvertSwitch(std::unique_ptr value, } void DSLWriter::ReportError(const char* msg, PositionInfo info) { - Instance().fEncounteredErrors = true; - if (Instance().fErrorHandler) { - Instance().fErrorHandler->handleError(msg, info); - } else if (info.file_name()) { - SK_ABORT("%s: %d: %sNo SkSL DSL error handler configured, treating this as a fatal error\n", - info.file_name(), info.line(), msg); + Instance().fErrorReporter->error(msg, info); +} + +void DSLWriter::DefaultErrorReporter::handleError(const char* msg, PositionInfo pos) { + if (pos.line() > -1) { + SK_ABORT("error: %s: %d: %sNo SkSL DSL error reporter configured, treating this as a fatal " + "error\n", pos.file_name(), pos.line(), msg); } else { - SK_ABORT("%sNo SkSL DSL error handler configured, treating this as a fatal error\n", msg); + SK_ABORT("error: %s\nNo SkSL DSL error reporter configured, treating this as a fatal " + "error\n", msg); } + } const SkSL::Variable* DSLWriter::Var(DSLVarBase& var) { @@ -279,16 +282,17 @@ void DSLWriter::MarkDeclared(DSLVarBase& var) { } void DSLWriter::ReportErrors(PositionInfo pos) { - if (Compiler().errorCount()) { - ReportError(DSLWriter::Compiler().errorText(/*showCount=*/false).c_str(), pos); - Compiler().resetErrors(); - } + GetErrorReporter()->reportPendingErrors(pos); } #if SKSL_USE_THREAD_LOCAL thread_local DSLWriter* instance = nullptr; +bool DSLWriter::IsActive() { + return instance != nullptr; +} + DSLWriter& DSLWriter::Instance() { SkASSERTF(instance, "dsl::Start() has not been called"); return *instance; @@ -318,6 +322,10 @@ static pthread_key_t get_pthread_key() { return sKey; } +bool DSLWriter::Active() { + return pthread_getspecific(get_pthread_key()) != nullptr; +} + DSLWriter& DSLWriter::Instance() { DSLWriter* instance = static_cast(pthread_getspecific(get_pthread_key())); SkASSERTF(instance, "dsl::Start() has not been called"); diff --git a/src/sksl/dsl/priv/DSLWriter.h b/src/sksl/dsl/priv/DSLWriter.h index 72c1f2f834..f76cdf2a65 100644 --- a/src/sksl/dsl/priv/DSLWriter.h +++ b/src/sksl/dsl/priv/DSLWriter.h @@ -42,7 +42,6 @@ namespace dsl { class DSLGlobalVar; class DSLParameter; class DSLVar; -class ErrorHandler; /** * Thread-safe class that tracks per-thread state associated with DSL output. This class is for @@ -55,6 +54,11 @@ public: ~DSLWriter(); + /** + * Returns true if the DSL has been started. + */ + static bool IsActive(); + /** * Returns the Compiler used by DSL operations in the current thread. */ @@ -219,20 +223,21 @@ public: bool isStatic); /** - * Returns the ErrorHandler associated with the current thread. This object will be notified - * when any DSL errors occur. With a null ErrorHandler (the default), any errors will be dumped + * Returns the ErrorReporter associated with the current thread. This object will be notified + * when any DSL errors occur. With a null ErrorReporter (the default), any errors will be dumped * to stderr and a fatal exception will be generated. */ - static ErrorHandler* GetErrorHandler() { - return Instance().fErrorHandler; + static ErrorReporter* GetErrorReporter() { + return Instance().fErrorReporter; } - static void SetErrorHandler(ErrorHandler* errorHandler) { - Instance().fErrorHandler = errorHandler; + static void SetErrorReporter(ErrorReporter* errorReporter) { + SkASSERT(errorReporter); + Instance().fErrorReporter = errorReporter; } /** - * Notifies the current ErrorHandler that a DSL error has occurred. With a null ErrorHandler + * Notifies the current ErrorReporter that a DSL error has occurred. With a null ErrorReporter * (the default), any errors will be dumped to stderr and a fatal exception will be generated. */ static void ReportError(const char* msg, PositionInfo info = PositionInfo::Capture()); @@ -268,7 +273,7 @@ public: } /** - * Forwards any pending Compiler errors to the DSL ErrorHandler. + * Forwards any pending Compiler errors to the DSL ErrorReporter. */ static void ReportErrors(PositionInfo pos = PositionInfo::Capture()); @@ -277,6 +282,10 @@ public: static void SetInstance(std::unique_ptr instance); private: + class DefaultErrorReporter : public ErrorReporter { + void handleError(const char* msg, PositionInfo pos) override; + }; + std::unique_ptr fConfig; std::unique_ptr fModifiersPool; SkSL::Compiler* fCompiler; @@ -285,11 +294,11 @@ private: SkSL::ModifiersPool* fOldModifiersPool; std::vector> fProgramElements; std::vector fSharedElements; - ErrorHandler* fErrorHandler = nullptr; + DefaultErrorReporter fDefaultErrorReporter; + ErrorReporter* fErrorReporter = &fDefaultErrorReporter; ProgramSettings fSettings; Mangler fMangler; bool fIsModule; - bool fEncounteredErrors = false; #if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU struct StackFrame { GrFragmentProcessor::ProgramImpl* fProcessor; diff --git a/src/sksl/ir/SkSLSwitchStatement.cpp b/src/sksl/ir/SkSLSwitchStatement.cpp index 45a5b469ad..d12ae7dd96 100644 --- a/src/sksl/ir/SkSLSwitchStatement.cpp +++ b/src/sksl/ir/SkSLSwitchStatement.cpp @@ -216,7 +216,7 @@ std::unique_ptr SwitchStatement::Convert(const Context& context, for (const SwitchCase* sc : duplicateCases) { if (sc->value() != nullptr) { context.errors().error(sc->fOffset, - "duplicate case value '" + sc->value()->description() + "'"); + "duplicate case value '" + sc->value()->description() + "'"); } else { context.errors().error(sc->fOffset, "duplicate default case"); } diff --git a/src/sksl/ir/SkSLSymbolTable.h b/src/sksl/ir/SkSLSymbolTable.h index 049a032ccb..41a2d5a4ed 100644 --- a/src/sksl/ir/SkSLSymbolTable.h +++ b/src/sksl/ir/SkSLSymbolTable.h @@ -12,7 +12,7 @@ #include "include/private/SkSLSymbol.h" #include "include/private/SkTArray.h" #include "include/private/SkTHash.h" -#include "src/sksl/SkSLErrorReporter.h" +#include "include/sksl/SkSLErrorReporter.h" #include #include diff --git a/tests/SkDSLRuntimeEffectTest.cpp b/tests/SkDSLRuntimeEffectTest.cpp index 838875f3e3..cc8aa2115c 100644 --- a/tests/SkDSLRuntimeEffectTest.cpp +++ b/tests/SkDSLRuntimeEffectTest.cpp @@ -200,22 +200,22 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrRecordingContext // Test error reporting. We put this before a couple of successful tests to ensure that a // failure doesn't leave us in a broken state. { - class SimpleErrorHandler : public ErrorHandler { + class SimpleErrorReporter : public SkSL::ErrorReporter { public: - void handleError(const char* msg, PositionInfo pos) override { - fMsg = msg; + void handleError(const char* msg, SkSL::PositionInfo pos) override { + fMsg += msg; } SkSL::String fMsg; - } errorHandler; + } errorReporter; effect.start(); - SetErrorHandler(&errorHandler); + SetErrorReporter(&errorReporter); Parameter p(kFloat2_Type, "p"); Function(kHalf4_Type, "main", p).define( Return(1) // Error, type mismatch ); effect.end(false); - REPORTER_ASSERT(r, errorHandler.fMsg == "error: expected 'half4', but found 'int'\n"); + REPORTER_ASSERT(r, errorReporter.fMsg == "expected 'half4', but found 'int'"); } // Mutating coords should work. (skbug.com/10918) diff --git a/tests/SkSLDSLErrorLineNumbers.cpp b/tests/SkSLDSLErrorLineNumbers.cpp index ecd7c4a024..326f177518 100644 --- a/tests/SkSLDSLErrorLineNumbers.cpp +++ b/tests/SkSLDSLErrorLineNumbers.cpp @@ -9,6 +9,7 @@ #include "include/sksl/DSL.h" #include "src/gpu/GrDirectContextPriv.h" #include "src/gpu/GrGpu.h" +#include "src/sksl/SkSLCompiler.h" #include "src/sksl/SkSLIRGenerator.h" #include "src/sksl/dsl/priv/DSLWriter.h" @@ -20,25 +21,27 @@ using namespace SkSL::dsl; #if defined(__GNUC__) || defined(__clang__) -class ExpectErrorLineNumber : public ErrorHandler { +class ExpectErrorLineNumber : public SkSL::ErrorReporter { public: ExpectErrorLineNumber(skiatest::Reporter* reporter, const char* msg, int line) : fMsg(msg) , fLine(line) - , fReporter(reporter) { - SetErrorHandler(this); + , fReporter(reporter) + , fOldReporter(&GetErrorReporter()) { + SetErrorReporter(this); } ~ExpectErrorLineNumber() override { REPORTER_ASSERT(fReporter, !fMsg); - SetErrorHandler(nullptr); + SetErrorReporter(fOldReporter); } - void handleError(const char* msg, PositionInfo pos) override { + void handleError(const char* msg, SkSL::PositionInfo pos) override { REPORTER_ASSERT(fReporter, !strcmp(msg, fMsg), "Error mismatch: expected:\n%sbut received:\n%s", fMsg, msg); REPORTER_ASSERT(fReporter, pos.line() == fLine, "Line number mismatch: expected %d, but received %d\n", fLine, pos.line()); + DSLWriter::Compiler().handleError(msg, pos); fMsg = nullptr; } @@ -46,13 +49,13 @@ private: const char* fMsg; int fLine; skiatest::Reporter* fReporter; + ErrorReporter* fOldReporter; }; DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLErrorLineNumbers, r, ctxInfo) { Start(ctxInfo.directContext()->priv().getGpu()->shaderCompiler()); { - ExpectErrorLineNumber error(r, - "error: type mismatch: '+' cannot operate on 'float', 'bool'\n", + ExpectErrorLineNumber error(r, "type mismatch: '+' cannot operate on 'float', 'bool'", __LINE__ + 1); (Float(1) + true).release(); } @@ -60,8 +63,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLErrorLineNumbers, r, ctxInfo) { { Var a(kBool_Type); DSLWriter::MarkDeclared(a); - ExpectErrorLineNumber error(r, - "error: type mismatch: '=' cannot operate on 'bool', 'float'\n", + ExpectErrorLineNumber error(r, "type mismatch: '=' cannot operate on 'bool', 'float'", __LINE__ + 1); (a = 5.0f).release(); } @@ -69,53 +71,39 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLErrorLineNumbers, r, ctxInfo) { { Var a(Array(kInt_Type, 5)); DSLWriter::MarkDeclared(a); - ExpectErrorLineNumber error(r, - "error: expected 'int', but found 'bool'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "expected 'int', but found 'bool'", __LINE__ + 1); (a[true]).release(); } { Var a(Array(kInt_Type, 5)); DSLWriter::MarkDeclared(a); - ExpectErrorLineNumber error(r, - "error: '++' cannot operate on 'int[5]'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "'++' cannot operate on 'int[5]'", __LINE__ + 1); (++a).release(); } { - ExpectErrorLineNumber error(r, - "error: expected 'bool', but found 'int'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "expected 'bool', but found 'int'", __LINE__ + 1); Do(Discard(), 5).release(); } { - ExpectErrorLineNumber error(r, - "error: expected 'bool', but found 'int'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "expected 'bool', but found 'int'", __LINE__ + 1); For(DSLStatement(), 5, DSLExpression(), Block()).release(); } { - ExpectErrorLineNumber error(r, - "error: expected 'bool', but found 'int'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "expected 'bool', but found 'int'", __LINE__ + 1); If(5, Discard()).release(); } { - ExpectErrorLineNumber error(r, - "error: expected 'bool', but found 'int'\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "expected 'bool', but found 'int'", __LINE__ + 1); While(5, Discard()).release(); } { - ExpectErrorLineNumber error(r, - "error: no match for abs(bool)\n", - __LINE__ + 1); + ExpectErrorLineNumber error(r, "no match for abs(bool)", __LINE__ + 1); Abs(true).release(); } End(); diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp index 8e1bfd27c6..90362a669d 100644 --- a/tests/SkSLDSLTest.cpp +++ b/tests/SkSLDSLTest.cpp @@ -50,29 +50,32 @@ public: } }; -class ExpectError : public ErrorHandler { +class ExpectError : public SkSL::ErrorReporter { public: ExpectError(skiatest::Reporter* reporter, const char* msg) : fMsg(msg) - , fReporter(reporter) { - SetErrorHandler(this); + , fReporter(reporter) + , fOldReporter(&GetErrorReporter()) { + SetErrorReporter(this); } ~ExpectError() override { REPORTER_ASSERT(fReporter, !fMsg, - "Error mismatch: expected:\n%sbut no error occurred\n", fMsg); - SetErrorHandler(nullptr); + "Error mismatch: expected:\n%s\nbut no error occurred\n", fMsg); + SetErrorReporter(fOldReporter); } - void handleError(const char* msg, PositionInfo pos) override { - REPORTER_ASSERT(fReporter, !strcmp(msg, fMsg), - "Error mismatch: expected:\n%sbut received:\n%s", fMsg, msg); + void handleError(const char* msg, SkSL::PositionInfo pos) override { + REPORTER_ASSERT(fReporter, fMsg, "Received unexpected extra error: %s\n", msg); + REPORTER_ASSERT(fReporter, !fMsg || !strcmp(msg, fMsg), + "Error mismatch: expected:\n%s\nbut received:\n%s", fMsg, msg); fMsg = nullptr; } private: const char* fMsg; skiatest::Reporter* fReporter; + ErrorReporter* fOldReporter; }; static bool whitespace_insensitive_compare(const char* a, const char* b) { @@ -209,24 +212,24 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFloat, r, ctxInfo) { EXPECT_EQUAL(y.x() = 1.0f, "(y.x = 1.0)"); { - ExpectError error(r, "error: floating point value is infinite\n"); + ExpectError error(r, "floating point value is infinite"); Float(std::numeric_limits::infinity()).release(); } { - ExpectError error(r, "error: floating point value is NaN\n"); + ExpectError error(r, "floating point value is NaN"); Float(std::numeric_limits::quiet_NaN()).release(); } { - ExpectError error(r, "error: 'float4' is not a valid parameter to 'float2' constructor; " - "use '.xy' instead\n"); + ExpectError error(r, "'float4' is not a valid parameter to 'float2' constructor; use '.xy' " + "instead"); Float2(Float4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'float4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'float4' constructor (expected 4 scalars, but " + "found 3)"); Float4(Float3(1)).release(); } } @@ -261,24 +264,24 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLHalf, r, ctxInfo) { "half4(0.0, 1.0, 2.0, 3.0)"); { - ExpectError error(r, "error: floating point value is infinite\n"); + ExpectError error(r, "floating point value is infinite"); Half(std::numeric_limits::infinity()).release(); } { - ExpectError error(r, "error: floating point value is NaN\n"); + ExpectError error(r, "floating point value is NaN"); Half(std::numeric_limits::quiet_NaN()).release(); } { - ExpectError error(r, "error: 'half4' is not a valid parameter to 'half2' constructor; use " - "'.xy' instead\n"); + ExpectError error(r, "'half4' is not a valid parameter to 'half2' constructor; use '.xy' " + "instead"); Half2(Half4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'half4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'half4' constructor (expected 4 scalars, but " + "found 3)"); Half4(Half3(1)).release(); } } @@ -308,14 +311,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLInt, r, ctxInfo) { "int4(0, 1, 2, 3)"); { - ExpectError error(r, "error: 'int4' is not a valid parameter to 'int2' constructor; use " - "'.xy' instead\n"); + ExpectError error(r, "'int4' is not a valid parameter to 'int2' constructor; use '.xy' " + "instead"); Int2(Int4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'int4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'int4' constructor (expected 4 scalars, but " + "found 3)"); Int4(Int3(1)).release(); } } @@ -345,14 +348,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLUInt, r, ctxInfo) { "uint4(0, 1, 2, 3)"); { - ExpectError error(r, "error: 'uint4' is not a valid parameter to 'uint2' constructor; use " - "'.xy' instead\n"); + ExpectError error(r, "'uint4' is not a valid parameter to 'uint2' constructor; use '.xy' " + "instead"); UInt2(UInt4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'uint4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'uint4' constructor (expected 4 scalars, but " + "found 3)"); UInt4(UInt3(1)).release(); } } @@ -382,14 +385,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLShort, r, ctxInfo) { "short4(0, 1, 2, 3)"); { - ExpectError error(r, "error: 'short4' is not a valid parameter to 'short2' constructor; " - "use '.xy' instead\n"); + ExpectError error(r, "'short4' is not a valid parameter to 'short2' constructor; use '.xy' " + "instead"); Short2(Short4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'short4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'short4' constructor (expected 4 scalars, but " + "found 3)"); Short4(Short3(1)).release(); } } @@ -419,14 +422,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLUShort, r, ctxInfo) { "ushort4(0, 1, 2, 3)"); { - ExpectError error(r, "error: 'ushort4' is not a valid parameter to 'ushort2' constructor; " - "use '.xy' instead\n"); + ExpectError error(r, "'ushort4' is not a valid parameter to 'ushort2' constructor; use " + "'.xy' instead"); UShort2(UShort4(1)).release(); } { - ExpectError error(r, "error: invalid arguments to 'ushort4' constructor (expected 4 " - "scalars, but found 3)\n"); + ExpectError error(r, "invalid arguments to 'ushort4' constructor (expected 4 scalars, but " + "found 3)"); UShort4(UShort3(1)).release(); } } @@ -454,14 +457,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBool, r, ctxInfo) { "bool4(false, true, false, true)"); { - ExpectError error(r, "error: 'bool4' is not a valid parameter to 'bool2' constructor; use " - "'.xy' instead\n"); + ExpectError error(r, "'bool4' is not a valid parameter to 'bool2' constructor; use '.xy' " + "instead"); Bool2(Bool4(true)).release(); } { - ExpectError error(r, "error: invalid arguments to 'bool4' constructor (expected 4 scalars," - " but found 3)\n"); + ExpectError error(r, "invalid arguments to 'bool4' constructor (expected 4 scalars, but " + "found 3)"); Bool4(Bool3(true)).release(); } } @@ -558,12 +561,12 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLType, r, ctxInfo) { e.release(); { - ExpectError error(r, "error: array size must be positive\n"); + ExpectError error(r, "array size must be positive"); Array(kFloat_Type, -1); } { - ExpectError error(r, "error: multidimensional arrays are not permitted\n"); + ExpectError error(r, "multidimensional arrays are not permitted"); Array(Array(kFloat_Type, 2), 2); } } @@ -630,37 +633,34 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLMatrices, r, ctxInfo) { EXPECT_EQUAL(Inverse(f44), "inverse(f44)"); { - ExpectError error(r, "error: invalid arguments to 'float3x3' constructor (expected 9 " - "scalars, but found 2)\n"); + ExpectError error(r, "invalid arguments to 'float3x3' constructor (expected 9 scalars, but " + "found 2)"); DSLExpression(Float3x3(Float2(1))).release(); } { - ExpectError error(r, "error: invalid arguments to 'half2x2' constructor (expected 4 " - "scalars, but found 5)\n"); + ExpectError error(r, "invalid arguments to 'half2x2' constructor (expected 4 scalars, but " + "found 5)"); DSLExpression(Half2x2(1, 2, 3, 4, 5)).release(); } { - ExpectError error(r, "error: type mismatch: '*' cannot operate on 'float4x3', 'float3'\n"); + ExpectError error(r, "type mismatch: '*' cannot operate on 'float4x3', 'float3'"); DSLExpression(f43 * Float3(1)).release(); } { - ExpectError error(r, "error: type mismatch: '=' cannot operate on 'float4x3', " - "'float3x3'\n"); + ExpectError error(r, "type mismatch: '=' cannot operate on 'float4x3', 'float3x3'"); DSLExpression(f43 = f33).release(); } { - ExpectError error(r, "error: type mismatch: '=' cannot operate on 'half2x2', " - "'float2x2'\n"); + ExpectError error(r, "type mismatch: '=' cannot operate on 'half2x2', 'float2x2'"); DSLExpression(h22 = f22).release(); } { - ExpectError error(r, - "error: no match for inverse(float4x3)\n"); + ExpectError error(r, "no match for inverse(float4x3)"); DSLExpression(Inverse(f43)).release(); } } @@ -683,22 +683,22 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLPlus, r, ctxInfo) { "(a + b)"); { - ExpectError error(r, "error: type mismatch: '+' cannot operate on 'bool2', 'float'\n"); + ExpectError error(r, "type mismatch: '+' cannot operate on 'bool2', 'float'"); DSLExpression((Bool2(true) + a)).release(); } { - ExpectError error(r, "error: type mismatch: '+=' cannot operate on 'float', 'bool2'\n"); + ExpectError error(r, "type mismatch: '+=' cannot operate on 'float', 'bool2'"); DSLExpression((a += Bool2(true))).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression((1.0 += a)).release(); } { - ExpectError error(r, "error: '+' cannot operate on 'bool'\n"); + ExpectError error(r, "'+' cannot operate on 'bool'"); Var c(kBool_Type); DSLExpression(+c).release(); } @@ -722,22 +722,22 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLMinus, r, ctxInfo) { "-(a - b)"); { - ExpectError error(r, "error: type mismatch: '-' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '-' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) - a).release(); } { - ExpectError error(r, "error: type mismatch: '-=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '-=' cannot operate on 'int', 'bool2'"); DSLExpression(a -= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1.0 -= a).release(); } { - ExpectError error(r, "error: '-' cannot operate on 'bool'\n"); + ExpectError error(r, "'-' cannot operate on 'bool'"); Var c(kBool_Type); DSLExpression(-c).release(); } @@ -757,17 +757,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLMultiply, r, ctxInfo) { "(a *= (b + 1.0))"); { - ExpectError error(r, "error: type mismatch: '*' cannot operate on 'bool2', 'float'\n"); + ExpectError error(r, "type mismatch: '*' cannot operate on 'bool2', 'float'"); DSLExpression(Bool2(true) * a).release(); } { - ExpectError error(r, "error: type mismatch: '*=' cannot operate on 'float', 'bool2'\n"); + ExpectError error(r, "type mismatch: '*=' cannot operate on 'float', 'bool2'"); DSLExpression(a *= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1.0 *= a).release(); } } @@ -788,28 +788,28 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLDivide, r, ctxInfo) { "(a /= (b + 1.0))"); { - ExpectError error(r, "error: type mismatch: '/' cannot operate on 'bool2', 'float'\n"); + ExpectError error(r, "type mismatch: '/' cannot operate on 'bool2', 'float'"); DSLExpression(Bool2(true) / a).release(); } { - ExpectError error(r, "error: type mismatch: '/=' cannot operate on 'float', 'bool2'\n"); + ExpectError error(r, "type mismatch: '/=' cannot operate on 'float', 'bool2'"); DSLExpression(a /= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1.0 /= a).release(); } { - ExpectError error(r, "error: division by zero\n"); + ExpectError error(r, "division by zero"); DSLExpression(a /= 0).release(); } { Var c(kFloat2_Type, "c"); - ExpectError error(r, "error: division by zero\n"); + ExpectError error(r, "division by zero"); DSLExpression(c /= Float2(Float(0), 1)).release(); } } @@ -830,28 +830,28 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLMod, r, ctxInfo) { EXPECT_EQUAL(e4, "(a %= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '%' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '%' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) % a).release(); } { - ExpectError error(r, "error: type mismatch: '%=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '%=' cannot operate on 'int', 'bool2'"); DSLExpression(a %= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 %= a).release(); } { - ExpectError error(r, "error: division by zero\n"); + ExpectError error(r, "division by zero"); DSLExpression(a %= 0).release(); } { Var c(kInt2_Type, "c"); - ExpectError error(r, "error: division by zero\n"); + ExpectError error(r, "division by zero"); DSLExpression(c %= Int2(Int(0), 1)).release(); } } @@ -872,17 +872,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLShl, r, ctxInfo) { EXPECT_EQUAL(e4, "(a <<= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '<<' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '<<' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) << a).release(); } { - ExpectError error(r, "error: type mismatch: '<<=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '<<=' cannot operate on 'int', 'bool2'"); DSLExpression(a <<= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 <<= a).release(); } } @@ -903,17 +903,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLShr, r, ctxInfo) { EXPECT_EQUAL(e4, "(a >>= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '>>' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '>>' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) >> a).release(); } { - ExpectError error(r, "error: type mismatch: '>>=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '>>=' cannot operate on 'int', 'bool2'"); DSLExpression(a >>= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 >>= a).release(); } } @@ -934,17 +934,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBitwiseAnd, r, ctxInfo) { EXPECT_EQUAL(e4, "(a &= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '&' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '&' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) & a).release(); } { - ExpectError error(r, "error: type mismatch: '&=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '&=' cannot operate on 'int', 'bool2'"); DSLExpression(a &= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 &= a).release(); } } @@ -965,17 +965,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBitwiseOr, r, ctxInfo) { EXPECT_EQUAL(e4, "(a |= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '|' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '|' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) | a).release(); } { - ExpectError error(r, "error: type mismatch: '|=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '|=' cannot operate on 'int', 'bool2'"); DSLExpression(a |= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 |= a).release(); } } @@ -996,17 +996,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBitwiseXor, r, ctxInfo) { EXPECT_EQUAL(e4, "(a ^= (b + 1))"); { - ExpectError error(r, "error: type mismatch: '^' cannot operate on 'bool2', 'int'\n"); + ExpectError error(r, "type mismatch: '^' cannot operate on 'bool2', 'int'"); DSLExpression(Bool2(true) ^ a).release(); } { - ExpectError error(r, "error: type mismatch: '^=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '^=' cannot operate on 'int', 'bool2'"); DSLExpression(a ^= Bool2(true)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(1 ^= a).release(); } } @@ -1024,7 +1024,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLogicalAnd, r, ctxInfo) { EXPECT_EQUAL(e3, "false"); { - ExpectError error(r, "error: type mismatch: '&&' cannot operate on 'bool', 'int'\n"); + ExpectError error(r, "type mismatch: '&&' cannot operate on 'bool', 'int'"); DSLExpression(a && 5).release(); } } @@ -1042,7 +1042,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLogicalOr, r, ctxInfo) { EXPECT_EQUAL(e3, "(a || b)"); { - ExpectError error(r, "error: type mismatch: '||' cannot operate on 'bool', 'int'\n"); + ExpectError error(r, "type mismatch: '||' cannot operate on 'bool', 'int'"); DSLExpression(a || 5).release(); } } @@ -1054,7 +1054,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLogicalXor, r, ctxInfo) { EXPECT_EQUAL(e1, "(a ^^ b)"); { - ExpectError error(r, "error: type mismatch: '^^' cannot operate on 'bool', 'int'\n"); + ExpectError error(r, "type mismatch: '^^' cannot operate on 'bool', 'int'"); DSLExpression(LogicalXor(a, 5)).release(); } } @@ -1079,7 +1079,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLEqual, r, ctxInfo) { EXPECT_EQUAL(e2, "(a == 5)"); { - ExpectError error(r, "error: type mismatch: '==' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '==' cannot operate on 'int', 'bool2'"); DSLExpression(a == Bool2(true)).release(); } } @@ -1094,7 +1094,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLNotEqual, r, ctxInfo) { EXPECT_EQUAL(e2, "(a != 5)"); { - ExpectError error(r, "error: type mismatch: '!=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '!=' cannot operate on 'int', 'bool2'"); DSLExpression(a != Bool2(true)).release(); } } @@ -1109,7 +1109,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLGreaterThan, r, ctxInfo) { EXPECT_EQUAL(e2, "(a > 5)"); { - ExpectError error(r, "error: type mismatch: '>' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '>' cannot operate on 'int', 'bool2'"); DSLExpression(a > Bool2(true)).release(); } } @@ -1124,7 +1124,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLGreaterThanOrEqual, r, ctxInfo) { EXPECT_EQUAL(e2, "(a >= 5)"); { - ExpectError error(r, "error: type mismatch: '>=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '>=' cannot operate on 'int', 'bool2'"); DSLExpression(a >= Bool2(true)).release(); } } @@ -1139,7 +1139,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLessThan, r, ctxInfo) { EXPECT_EQUAL(e2, "(a < 5)"); { - ExpectError error(r, "error: type mismatch: '<' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '<' cannot operate on 'int', 'bool2'"); DSLExpression(a < Bool2(true)).release(); } } @@ -1154,7 +1154,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLessThanOrEqual, r, ctxInfo) { EXPECT_EQUAL(e2, "(a <= 5)"); { - ExpectError error(r, "error: type mismatch: '<=' cannot operate on 'int', 'bool2'\n"); + ExpectError error(r, "type mismatch: '<=' cannot operate on 'int', 'bool2'"); DSLExpression(a <= Bool2(true)).release(); } } @@ -1166,7 +1166,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLogicalNot, r, ctxInfo) { EXPECT_EQUAL(e1, "!(a <= b)"); { - ExpectError error(r, "error: '!' cannot operate on 'int'\n"); + ExpectError error(r, "'!' cannot operate on 'int'"); DSLExpression(!a).release(); } } @@ -1178,7 +1178,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBitwiseNot, r, ctxInfo) { EXPECT_EQUAL(e1, "~a"); { - ExpectError error(r, "error: '~' cannot operate on 'bool'\n"); + ExpectError error(r, "'~' cannot operate on 'bool'"); DSLExpression(~b).release(); } } @@ -1193,22 +1193,22 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLIncrement, r, ctxInfo) { EXPECT_EQUAL(e2, "a++"); { - ExpectError error(r, "error: '++' cannot operate on 'bool'\n"); + ExpectError error(r, "'++' cannot operate on 'bool'"); DSLExpression(++b).release(); } { - ExpectError error(r, "error: '++' cannot operate on 'bool'\n"); + ExpectError error(r, "'++' cannot operate on 'bool'"); DSLExpression(b++).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(++(a + 1)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression((a + 1)++).release(); } } @@ -1223,22 +1223,22 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLDecrement, r, ctxInfo) { EXPECT_EQUAL(e2, "a--"); { - ExpectError error(r, "error: '--' cannot operate on 'bool'\n"); + ExpectError error(r, "'--' cannot operate on 'bool'"); DSLExpression(--b).release(); } { - ExpectError error(r, "error: '--' cannot operate on 'bool'\n"); + ExpectError error(r, "'--' cannot operate on 'bool'"); DSLExpression(b--).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression(--(a + 1)).release(); } { - ExpectError error(r, "error: cannot assign to this expression\n"); + ExpectError error(r, "cannot assign to this expression"); DSLExpression((a + 1)--).release(); } } @@ -1290,7 +1290,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBreak, r, ctxInfo) { "void success() { for (int i = 0; (i < 10); ++i) { if ((i > 5)) break; } }"); { - ExpectError error(r, "error: break statement must be inside a loop or switch\n"); + ExpectError error(r, "break statement must be inside a loop or switch"); DSLFunction(kVoid_Type, "fail").define( Break() ); @@ -1310,7 +1310,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLContinue, r, ctxInfo) { "void success() { for (int i = 0; (i < 10); ++i) { if ((i < 5)) continue; } }"); { - ExpectError error(r, "error: continue statement must be inside a loop\n"); + ExpectError error(r, "continue statement must be inside a loop"); DSLFunction(kVoid_Type, "fail").define( Continue() ); @@ -1359,7 +1359,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLDeclare, r, ctxInfo) { { DSLWriter::Reset(); Var a(kHalf4_Type, "a", 1); - ExpectError error(r, "error: expected 'half4', but found 'int'\n"); + ExpectError error(r, "expected 'half4', but found 'int'"); Declare(a).release(); } @@ -1367,14 +1367,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLDeclare, r, ctxInfo) { DSLWriter::Reset(); Var a(kInt_Type, "a"); Declare(a).release(); - ExpectError error(r, "error: variable has already been declared\n"); + ExpectError error(r, "variable has already been declared"); Declare(a).release(); } { DSLWriter::Reset(); Var a(kUniform_Modifier, kInt_Type, "a"); - ExpectError error(r, "error: 'uniform' is not permitted here\n"); + ExpectError error(r, "'uniform' is not permitted here"); Declare(a).release(); } } @@ -1406,7 +1406,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLDo, r, ctxInfo) { EXPECT_EQUAL(y, "do { a++; --b; } while ((a != b));"); { - ExpectError error(r, "error: expected 'bool', but found 'int'\n"); + ExpectError error(r, "expected 'bool', but found 'int'"); Do(Block(), 7).release(); } } @@ -1431,12 +1431,12 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFor, r, ctxInfo) { )"); { - ExpectError error(r, "error: expected 'bool', but found 'int'\n"); + ExpectError error(r, "expected 'bool', but found 'int'"); For(i = 0, i + 10, ++i, i += 5).release(); } { - ExpectError error(r, "error: invalid for loop initializer\n"); + ExpectError error(r, "invalid for loop initializer"); For(If(i == 0, i = 1), i < 10, ++i, i += 5).release(); } } @@ -1494,7 +1494,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) { } { - ExpectError error(r, "error: expected 'float', but found 'bool'\n"); + ExpectError error(r, "expected 'float', but found 'bool'"); DSLWriter::Reset(); DSLFunction(kFloat_Type, "broken").define( Return(true) @@ -1502,7 +1502,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) { } { - ExpectError error(r, "error: expected function to return 'float'\n"); + ExpectError error(r, "expected function to return 'float'"); DSLWriter::Reset(); DSLFunction(kFloat_Type, "broken").define( Return() @@ -1510,7 +1510,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) { } { - ExpectError error(r, "error: function 'broken' can exit without returning a value\n"); + ExpectError error(r, "function 'broken' can exit without returning a value"); DSLWriter::Reset(); Var x(kFloat_Type, "x", 0); DSLFunction(kFloat_Type, "broken").define( @@ -1520,7 +1520,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) { } { - ExpectError error(r, "error: may not return a value from a void function\n"); + ExpectError error(r, "may not return a value from a void function"); DSLWriter::Reset(); DSLFunction(kVoid_Type, "broken").define( Return(0) @@ -1528,14 +1528,14 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) { } { - ExpectError error(r, "error: function 'broken' can exit without returning a value\n"); + ExpectError error(r, "function 'broken' can exit without returning a value"); DSLWriter::Reset(); DSLFunction(kFloat_Type, "broken").define( ); } { - ExpectError error(r, "error: parameter has already been used in another function\n"); + ExpectError error(r, "parameter has already been used in another function"); DSLWriter::Reset(); DSLParameter p(kFloat_Type); DSLFunction(kVoid_Type, "ok", p).define( @@ -1558,7 +1558,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLIf, r, ctxInfo) { EXPECT_EQUAL(z, "@if ((a > b)) (a -= b); else (b -= a);"); { - ExpectError error(r, "error: expected 'bool', but found 'float'\n"); + ExpectError error(r, "expected 'bool', but found 'float'"); If(a + b, a -= b).release(); } } @@ -1605,12 +1605,12 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLSelect, r, ctxInfo) { EXPECT_EQUAL(x, "((a > 0) ? 1 : -1)"); { - ExpectError error(r, "error: expected 'bool', but found 'int'\n"); + ExpectError error(r, "expected 'bool', but found 'int'"); Select(a, 1, -1).release(); } { - ExpectError error(r, "error: ternary operator result mismatch: 'float2', 'float3'\n"); + ExpectError error(r, "ternary operator result mismatch: 'float2', 'float3'"); Select(a > 0, Float2(1), Float3(1)).release(); } } @@ -1660,17 +1660,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLSwitch, r, ctxInfo) { "switch (b) { default: case 0: case 1: }"); { - ExpectError error(r, "error: duplicate case value '0'\n"); + ExpectError error(r, "duplicate case value '0'"); DSLStatement(Switch(0, Case(0), Case(0))).release(); } { - ExpectError error(r, "error: duplicate default case\n"); + ExpectError error(r, "duplicate default case"); DSLStatement(Switch(0, Default(a = 0), Default(a = 1))).release(); } { - ExpectError error(r, "error: case value must be a constant integer\n"); + ExpectError error(r, "case value must be a constant integer"); Var c(kInt_Type); DSLStatement(Switch(0, Case(c))).release(); } @@ -1733,7 +1733,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLWhile, r, ctxInfo) { EXPECT_EQUAL(y, "for (; (a != b);) { a++; --b; }"); { - ExpectError error(r, "error: expected 'bool', but found 'int'\n"); + ExpectError error(r, "expected 'bool', but found 'int'"); While(7, Block()).release(); } } @@ -1746,17 +1746,17 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLIndex, r, ctxInfo) { EXPECT_EQUAL(a[b], "a[b]"); { - ExpectError error(r, "error: expected 'int', but found 'bool'\n"); + ExpectError error(r, "expected 'int', but found 'bool'"); a[true].release(); } { - ExpectError error(r, "error: expected array, but found 'int'\n"); + ExpectError error(r, "expected array, but found 'int'"); b[0].release(); } { - ExpectError error(r, "error: index -1 out of range for 'int[5]'\n"); + ExpectError error(r, "index -1 out of range for 'int[5]'"); a[-1].release(); } } @@ -1816,7 +1816,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLBuiltins, r, ctxInfo) { // these calls all go through the normal channels, so it ought to be sufficient to prove that // one of them reports errors correctly { - ExpectError error(r, "error: no match for ceil(bool)\n"); + ExpectError error(r, "no match for ceil(bool)"); Ceil(a == b).release(); } } @@ -1881,67 +1881,65 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLayout, r, ctxInfo) { EXPECT_EQUAL(Declare(v5), "layout (blend_support_all_equations) half4 v5;"); { - ExpectError error(r, "error: 'srgb_unpremul' is only permitted in runtime effects\n"); + ExpectError error(r, "'srgb_unpremul' is only permitted in runtime effects"); DSLGlobalVar v(DSLModifiers(DSLLayout().srgbUnpremul(), kUniform_Modifier), kHalf4_Type, "v"); Declare(v); } { - ExpectError error(r, "error: layout qualifier 'location' appears more than once\n"); + ExpectError error(r, "layout qualifier 'location' appears more than once"); DSLLayout().location(1).location(2); } { - ExpectError error(r, "error: layout qualifier 'set' appears more than once\n"); + ExpectError error(r, "layout qualifier 'set' appears more than once"); DSLLayout().set(1).set(2); } { - ExpectError error(r, "error: layout qualifier 'binding' appears more than once\n"); + ExpectError error(r, "layout qualifier 'binding' appears more than once"); DSLLayout().binding(1).binding(2); } { - ExpectError error(r, "error: layout qualifier 'offset' appears more than once\n"); + ExpectError error(r, "layout qualifier 'offset' appears more than once"); DSLLayout().offset(1).offset(2); } { - ExpectError error(r, "error: layout qualifier 'index' appears more than once\n"); + ExpectError error(r, "layout qualifier 'index' appears more than once"); DSLLayout().index(1).index(2); } { - ExpectError error(r, "error: layout qualifier 'builtin' appears more than once\n"); + ExpectError error(r, "layout qualifier 'builtin' appears more than once"); DSLLayout().builtin(1).builtin(2); } { - ExpectError error(r, "error: layout qualifier 'input_attachment_index' appears more than " - "once\n"); + ExpectError error(r, "layout qualifier 'input_attachment_index' appears more than once"); DSLLayout().inputAttachmentIndex(1).inputAttachmentIndex(2); } { - ExpectError error(r, "error: layout qualifier 'origin_upper_left' appears more than " - "once\n"); + ExpectError error(r, "layout qualifier 'origin_upper_left' appears more than once"); DSLLayout().originUpperLeft().originUpperLeft(); } { - ExpectError error(r, "error: layout qualifier 'push_constant' appears more than once\n"); + ExpectError error(r, "layout qualifier 'push_constant' appears more than once"); DSLLayout().pushConstant().pushConstant(); } { - ExpectError error(r, "error: layout qualifier 'blend_support_all_equations' appears more " - "than once\n"); + ExpectError error(r, "layout qualifier 'blend_support_all_equations' appears more than " + "once"); DSLLayout().blendSupportAllEquations().blendSupportAllEquations(); } { - ExpectError error(r, "error: layout qualifier 'srgb_unpremul' appears more than once\n"); + ExpectError error(r, "layout qualifier 'srgb_unpremul' appears more than once"); DSLLayout().srgbUnpremul().srgbUnpremul(); } } @@ -1953,7 +1951,7 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLSampleShader, r, ctxInfo) { EXPECT_EQUAL(Sample(shader, Float2(0, 0)), "sample(shader, float2(0.0, 0.0))"); { - ExpectError error(r, "error: no match for sample(shader, half4)\n"); + ExpectError error(r, "no match for sample(shader, half4)"); Sample(shader, Half4(1)).release(); } } diff --git a/tests/SkSLMemoryLayoutTest.cpp b/tests/SkSLMemoryLayoutTest.cpp index 526d8a0a88..e6a50229d3 100644 --- a/tests/SkSLMemoryLayoutTest.cpp +++ b/tests/SkSLMemoryLayoutTest.cpp @@ -5,8 +5,8 @@ * found in the LICENSE file. */ +#include "include/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLContext.h" -#include "src/sksl/SkSLErrorReporter.h" #include "src/sksl/SkSLMemoryLayout.h" #include "tests/Test.h" diff --git a/tests/SkSLTypeTest.cpp b/tests/SkSLTypeTest.cpp index c10361e196..338896d5d9 100644 --- a/tests/SkSLTypeTest.cpp +++ b/tests/SkSLTypeTest.cpp @@ -7,9 +7,9 @@ #include +#include "include/sksl/SkSLErrorReporter.h" #include "src/gpu/GrCaps.h" #include "src/sksl/SkSLContext.h" -#include "src/sksl/SkSLErrorReporter.h" #include "tests/Test.h" DEF_TEST(SkSLTypeLimits, r) {