Pass dsl::PositionInfo by value
We were previously using a mix of pass-by-value and pass-by-pointer (to
allow for explicitly null PositionInfo). Being able to pass a null
PositionInfo didn't really add much, since we can just use a nullary-
constructor PositionInfo instead, so these have all been migrated to
by-value.
Change-Id: Ia31e252cac94f64c4b38c29a54e6e7f752e70672
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437276
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/include/sksl/DSLErrorHandling.h b/include/sksl/DSLErrorHandling.h
index 3b9e8ae..b287e43 100644
--- a/include/sksl/DSLErrorHandling.h
+++ b/include/sksl/DSLErrorHandling.h
@@ -49,7 +49,7 @@
/**
* 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;
+ virtual void handleError(const char* msg, PositionInfo position) = 0;
};
} // namespace dsl
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index aa8d115..b3ea6cd 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -304,7 +304,7 @@
// 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; }
+ void handleError(const char*, dsl::PositionInfo) override { ++fErrorCount; }
int errorCount() override { return fErrorCount; }
void setErrorCount(int c) override { fErrorCount = c; }
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 27557c6..90fdf94 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -986,15 +986,14 @@
#endif // defined(SKSL_STANDALONE) || SK_SUPPORT_GPU
-void Compiler::handleError(const char* msg, dsl::PositionInfo* pos) {
+void Compiler::handleError(const char* msg, dsl::PositionInfo pos) {
if (strstr(msg, POISON_TAG)) {
// don't report errors on poison values
return;
}
fErrorCount++;
fErrorTextLength.push_back(fErrorText.length());
- fErrorText += "error: " + (pos && pos->line() >= 1 ? to_string(pos->line()) + ": " : "") + msg +
- "\n";
+ fErrorText += "error: " + (pos.line() >= 1 ? to_string(pos.line()) + ": " : "") + msg + "\n";
}
void Compiler::setErrorCount(int c) {
diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h
index bb1e34e..868db07 100644
--- a/src/sksl/SkSLCompiler.h
+++ b/src/sksl/SkSLCompiler.h
@@ -161,7 +161,7 @@
bool toMetal(Program& program, String* out);
- void handleError(const char* msg, dsl::PositionInfo* pos) override;
+ void handleError(const char* msg, dsl::PositionInfo pos) override;
String errorText(bool showCount = true);
diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp
index 1b46bcd..bca062c 100644
--- a/src/sksl/SkSLDSLParser.cpp
+++ b/src/sksl/SkSLDSLParser.cpp
@@ -215,7 +215,7 @@
}
}
- void handleError(const char* msg, PositionInfo* position) override {
+ void handleError(const char* msg, PositionInfo position) override {
fErrors.push_back(msg);
}
diff --git a/src/sksl/SkSLErrorReporter.h b/src/sksl/SkSLErrorReporter.h
index 08027d1..cb8dd7c 100644
--- a/src/sksl/SkSLErrorReporter.h
+++ b/src/sksl/SkSLErrorReporter.h
@@ -27,13 +27,11 @@
/** Reports an error message at the given character offset of the source text. */
void error(int offset, String msg) {
- dsl::PositionInfo pos = this->position(offset);
- this->handleError(msg.c_str(), &pos);
+ this->handleError(msg.c_str(), this->position(offset));
}
void error(int offset, const char* msg) {
- dsl::PositionInfo pos = this->position(offset);
- this->handleError(msg, &pos);
+ this->handleError(msg, this->position(offset));
}
/** Returns the number of errors that have been reported. */
@@ -69,7 +67,7 @@
*/
class TestingOnly_AbortErrorReporter : public ErrorReporter {
public:
- void handleError(const char* msg, dsl::PositionInfo* pos) override { SK_ABORT("%s", msg); }
+ void handleError(const char* msg, dsl::PositionInfo pos) override { SK_ABORT("%s", msg); }
int errorCount() override { return 0; }
void setErrorCount(int) override {}
};
diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp
index 94f6416..9701a5c 100644
--- a/src/sksl/dsl/DSLCore.cpp
+++ b/src/sksl/dsl/DSLCore.cpp
@@ -135,7 +135,7 @@
static DSLStatement Declare(DSLVar& var, PositionInfo pos) {
if (var.fDeclared) {
- DSLWriter::ReportError("error: variable has already been declared\n", &pos);
+ DSLWriter::ReportError("error: variable has already been declared\n", pos);
}
var.fDeclared = true;
return DSLWriter::Declaration(var);
@@ -151,7 +151,7 @@
static void Declare(DSLGlobalVar& var, PositionInfo pos) {
if (var.fDeclared) {
- DSLWriter::ReportError("error: variable has already been declared\n", &pos);
+ DSLWriter::ReportError("error: variable has already been declared\n", pos);
}
var.fDeclared = true;
std::unique_ptr<SkSL::Statement> stmt = DSLWriter::Declaration(var);
diff --git a/src/sksl/dsl/DSLLayout.cpp b/src/sksl/dsl/DSLLayout.cpp
index 3d70e60..001da33 100644
--- a/src/sksl/dsl/DSLLayout.cpp
+++ b/src/sksl/dsl/DSLLayout.cpp
@@ -16,7 +16,7 @@
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);
+ "' appears more than once\n").c_str(), pos);
}
fSkSLLayout.fFlags |= mask;
return *this;
diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp
index edef9e0..4da52be 100644
--- a/src/sksl/dsl/priv/DSLWriter.cpp
+++ b/src/sksl/dsl/priv/DSLWriter.cpp
@@ -208,16 +208,13 @@
IRGenerator().fSymbolTable);
}
-void DSLWriter::ReportError(const char* msg, PositionInfo* info) {
+void DSLWriter::ReportError(const char* msg, PositionInfo info) {
Instance().fEncounteredErrors = true;
- if (info && !info->file_name()) {
- info = nullptr;
- }
if (Instance().fErrorHandler) {
Instance().fErrorHandler->handleError(msg, info);
- } else if (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);
+ info.file_name(), info.line(), msg);
} else {
SK_ABORT("%sNo SkSL DSL error handler configured, treating this as a fatal error\n", msg);
}
@@ -284,7 +281,7 @@
void DSLWriter::ReportErrors(PositionInfo pos) {
if (Compiler().errorCount()) {
- ReportError(DSLWriter::Compiler().errorText(/*showCount=*/false).c_str(), &pos);
+ ReportError(DSLWriter::Compiler().errorText(/*showCount=*/false).c_str(), pos);
Compiler().setErrorCount(0);
}
}
diff --git a/src/sksl/dsl/priv/DSLWriter.h b/src/sksl/dsl/priv/DSLWriter.h
index 0af14d7..9665d6c 100644
--- a/src/sksl/dsl/priv/DSLWriter.h
+++ b/src/sksl/dsl/priv/DSLWriter.h
@@ -235,7 +235,7 @@
* Notifies the current ErrorHandler that a DSL error has occurred. With a null ErrorHandler
* (the default), any errors will be dumped to stderr and a fatal exception will be generated.
*/
- static void ReportError(const char* msg, PositionInfo* info = nullptr);
+ static void ReportError(const char* msg, PositionInfo info = PositionInfo());
/**
* Returns whether name mangling is enabled. Mangling is important for the DSL because its
diff --git a/tests/SkDSLRuntimeEffectTest.cpp b/tests/SkDSLRuntimeEffectTest.cpp
index 3f283af..838875f 100644
--- a/tests/SkDSLRuntimeEffectTest.cpp
+++ b/tests/SkDSLRuntimeEffectTest.cpp
@@ -202,7 +202,7 @@
{
class SimpleErrorHandler : public ErrorHandler {
public:
- void handleError(const char* msg, PositionInfo* pos) override {
+ void handleError(const char* msg, PositionInfo pos) override {
fMsg = msg;
}
diff --git a/tests/SkSLDSLErrorLineNumbers.cpp b/tests/SkSLDSLErrorLineNumbers.cpp
index 08ca63f..ecd7c4a 100644
--- a/tests/SkSLDSLErrorLineNumbers.cpp
+++ b/tests/SkSLDSLErrorLineNumbers.cpp
@@ -34,12 +34,11 @@
SetErrorHandler(nullptr);
}
- void handleError(const char* msg, PositionInfo* pos) override {
+ void handleError(const char* msg, PositionInfo pos) override {
REPORTER_ASSERT(fReporter, !strcmp(msg, fMsg),
"Error mismatch: expected:\n%sbut received:\n%s", fMsg, msg);
- REPORTER_ASSERT(fReporter, pos);
- REPORTER_ASSERT(fReporter, pos->line() == fLine,
- "Line number mismatch: expected %d, but received %d\n", fLine, pos->line());
+ REPORTER_ASSERT(fReporter, pos.line() == fLine,
+ "Line number mismatch: expected %d, but received %d\n", fLine, pos.line());
fMsg = nullptr;
}
diff --git a/tests/SkSLDSLTest.cpp b/tests/SkSLDSLTest.cpp
index c961115d..94095f7 100644
--- a/tests/SkSLDSLTest.cpp
+++ b/tests/SkSLDSLTest.cpp
@@ -64,7 +64,7 @@
SetErrorHandler(nullptr);
}
- void handleError(const char* msg, PositionInfo* pos) override {
+ void handleError(const char* msg, PositionInfo pos) override {
REPORTER_ASSERT(fReporter, !strcmp(msg, fMsg),
"Error mismatch: expected:\n%sbut received:\n%s", fMsg, msg);
fMsg = nullptr;
