[tint][glsl] Use tint::Result for Generate()
This is more consistent with the rest of the codebase.
Also renames `writer::Result` to `writer::Output` to avoid clashing
with tint::Result.
Change-Id: I1c96e1c199d0cb0d795da9c93a6a38c0e143ee15
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/144263
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index ee84ba74..a8cb310 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -268,11 +268,11 @@
tintOptions.allow_collisions = true;
auto result = tint::glsl::writer::Generate(&program, tintOptions, r.entryPointName);
- DAWN_INVALID_IF(!result.success, "An error occured while generating GLSL: %s.",
- result.error);
+ DAWN_INVALID_IF(!result, "An error occured while generating GLSL: %s.",
+ result.Failure());
return GLSLCompilation{
- {std::move(result.glsl), needsPlaceholderSampler, std::move(combinedSamplerInfo)}};
+ {std::move(result->glsl), needsPlaceholderSampler, std::move(combinedSamplerInfo)}};
},
"OpenGL.CompileShaderToGLSL");
diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn
index 992d13d..29b342a 100644
--- a/src/tint/BUILD.gn
+++ b/src/tint/BUILD.gn
@@ -1216,8 +1216,8 @@
"lang/glsl/writer/common/options.cc",
"lang/glsl/writer/common/options.h",
"lang/glsl/writer/common/version.h",
- "lang/glsl/writer/result.cc",
- "lang/glsl/writer/result.h",
+ "lang/glsl/writer/output.cc",
+ "lang/glsl/writer/output.h",
"lang/glsl/writer/writer.cc",
"lang/glsl/writer/writer.h",
]
diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt
index 9f3e025..39b7ade 100644
--- a/src/tint/CMakeLists.txt
+++ b/src/tint/CMakeLists.txt
@@ -743,8 +743,8 @@
lang/glsl/writer/common/options.cc
lang/glsl/writer/common/options.h
lang/glsl/writer/common/version.h
- lang/glsl/writer/result.cc
- lang/glsl/writer/result.h
+ lang/glsl/writer/output.cc
+ lang/glsl/writer/output.h
lang/glsl/writer/writer.cc
lang/glsl/writer/writer.h
)
diff --git a/src/tint/cmd/loopy.cc b/src/tint/cmd/loopy.cc
index 118bc08..88b79ff 100644
--- a/src/tint/cmd/loopy.cc
+++ b/src/tint/cmd/loopy.cc
@@ -270,9 +270,9 @@
gen_options.external_texture_options.bindings_map =
tint::cmd::GenerateExternalTextureBindings(program);
auto result = tint::glsl::writer::Generate(program, gen_options, "");
- if (!result.success) {
+ if (result) {
tint::cmd::PrintWGSL(std::cerr, *program);
- std::cerr << "Failed to generate: " << result.error << std::endl;
+ std::cerr << "Failed to generate: " << result.Failure() << std::endl;
return false;
}
diff --git a/src/tint/cmd/main.cc b/src/tint/cmd/main.cc
index 88d88a4..eee7908 100644
--- a/src/tint/cmd/main.cc
+++ b/src/tint/cmd/main.cc
@@ -833,27 +833,27 @@
gen_options.external_texture_options.bindings_map =
tint::cmd::GenerateExternalTextureBindings(prg);
auto result = tint::glsl::writer::Generate(prg, gen_options, entry_point_name);
- if (!result.success) {
+ if (!result) {
tint::cmd::PrintWGSL(std::cerr, *prg);
- std::cerr << "Failed to generate: " << result.error << std::endl;
+ std::cerr << "Failed to generate: " << result.Failure() << std::endl;
return false;
}
- if (!WriteFile(options.output_file, "w", result.glsl)) {
+ if (!WriteFile(options.output_file, "w", result->glsl)) {
return false;
}
- const auto hash = tint::CRC32(result.glsl.c_str());
+ const auto hash = tint::CRC32(result->glsl.c_str());
if (options.print_hash) {
PrintHash(hash);
}
if (options.validate && options.skip_hash.count(hash) == 0) {
- for (auto entry_pt : result.entry_points) {
+ for (auto entry_pt : result->entry_points) {
EShLanguage lang = pipeline_stage_to_esh_language(entry_pt.second);
glslang::TShader shader(lang);
- const char* strings[1] = {result.glsl.c_str()};
- int lengths[1] = {static_cast<int>(result.glsl.length())};
+ const char* strings[1] = {result->glsl.c_str()};
+ int lengths[1] = {static_cast<int>(result->glsl.length())};
shader.setStringsWithLengths(strings, lengths, 1);
shader.setEntryPoint("main");
bool glslang_result = shader.parse(GetDefaultResources(), 310, EEsProfile, false,
diff --git a/src/tint/fuzzers/tint_concurrency_fuzzer.cc b/src/tint/fuzzers/tint_concurrency_fuzzer.cc
index f09524d..0031c6b 100644
--- a/src/tint/fuzzers/tint_concurrency_fuzzer.cc
+++ b/src/tint/fuzzers/tint_concurrency_fuzzer.cc
@@ -102,7 +102,7 @@
#if TINT_BUILD_GLSL_WRITER
case Writer::kGLSL: {
- tint::glsl::writer::Generate(&program, {}, entry_point);
+ (void)tint::glsl::writer::Generate(&program, {}, entry_point);
break;
}
#endif // TINT_BUILD_GLSL_WRITER
diff --git a/src/tint/lang/glsl/writer/ast_printer/ast_printer.cc b/src/tint/lang/glsl/writer/ast_printer/ast_printer.cc
index 3a1a367..6389ed8 100644
--- a/src/tint/lang/glsl/writer/ast_printer/ast_printer.cc
+++ b/src/tint/lang/glsl/writer/ast_printer/ast_printer.cc
@@ -260,7 +260,7 @@
ASTPrinter::~ASTPrinter() = default;
-void ASTPrinter::Generate() {
+bool ASTPrinter::Generate() {
{
auto out = Line();
out << "#version " << version_.major_version << version_.minor_version << "0";
@@ -337,6 +337,8 @@
current_buffer_->Insert(helpers_, helpers_insertion_point, indent);
helpers_insertion_point += helpers_.lines.size();
}
+
+ return !diagnostics_.contains_errors();
}
void ASTPrinter::RecordExtension(const ast::Enable* enable) {
diff --git a/src/tint/lang/glsl/writer/ast_printer/ast_printer.h b/src/tint/lang/glsl/writer/ast_printer/ast_printer.h
index d670dcf..4a34a4e 100644
--- a/src/tint/lang/glsl/writer/ast_printer/ast_printer.h
+++ b/src/tint/lang/glsl/writer/ast_printer/ast_printer.h
@@ -74,7 +74,8 @@
~ASTPrinter() override;
/// Generates the GLSL shader
- void Generate();
+ /// @returns true on successful generation, false otherwise
+ bool Generate();
/// Record an extension directive within the generator
/// @param ext the extension to record
diff --git a/src/tint/lang/glsl/writer/ast_printer/ast_printer_test.cc b/src/tint/lang/glsl/writer/ast_printer/ast_printer_test.cc
index 17a6349..ee9da34 100644
--- a/src/tint/lang/glsl/writer/ast_printer/ast_printer_test.cc
+++ b/src/tint/lang/glsl/writer/ast_printer/ast_printer_test.cc
@@ -27,7 +27,8 @@
auto program = std::make_unique<Program>(resolver::Resolve(*this));
ASSERT_FALSE(program->IsValid());
auto result = Generate(program.get(), Options{}, "");
- EXPECT_EQ(result.error, "input program is not valid");
+ EXPECT_FALSE(result);
+ EXPECT_EQ(result.Failure(), "input program is not valid");
}
TEST_F(GlslASTPrinterTest, Generate) {
diff --git a/src/tint/lang/glsl/writer/result.cc b/src/tint/lang/glsl/writer/output.cc
similarity index 82%
rename from src/tint/lang/glsl/writer/result.cc
rename to src/tint/lang/glsl/writer/output.cc
index d891e40..2e340d7 100644
--- a/src/tint/lang/glsl/writer/result.cc
+++ b/src/tint/lang/glsl/writer/output.cc
@@ -12,14 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include "src/tint/lang/glsl/writer/result.h"
+#include "src/tint/lang/glsl/writer/output.h"
namespace tint::glsl::writer {
-Result::Result() = default;
+Output::Output() = default;
-Result::~Result() = default;
+Output::~Output() = default;
-Result::Result(const Result&) = default;
+Output::Output(const Output&) = default;
} // namespace tint::glsl::writer
diff --git a/src/tint/lang/glsl/writer/result.h b/src/tint/lang/glsl/writer/output.h
similarity index 71%
rename from src/tint/lang/glsl/writer/result.h
rename to src/tint/lang/glsl/writer/output.h
index 043f664..084b0ef 100644
--- a/src/tint/lang/glsl/writer/result.h
+++ b/src/tint/lang/glsl/writer/output.h
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef SRC_TINT_LANG_GLSL_WRITER_RESULT_H_
-#define SRC_TINT_LANG_GLSL_WRITER_RESULT_H_
+#ifndef SRC_TINT_LANG_GLSL_WRITER_OUTPUT_H_
+#define SRC_TINT_LANG_GLSL_WRITER_OUTPUT_H_
#include <string>
#include <utility>
@@ -23,22 +23,16 @@
namespace tint::glsl::writer {
-/// The result produced when generating GLSL.
-struct Result {
+/// The output produced when generating GLSL.
+struct Output {
/// Constructor
- Result();
+ Output();
/// Destructor
- ~Result();
+ ~Output();
/// Copy constructor
- Result(const Result&);
-
- /// True if generation was successful.
- bool success = false;
-
- /// The errors generated during code generation, if any.
- std::string error;
+ Output(const Output&);
/// The generated GLSL.
std::string glsl = "";
@@ -49,4 +43,4 @@
} // namespace tint::glsl::writer
-#endif // SRC_TINT_LANG_GLSL_WRITER_RESULT_H_
+#endif // SRC_TINT_LANG_GLSL_WRITER_OUTPUT_H_
diff --git a/src/tint/lang/glsl/writer/writer.cc b/src/tint/lang/glsl/writer/writer.cc
index 634282b..4124346 100644
--- a/src/tint/lang/glsl/writer/writer.cc
+++ b/src/tint/lang/glsl/writer/writer.cc
@@ -22,37 +22,37 @@
namespace tint::glsl::writer {
-Result Generate(const Program* program, const Options& options, const std::string& entry_point) {
- Result result;
+Result<Output, std::string> Generate(const Program* program,
+ const Options& options,
+ const std::string& entry_point) {
if (!program->IsValid()) {
- result.error = "input program is not valid";
- return result;
+ return std::string("input program is not valid");
}
// Sanitize the program.
auto sanitized_result = Sanitize(program, options, entry_point);
if (!sanitized_result.program.IsValid()) {
- result.success = false;
- result.error = sanitized_result.program.Diagnostics().str();
- return result;
+ return sanitized_result.program.Diagnostics().str();
}
// Generate the GLSL code.
auto impl = std::make_unique<ASTPrinter>(&sanitized_result.program, options.version);
- impl->Generate();
- result.success = impl->Diagnostics().empty();
- result.error = impl->Diagnostics().str();
- result.glsl = impl->Result();
+ if (!impl->Generate()) {
+ return impl->Diagnostics().str();
+ }
+
+ Output output;
+ output.glsl = impl->Result();
// Collect the list of entry points in the sanitized program.
for (auto* func : sanitized_result.program.AST().Functions()) {
if (func->IsEntryPoint()) {
auto name = func->name->symbol.Name();
- result.entry_points.push_back({name, func->PipelineStage()});
+ output.entry_points.push_back({name, func->PipelineStage()});
}
}
- return result;
+ return output;
}
} // namespace tint::glsl::writer
diff --git a/src/tint/lang/glsl/writer/writer.h b/src/tint/lang/glsl/writer/writer.h
index eea9fa0..7ced20e 100644
--- a/src/tint/lang/glsl/writer/writer.h
+++ b/src/tint/lang/glsl/writer/writer.h
@@ -18,7 +18,8 @@
#include <string>
#include "src/tint/lang/glsl/writer/common/options.h"
-#include "src/tint/lang/glsl/writer/result.h"
+#include "src/tint/lang/glsl/writer/output.h"
+#include "src/tint/utils/result/result.h"
// Forward declarations
namespace tint {
@@ -28,13 +29,15 @@
namespace tint::glsl::writer {
/// Generate GLSL for a program, according to a set of configuration options.
-/// The result will contain the GLSL, as well as success status and diagnostic
+/// The result will contain the GLSL and supplementary information, or an error string.
/// information.
/// @param program the program to translate to GLSL
/// @param options the configuration options to use when generating GLSL
/// @param entry_point the entry point to generate GLSL for
-/// @returns the resulting GLSL and supplementary information
-Result Generate(const Program* program, const Options& options, const std::string& entry_point);
+/// @returns the resulting GLSL and supplementary information, or an error string
+Result<Output, std::string> Generate(const Program* program,
+ const Options& options,
+ const std::string& entry_point);
} // namespace tint::glsl::writer
diff --git a/src/tint/lang/glsl/writer/writer_bench.cc b/src/tint/lang/glsl/writer/writer_bench.cc
index 270e27f..a533f1a 100644
--- a/src/tint/lang/glsl/writer/writer_bench.cc
+++ b/src/tint/lang/glsl/writer/writer_bench.cc
@@ -38,8 +38,8 @@
for (auto _ : state) {
for (auto& ep : entry_points) {
auto res = Generate(&program, {}, ep);
- if (!res.error.empty()) {
- state.SkipWithError(res.error.c_str());
+ if (!res) {
+ state.SkipWithError(res.Failure().c_str());
}
}
}