Change interface of Pass::Process() to return possible failures.

5 files changed, 50 insertions, 43 deletions

diff --git a/test/opt/pass_fixture.h b/test/opt/pass_fixture.h
index 9cc831cf..9ce5fc99 100644
--- a/test/opt/pass_fixture.h
+++ b/test/opt/pass_fixture.h
@@ -48,17 +48,17 @@ class PassTest : public TestT {
   // Runs the given |pass| on the binary assembled from the |assembly|, and
   // disassebles the optimized binary. Returns a tuple of disassembly string
   // and the boolean value returned from pass Process() function.
-  std::tuple<std::string, bool> OptimizeAndDisassemble(
+  std::tuple<std::string, opt::Pass::Status> OptimizeAndDisassemble(
       opt::Pass* pass, const std::string& original, bool skip_nop) {
     std::unique_ptr<ir::Module> module =
         BuildModule(SPV_ENV_UNIVERSAL_1_1, consumer_, original);
     EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n"
                                << original << std::endl;
     if (!module) {
-      return std::make_tuple(std::string(), false);
+      return std::make_tuple(std::string(), opt::Pass::Status::Failure);
     }
 
-    const bool modified = pass->Process(module.get());
+    const auto status = pass->Process(module.get());
 
     std::vector<uint32_t> binary;
     module->ToBinary(&binary, skip_nop);
@@ -66,14 +66,14 @@ class PassTest : public TestT {
     EXPECT_TRUE(tools_.Disassemble(binary, &optimized))
         << "Disassembling failed for shader:\n"
         << original << std::endl;
-    return std::make_tuple(optimized, modified);
+    return std::make_tuple(optimized, status);
   }
 
   // Runs a single pass of class |PassT| on the binary assembled from the
   // |assembly|, disassembles the optimized binary. Returns a tuple of
   // disassembly string and the boolean value from the pass Process() function.
   template <typename PassT, typename... Args>
-  std::tuple<std::string, bool> SinglePassRunAndDisassemble(
+  std::tuple<std::string, opt::Pass::Status> SinglePassRunAndDisassemble(
       const std::string& assembly, bool skip_nop, Args&&... args) {
     auto pass = MakeUnique<PassT>(consumer_, std::forward<Args>(args)...);
     return OptimizeAndDisassemble(pass.get(), assembly, skip_nop);
@@ -88,11 +88,13 @@ class PassTest : public TestT {
                              const std::string& expected, bool skip_nop,
                              Args&&... args) {
     std::string optimized;
-    bool modified = false;
-    std::tie(optimized, modified) = SinglePassRunAndDisassemble<PassT>(
+    auto status = opt::Pass::Status::SuccessWithoutChange;
+    std::tie(optimized, status) = SinglePassRunAndDisassemble<PassT>(
         original, skip_nop, std::forward<Args>(args)...);
     // Check whether the pass returns the correct modification indication.
-    EXPECT_EQ(original != expected, modified);
+    EXPECT_NE(opt::Pass::Status::Failure, status);
+    EXPECT_EQ(original == expected,
+              status == opt::Pass::Status::SuccessWithoutChange);
     EXPECT_EQ(expected, optimized);
   }
 
diff --git a/test/opt/test_fold_spec_const_op_composite.cpp b/test/opt/test_fold_spec_const_op_composite.cpp
index 051c9131..b84bf268 100644
--- a/test/opt/test_fold_spec_const_op_composite.cpp
+++ b/test/opt/test_fold_spec_const_op_composite.cpp
@@ -206,15 +206,16 @@ TEST_P(FoldSpecConstantOpAndCompositePassTest, ParamTestCase) {
 
   // Run the optimization and get the result code in disassembly.
   std::string optimized;
-  bool modified = false;
-  std::tie(optimized, modified) =
+  auto status = opt::Pass::Status::SuccessWithoutChange;
+  std::tie(optimized, status) =
       SinglePassRunAndDisassemble<opt::FoldSpecConstantOpAndCompositePass>(
           original, /* skip_nop = */ true);
 
   // Check the optimized code, but ignore the OpName instructions.
+  EXPECT_NE(opt::Pass::Status::Failure, status);
   EXPECT_EQ(
-      StripOpNameInstructions(expected) != StripOpNameInstructions(original),
-      modified);
+      StripOpNameInstructions(expected) == StripOpNameInstructions(original),
+      status == opt::Pass::Status::SuccessWithoutChange);
   EXPECT_EQ(StripOpNameInstructions(expected),
             StripOpNameInstructions(optimized));
 }
diff --git a/test/opt/test_line_debug_info.cpp b/test/opt/test_line_debug_info.cpp
index 3e86ed0c..a4af4974 100644
--- a/test/opt/test_line_debug_info.cpp
+++ b/test/opt/test_line_debug_info.cpp
@@ -25,10 +25,15 @@ class NopifyPass : public opt::Pass {
   explicit NopifyPass(const MessageConsumer& c) : opt::Pass(c) {}
 
   const char* name() const override { return "NopifyPass"; }
-  bool Process(ir::Module* module) override {
-    module->ForEachInst([](ir::Instruction* inst) { inst->ToNop(); },
-                        /* run_on_debug_line_insts = */ false);
-    return true;
+  Status Process(ir::Module* module) override {
+    bool modified = false;
+    module->ForEachInst(
+        [&modified](ir::Instruction* inst) {
+          inst->ToNop();
+          modified = true;
+        },
+        /* run_on_debug_line_insts = */ false);
+    return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
   }
 };
 
diff --git a/test/opt/test_pass_manager.cpp b/test/opt/test_pass_manager.cpp
index b365ad25..71ca3aca 100644
--- a/test/opt/test_pass_manager.cpp
+++ b/test/opt/test_pass_manager.cpp
@@ -78,10 +78,9 @@ class AppendOpNopPass : public opt::Pass {
   explicit AppendOpNopPass(const MessageConsumer& c) : opt::Pass(c) {}
 
   const char* name() const override { return "AppendOpNop"; }
-  bool Process(ir::Module* module) override {
-    auto inst = MakeUnique<ir::Instruction>();
-    module->AddDebugInst(std::move(inst));
-    return true;
+  Status Process(ir::Module* module) override {
+    module->AddDebugInst(MakeUnique<ir::Instruction>());
+    return Status::SuccessWithChange;
   }
 };
 
@@ -93,12 +92,11 @@ class AppendMultipleOpNopPass : public opt::Pass {
       : opt::Pass(c), num_nop_(num_nop) {}
 
   const char* name() const override { return "AppendOpNop"; }
-  bool Process(ir::Module* module) override {
+  Status Process(ir::Module* module) override {
     for (uint32_t i = 0; i < num_nop_; i++) {
-      auto inst = MakeUnique<ir::Instruction>();
-      module->AddDebugInst(std::move(inst));
+      module->AddDebugInst(MakeUnique<ir::Instruction>());
     }
-    return true;
+    return Status::SuccessWithChange;
   }
 
  private:
@@ -111,10 +109,10 @@ class DuplicateInstPass : public opt::Pass {
   explicit DuplicateInstPass(const MessageConsumer& c) : opt::Pass(c) {}
 
   const char* name() const override { return "DuplicateInst"; }
-  bool Process(ir::Module* module) override {
+  Status Process(ir::Module* module) override {
     auto inst = MakeUnique<ir::Instruction>(*(--module->debug_end()));
     module->AddDebugInst(std::move(inst));
-    return true;
+    return Status::SuccessWithChange;
   }
 };
 
@@ -149,11 +147,11 @@ class AppendTypeVoidInstPass : public opt::Pass {
       : opt::Pass(c), result_id_(result_id) {}
 
   const char* name() const override { return "AppendTypeVoidInstPass"; }
-  bool Process(ir::Module* module) override {
+  Status Process(ir::Module* module) override {
     auto inst = MakeUnique<ir::Instruction>(SpvOpTypeVoid, 0, result_id_,
                                             std::vector<ir::Operand>{});
     module->AddType(std::move(inst));
-    return true;
+    return Status::SuccessWithChange;
   }
 
  private:
diff --git a/test/opt/test_unify_const.cpp b/test/opt/test_unify_const.cpp
index 3a323721..58132017 100644
--- a/test/opt/test_unify_const.cpp
+++ b/test/opt/test_unify_const.cpp
@@ -114,8 +114,8 @@ class UnifyConstantTest : public PassTest<T> {
 
     // optimized code
     std::string optimized_before_strip;
-    bool modified = false;
-    std::tie(optimized_before_strip, modified) =
+    auto status = opt::Pass::Status::SuccessWithoutChange;
+    std::tie(optimized_before_strip, status) =
         this->template SinglePassRunAndDisassemble<opt::UnifyConstantPass>(
             test_builder.GetCode(),
             /* skip_nop = */ true);
@@ -124,8 +124,10 @@ class UnifyConstantTest : public PassTest<T> {
     std::tie(optimized_without_opnames, optimized_opnames) =
         StripOpNameInstructionsToSet(optimized_before_strip);
 
-    // Flag "modified" should be returned correctly.
-    EXPECT_EQ(expected_without_opnames != original_without_opnames, modified);
+    // Flag "status" should be returned correctly.
+    EXPECT_NE(opt::Pass::Status::Failure, status);
+    EXPECT_EQ(expected_without_opnames == original_without_opnames,
+              status == opt::Pass::Status::SuccessWithoutChange);
     // Code except OpName instructions should be exactly the same.
     EXPECT_EQ(expected_without_opnames, optimized_without_opnames);
     // OpName instructions can be in different order, but the content must be
@@ -208,11 +210,11 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) {
       });
 
   expected_builder
-    .AppendAnnotations({
+      .AppendAnnotations({
           "OpDecorate %f_1 RelaxedPrecision",
           "OpDecorate %f_2_dup RelaxedPrecision",
-        })
-  .AppendTypesConstantsGlobals({
+      })
+      .AppendTypesConstantsGlobals({
           // clang-format off
           "%float = OpTypeFloat 32",
           "%_pf_float = OpTypePointer Function %float",
@@ -223,7 +225,7 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) {
           "%f_3 = OpConstant %float 3",
           // clang-format on
       })
-  .AppendInMain({
+      .AppendInMain({
           // clang-format off
           "%f_var = OpVariable %_pf_float Function",
           "OpStore %f_var %f_1_dup",
@@ -231,8 +233,8 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) {
           "OpStore %f_var %f_3",
           // clang-format on
       })
-  .AppendNames({
-      "OpName %f_3 \"f_3_dup\"",
+      .AppendNames({
+          "OpName %f_3 \"f_3_dup\"",
       });
 
   Check(expected_builder, test_builder);
@@ -300,8 +302,7 @@ TEST_F(UnifyFrontEndConstantSingleTest, UnifyWithDecorationOnTypes) {
           "OpStore %flat_d_var %flat_d_1",
       })
       .AppendNames({
-          "OpName %flat_1 \"flat_1_dup\"",
-          "OpName %flat_d_1 \"flat_d_1_dup\"",
+          "OpName %flat_1 \"flat_1_dup\"", "OpName %flat_d_1 \"flat_d_1_dup\"",
       });
 
   Check(expected_builder, test_builder);
@@ -348,7 +349,7 @@ TEST_P(UnifyFrontEndConstantParamTest, TestCase) {
 
 INSTANTIATE_TEST_CASE_P(Case, UnifyFrontEndConstantParamTest,
                         ::testing::ValuesIn(std::vector<UnifyConstantTestCase>({
-        // clang-format off
+                            // clang-format off
         // basic tests for scalar constants
         {
           // preserved constants
@@ -974,7 +975,7 @@ INSTANTIATE_TEST_CASE_P(Case, UnifyFrontEndConstantParamTest,
             "OpName %signed_22 \"signed_22_dup\"",
           },
         },
-        // clang-format on
-    })));
+                            // clang-format on
+                        })));
 
 }  // anonymous namespace