bugpoint: Stop threading errors through APIs that never fail

This simplifies ListReducer and most of its subclasses by removing the
std::string &Error that was threaded through all of them but almost
never used. If we end up needing error handling in more places here we
can reinstate it using llvm::Error instead of these unwieldy strings.

The 2 cases (out of 12) that actually can hit the error cases are a
little bit awkward now, but those will clean up as I refactor this API
further.

llvm-svn: 280690

llvm/tools/bugpoint/BugDriver.cpp

@@ -146,11 +146,11 @@ bool BugDriver::addSources(const std::vector<std::string> &Filenames) {
 /// run - The top level method that is invoked after all of the instance
 /// variables are set up from command line arguments.
 ///
-bool BugDriver::run(std::string &ErrMsg) {
+bool BugDriver::run() {
   if (run_find_bugs) {
     // Rearrange the passes and apply them to the program. Repeat this process
     // until the user kills the program or we find a bug.
-    return runManyPasses(PassesToRun, ErrMsg);
+    return runManyPasses(PassesToRun);
   }
 
   // If we're not running as a child, the first thing that we must do is
@@ -176,7 +176,7 @@ bool BugDriver::run(std::string &ErrMsg) {
   compileProgram(Program, &Error);
   if (!Error.empty()) {
     outs() << Error;
-    return debugCodeGeneratorCrash(ErrMsg);
+    return debugCodeGeneratorCrash();
   }
   outs() << '\n';
 
@@ -188,7 +188,7 @@ bool BugDriver::run(std::string &ErrMsg) {
   if (ReferenceOutputFile.empty()) {
     outs() << "Generating reference output from raw program: ";
     if (!createReferenceFile(Program)) {
-      return debugCodeGeneratorCrash(ErrMsg);
+      return debugCodeGeneratorCrash();
     }
     CreatedOutput = true;
   }
@@ -205,14 +205,14 @@ bool BugDriver::run(std::string &ErrMsg) {
   bool Diff = diffProgram(Program, "", "", false, &Error);
   if (!Error.empty()) {
     errs() << Error;
-    return debugCodeGeneratorCrash(ErrMsg);
+    return debugCodeGeneratorCrash();
   }
   if (!Diff) {
     outs() << "\n*** Output matches: Debugging miscompilation!\n";
     debugMiscompilation(&Error);
     if (!Error.empty()) {
       errs() << Error;
-      return debugCodeGeneratorCrash(ErrMsg);
+      return debugCodeGeneratorCrash();
     }
     return false;
   }
@@ -222,7 +222,7 @@ bool BugDriver::run(std::string &ErrMsg) {
   bool Failure = debugCodeGenerator(&Error);
   if (!Error.empty()) {
     errs() << Error;
-    return debugCodeGeneratorCrash(ErrMsg);
+    return debugCodeGeneratorCrash();
   }
   return Failure;
 }

llvm/tools/bugpoint/BugDriver.h

@@ -85,7 +85,7 @@ public:
   /// variables are set up from command line arguments. The \p as_child argument
   /// indicates whether the driver is to run in parent mode or child mode.
   ///
-  bool run(std::string &ErrMsg);
+  bool run();
 
   /// debugOptimizerCrash - This method is called when some optimizer pass
   /// crashes on input.  It attempts to prune down the testcase to something
@@ -96,7 +96,7 @@ public:
   /// debugCodeGeneratorCrash - This method is called when the code generator
   /// crashes on an input.  It attempts to reduce the input as much as possible
   /// while still causing the code generator to crash.
-  bool debugCodeGeneratorCrash(std::string &Error);
+  bool debugCodeGeneratorCrash();
 
   /// debugMiscompilation - This method is used when the passes selected are not
   /// crashing, but the generated output is semantically different from the
@@ -266,8 +266,7 @@ public:
   /// If the passes did not compile correctly, output the command required to
   /// recreate the failure. This returns true if a compiler error is found.
   ///
-  bool runManyPasses(const std::vector<std::string> &AllPasses,
+  bool runManyPasses(const std::vector<std::string> &AllPasses);
-                     std::string &ErrMsg);
 
   /// writeProgramToFile - This writes the current "Program" to the named
   /// bitcode file.  If an error occurs, true is returned.

llvm/tools/bugpoint/CrashDebugger.cpp

@@ -71,14 +71,13 @@ public:
   // passes.  If we return true, we update the current module of bugpoint.
   //
   TestResult doTest(std::vector<std::string> &Removed,
-                    std::vector<std::string> &Kept,
+                    std::vector<std::string> &Kept) override;
-                    std::string &Error) override;
 };
 }
 
 ReducePassList::TestResult
 ReducePassList::doTest(std::vector<std::string> &Prefix,
-                       std::vector<std::string> &Suffix, std::string &Error) {
+                       std::vector<std::string> &Suffix) {
   std::string PrefixOutput;
   Module *OrigProgram = nullptr;
   if (!Prefix.empty()) {
@@ -129,8 +128,7 @@ public:
       : BD(bd), TestFn(testFn) {}
 
   TestResult doTest(std::vector<GlobalVariable *> &Prefix,
-                    std::vector<GlobalVariable *> &Kept,
+                    std::vector<GlobalVariable *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestGlobalVariables(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestGlobalVariables(Prefix))
@@ -199,8 +197,7 @@ public:
       : BD(bd), TestFn(testFn) {}
 
   TestResult doTest(std::vector<Function *> &Prefix,
-                    std::vector<Function *> &Kept,
+                    std::vector<Function *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestFuncs(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestFuncs(Prefix))
@@ -373,8 +370,7 @@ public:
       : BD(BD), TestFn(testFn) {}
 
   TestResult doTest(std::vector<const BasicBlock *> &Prefix,
-                    std::vector<const BasicBlock *> &Kept,
+                    std::vector<const BasicBlock *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestBlocks(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestBlocks(Prefix))
@@ -495,8 +491,7 @@ public:
       : BD(bd), TestFn(testFn), Direction(Direction) {}
 
   TestResult doTest(std::vector<const BasicBlock *> &Prefix,
-                    std::vector<const BasicBlock *> &Kept,
+                    std::vector<const BasicBlock *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestBlocks(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestBlocks(Prefix))
@@ -603,8 +598,7 @@ public:
       : BD(bd), TestFn(testFn), TTI(bd.getProgram()->getDataLayout()) {}
 
   TestResult doTest(std::vector<const BasicBlock *> &Prefix,
-                    std::vector<const BasicBlock *> &Kept,
+                    std::vector<const BasicBlock *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestBlocks(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestBlocks(Prefix))
@@ -698,8 +692,7 @@ public:
       : BD(bd), TestFn(testFn) {}
 
   TestResult doTest(std::vector<const Instruction *> &Prefix,
-                    std::vector<const Instruction *> &Kept,
+                    std::vector<const Instruction *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestInsts(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestInsts(Prefix))
@@ -775,8 +768,7 @@ public:
       : BD(bd), TestFn(testFn) {}
 
   TestResult doTest(std::vector<std::string> &Prefix,
-                    std::vector<std::string> &Kept,
+                    std::vector<std::string> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestNamedMDs(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestNamedMDs(Prefix))
@@ -845,8 +837,7 @@ public:
       : BD(bd), TestFn(testFn) {}
 
   TestResult doTest(std::vector<const MDNode *> &Prefix,
-                    std::vector<const MDNode *> &Kept,
+                    std::vector<const MDNode *> &Kept) override {
-                    std::string &Error) override {
     if (!Kept.empty() && TestNamedMDOps(Kept))
       return KeepSuffix;
     if (!Prefix.empty() && TestNamedMDOps(Prefix))
@@ -909,8 +900,7 @@ bool ReduceCrashingNamedMDOps::TestNamedMDOps(
 
 static void ReduceGlobalInitializers(BugDriver &BD,
                                      bool (*TestFn)(const BugDriver &,
-                                                    Module *),
+                                                    Module *)) {
-                                     std::string &Error) {
   if (BD.getProgram()->global_begin() != BD.getProgram()->global_end()) {
     // Now try to reduce the number of global variable initializers in the
     // module to something small.
@@ -952,8 +942,7 @@ static void ReduceGlobalInitializers(BugDriver &BD,
                  << "variables in the testcase\n";
 
           unsigned OldSize = GVs.size();
-          ReduceCrashingGlobalVariables(BD, TestFn).reduceList(GVs, Error);
+          ReduceCrashingGlobalVariables(BD, TestFn).reduceList(GVs);
-          assert(!Error.empty());
 
           if (GVs.size() < OldSize)
             BD.EmitProgressBitcode(BD.getProgram(), "reduced-global-variables");
@@ -964,8 +953,7 @@ static void ReduceGlobalInitializers(BugDriver &BD,
 }
 
 static void ReduceInsts(BugDriver &BD,
-                        bool (*TestFn)(const BugDriver &, Module *),
+                        bool (*TestFn)(const BugDriver &, Module *)) {
-                        std::string &Error) {
   // Attempt to delete instructions using bisection. This should help out nasty
   // cases with large basic blocks where the problem is at one end.
   if (!BugpointIsInterrupted) {
@@ -976,7 +964,7 @@ static void ReduceInsts(BugDriver &BD,
           if (!isa<TerminatorInst>(&I))
             Insts.push_back(&I);
 
-    ReduceCrashingInstructions(BD, TestFn).reduceList(Insts, Error);
+    ReduceCrashingInstructions(BD, TestFn).reduceList(Insts);
   }
 
   unsigned Simplification = 2;
@@ -1046,12 +1034,11 @@ static void ReduceInsts(BugDriver &BD,
 /// on a program, try to destructively reduce the program while still keeping
 /// the predicate true.
 static bool DebugACrash(BugDriver &BD,
-                        bool (*TestFn)(const BugDriver &, Module *),
+                        bool (*TestFn)(const BugDriver &, Module *)) {
-                        std::string &Error) {
   // See if we can get away with nuking some of the global variable initializers
   // in the program...
   if (!NoGlobalRM)
-    ReduceGlobalInitializers(BD, TestFn, Error);
+    ReduceGlobalInitializers(BD, TestFn);
 
   // Now try to reduce the number of functions in the module to something small.
   std::vector<Function *> Functions;
@@ -1064,7 +1051,7 @@ static bool DebugACrash(BugDriver &BD,
               "in the testcase\n";
 
     unsigned OldSize = Functions.size();
-    ReduceCrashingFunctions(BD, TestFn).reduceList(Functions, Error);
+    ReduceCrashingFunctions(BD, TestFn).reduceList(Functions);
 
     if (Functions.size() < OldSize)
       BD.EmitProgressBitcode(BD.getProgram(), "reduced-function");
@@ -1078,8 +1065,8 @@ static bool DebugACrash(BugDriver &BD,
       for (BasicBlock &BB : F)
         Blocks.push_back(&BB);
     unsigned OldSize = Blocks.size();
-    ReduceCrashingConditionals(BD, TestFn, true).reduceList(Blocks, Error);
+    ReduceCrashingConditionals(BD, TestFn, true).reduceList(Blocks);
-    ReduceCrashingConditionals(BD, TestFn, false).reduceList(Blocks, Error);
+    ReduceCrashingConditionals(BD, TestFn, false).reduceList(Blocks);
     if (Blocks.size() < OldSize)
       BD.EmitProgressBitcode(BD.getProgram(), "reduced-conditionals");
   }
@@ -1095,7 +1082,7 @@ static bool DebugACrash(BugDriver &BD,
       for (BasicBlock &BB : F)
         Blocks.push_back(&BB);
     unsigned OldSize = Blocks.size();
-    ReduceCrashingBlocks(BD, TestFn).reduceList(Blocks, Error);
+    ReduceCrashingBlocks(BD, TestFn).reduceList(Blocks);
     if (Blocks.size() < OldSize)
       BD.EmitProgressBitcode(BD.getProgram(), "reduced-blocks");
   }
@@ -1106,7 +1093,7 @@ static bool DebugACrash(BugDriver &BD,
       for (BasicBlock &BB : F)
         Blocks.push_back(&BB);
     unsigned OldSize = Blocks.size();
-    ReduceSimplifyCFG(BD, TestFn).reduceList(Blocks, Error);
+    ReduceSimplifyCFG(BD, TestFn).reduceList(Blocks);
     if (Blocks.size() < OldSize)
       BD.EmitProgressBitcode(BD.getProgram(), "reduced-simplifycfg");
   }
@@ -1114,7 +1101,7 @@ static bool DebugACrash(BugDriver &BD,
   // Attempt to delete instructions using bisection. This should help out nasty
   // cases with large basic blocks where the problem is at one end.
   if (!BugpointIsInterrupted)
-    ReduceInsts(BD, TestFn, Error);
+    ReduceInsts(BD, TestFn);
 
   if (!NoNamedMDRM) {
     if (!BugpointIsInterrupted) {
@@ -1124,7 +1111,7 @@ static bool DebugACrash(BugDriver &BD,
       std::vector<std::string> NamedMDNames;
       for (auto &NamedMD : BD.getProgram()->named_metadata())
         NamedMDNames.push_back(NamedMD.getName().str());
-      ReduceCrashingNamedMD(BD, TestFn).reduceList(NamedMDNames, Error);
+      ReduceCrashingNamedMD(BD, TestFn).reduceList(NamedMDNames);
     }
 
     if (!BugpointIsInterrupted) {
@@ -1134,7 +1121,7 @@ static bool DebugACrash(BugDriver &BD,
       for (auto &NamedMD : BD.getProgram()->named_metadata())
         for (auto op : NamedMD.operands())
           NamedMDOps.push_back(op);
-      ReduceCrashingNamedMDOps(BD, TestFn).reduceList(NamedMDOps, Error);
+      ReduceCrashingNamedMDOps(BD, TestFn).reduceList(NamedMDOps);
     }
     BD.EmitProgressBitcode(BD.getProgram(), "reduced-named-md");
   }
@@ -1169,11 +1156,9 @@ static bool TestForOptimizerCrash(const BugDriver &BD, Module *M) {
 bool BugDriver::debugOptimizerCrash(const std::string &ID) {
   outs() << "\n*** Debugging optimizer crash!\n";
 
-  std::string Error;
   // Reduce the list of passes which causes the optimizer to crash...
   if (!BugpointIsInterrupted && !DontReducePassList)
-    ReducePassList(*this).reduceList(PassesToRun, Error);
+    ReducePassList(*this).reduceList(PassesToRun);
-  assert(Error.empty());
 
   outs() << "\n*** Found crashing pass"
          << (PassesToRun.size() == 1 ? ": " : "es: ")
@@ -1181,8 +1166,7 @@ bool BugDriver::debugOptimizerCrash(const std::string &ID) {
 
   EmitProgressBitcode(Program, ID);
 
-  bool Success = DebugACrash(*this, TestForOptimizerCrash, Error);
+  bool Success = DebugACrash(*this, TestForOptimizerCrash);
-  assert(Error.empty());
   return Success;
 }
 
@@ -1203,8 +1187,8 @@ static bool TestForCodeGenCrash(const BugDriver &BD, Module *M) {
 /// debugCodeGeneratorCrash - This method is called when the code generator
 /// crashes on an input.  It attempts to reduce the input as much as possible
 /// while still causing the code generator to crash.
-bool BugDriver::debugCodeGeneratorCrash(std::string &Error) {
+bool BugDriver::debugCodeGeneratorCrash() {
   errs() << "*** Debugging code generator crash!\n";
 
-  return DebugACrash(*this, TestForCodeGenCrash, Error);
+  return DebugACrash(*this, TestForCodeGenCrash);
 }

llvm/tools/bugpoint/FindBugs.cpp

@@ -30,8 +30,7 @@ using namespace llvm;
 /// If the passes did not compile correctly, output the command required to
 /// recreate the failure. This returns true if a compiler error is found.
 ///
-bool BugDriver::runManyPasses(const std::vector<std::string> &AllPasses,
+bool BugDriver::runManyPasses(const std::vector<std::string> &AllPasses) {
-                              std::string &ErrMsg) {
   setPassesToRun(AllPasses);
   outs() << "Starting bug finding procedure...\n\n";
 
@@ -82,7 +81,7 @@ bool BugDriver::runManyPasses(const std::vector<std::string> &AllPasses,
     if (!Error.empty()) {
       outs() << "\n*** compileProgram threw an exception: ";
       outs() << Error;
-      return debugCodeGeneratorCrash(ErrMsg);
+      return debugCodeGeneratorCrash();
     }
     outs() << '\n';
 
@@ -100,7 +99,7 @@ bool BugDriver::runManyPasses(const std::vector<std::string> &AllPasses,
     }
     if (!Error.empty()) {
       errs() << Error;
-      debugCodeGeneratorCrash(ErrMsg);
+      debugCodeGeneratorCrash();
       return true;
     }
     outs() << "\n*** diff'd output matches!\n";

llvm/tools/bugpoint/ListReducer.h

@@ -15,7 +15,6 @@
 #ifndef LLVM_TOOLS_BUGPOINT_LISTREDUCER_H
 #define LLVM_TOOLS_BUGPOINT_LISTREDUCER_H
 
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cstdlib>
@@ -39,16 +38,16 @@ template <typename ElTy> struct ListReducer {
   /// test desired.  The testcase is only required to test to see if the Kept
   /// list still satisfies the property, but if it is going to check the prefix
   /// anyway, it can.
-  virtual TestResult doTest(std::vector<ElTy> &Prefix, std::vector<ElTy> &Kept,
+  virtual TestResult doTest(std::vector<ElTy> &Prefix,
-                            std::string &Error) = 0;
+                            std::vector<ElTy> &Kept) = 0;
 
   /// This function attempts to reduce the length of the specified list while
   /// still maintaining the "test" property.  This is the core of the "work"
   /// that bugpoint does.
-  bool reduceList(std::vector<ElTy> &TheList, std::string &Error) {
+  bool reduceList(std::vector<ElTy> &TheList) {
     std::vector<ElTy> empty;
     std::srand(0x6e5ea738); // Seed the random number generator
-    switch (doTest(TheList, empty, Error)) {
+    switch (doTest(TheList, empty)) {
     case KeepPrefix:
       if (TheList.size() == 1) // we are done, it's the base case and it fails
         return true;
@@ -64,7 +63,6 @@ template <typename ElTy> struct ListReducer {
       return false; // there is no failure with the full set of passes/funcs!
 
     case InternalError:
-      assert(!Error.empty());
       return true;
     }
 
@@ -97,7 +95,7 @@ template <typename ElTy> struct ListReducer {
         std::random_shuffle(ShuffledList.begin(), ShuffledList.end());
         errs() << "\n\n*** Testing shuffled set...\n\n";
         // Check that random shuffle doesn't loose the bug
-        if (doTest(ShuffledList, empty, Error) == KeepPrefix) {
+        if (doTest(ShuffledList, empty) == KeepPrefix) {
           // If the bug is still here, use the shuffled list.
           TheList.swap(ShuffledList);
           MidTop = TheList.size();
@@ -116,7 +114,7 @@ template <typename ElTy> struct ListReducer {
       std::vector<ElTy> Prefix(TheList.begin(), TheList.begin() + Mid);
       std::vector<ElTy> Suffix(TheList.begin() + Mid, TheList.end());
 
-      switch (doTest(Prefix, Suffix, Error)) {
+      switch (doTest(Prefix, Suffix)) {
       case KeepSuffix:
         // The property still holds.  We can just drop the prefix elements, and
         // shorten the list to the "kept" elements.
@@ -141,9 +139,8 @@ template <typename ElTy> struct ListReducer {
         NumOfIterationsWithoutProgress++;
         break;
       case InternalError:
-        return true; // Error was set by doTest.
+        return true;
       }
-      assert(Error.empty() && "doTest did not return InternalError for error");
     }
 
     // Probability of backjumping from the trimming loop back to the binary
@@ -179,14 +176,18 @@ template <typename ElTy> struct ListReducer {
           std::vector<ElTy> TestList(TheList);
           TestList.erase(TestList.begin() + i);
 
-          if (doTest(EmptyList, TestList, Error) == KeepSuffix) {
+          switch (doTest(EmptyList, TestList)) {
+          case KeepSuffix:
             // We can trim down the list!
             TheList.swap(TestList);
             --i; // Don't skip an element of the list
             Changed = true;
-          }
+            break;
-          if (!Error.empty())
+          case InternalError:
             return true;
+          default:
+            break;
+          }
         }
         if (TrimIterations >= MaxTrimIterationsWithoutBackJump)
           break;

llvm/tools/bugpoint/Miscompilation.cpp

@@ -46,13 +46,14 @@ static llvm::cl::opt<bool> DisableBlockExtraction(
 
 class ReduceMiscompilingPasses : public ListReducer<std::string> {
   BugDriver &BD;
+  std::string &Error;
 
 public:
-  ReduceMiscompilingPasses(BugDriver &bd) : BD(bd) {}
+  ReduceMiscompilingPasses(BugDriver &bd, std::string &Error)
+      : BD(bd), Error(Error) {}
 
   TestResult doTest(std::vector<std::string> &Prefix,
-                    std::vector<std::string> &Suffix,
+                    std::vector<std::string> &Suffix) override;
-                    std::string &Error) override;
 };
 } // end anonymous namespace
 
@@ -61,8 +62,7 @@ public:
 ///
 ReduceMiscompilingPasses::TestResult
 ReduceMiscompilingPasses::doTest(std::vector<std::string> &Prefix,
-                                 std::vector<std::string> &Suffix,
+                                 std::vector<std::string> &Suffix) {
-                                 std::string &Error) {
   // First, run the program with just the Suffix passes.  If it is still broken
   // with JUST the kept passes, discard the prefix passes.
   outs() << "Checking to see if '" << getPassesString(Suffix)
@@ -181,25 +181,26 @@ class ReduceMiscompilingFunctions : public ListReducer<Function *> {
   BugDriver &BD;
   bool (*TestFn)(BugDriver &, std::unique_ptr<Module>, std::unique_ptr<Module>,
                  std::string &);
+  std::string &Error;
 
 public:
   ReduceMiscompilingFunctions(BugDriver &bd,
                               bool (*F)(BugDriver &, std::unique_ptr<Module>,
-                                        std::unique_ptr<Module>, std::string &))
+                                        std::unique_ptr<Module>, std::string &),
-      : BD(bd), TestFn(F) {}
+                              std::string &Error)
+      : BD(bd), TestFn(F), Error(Error) {}
 
   TestResult doTest(std::vector<Function *> &Prefix,
-                    std::vector<Function *> &Suffix,
+                    std::vector<Function *> &Suffix) override {
-                    std::string &Error) override {
     if (!Suffix.empty()) {
-      bool Ret = TestFuncs(Suffix, Error);
+      bool Ret = TestFuncs(Suffix);
       if (!Error.empty())
         return InternalError;
       if (Ret)
         return KeepSuffix;
     }
     if (!Prefix.empty()) {
-      bool Ret = TestFuncs(Prefix, Error);
+      bool Ret = TestFuncs(Prefix);
       if (!Error.empty())
         return InternalError;
       if (Ret)
@@ -208,7 +209,7 @@ public:
     return NoFailure;
   }
 
-  bool TestFuncs(const std::vector<Function *> &Prefix, std::string &Error);
+  bool TestFuncs(const std::vector<Function *> &Prefix);
 };
 } // end anonymous namespace
 
@@ -238,7 +239,7 @@ static std::unique_ptr<Module> testMergedProgram(const BugDriver &BD,
 /// accordingly. Each group of functions becomes a separate Module.
 ///
 bool ReduceMiscompilingFunctions::TestFuncs(
-    const std::vector<Function *> &Funcs, std::string &Error) {
+    const std::vector<Function *> &Funcs) {
   // Test to see if the function is misoptimized if we ONLY run it on the
   // functions listed in Funcs.
   outs() << "Checking to see if the program is misoptimized when "
@@ -444,26 +445,27 @@ class ReduceMiscompiledBlocks : public ListReducer<BasicBlock *> {
   bool (*TestFn)(BugDriver &, std::unique_ptr<Module>, std::unique_ptr<Module>,
                  std::string &);
   std::vector<Function *> FunctionsBeingTested;
+  std::string &Error;
 
 public:
   ReduceMiscompiledBlocks(BugDriver &bd,
                           bool (*F)(BugDriver &, std::unique_ptr<Module>,
                                     std::unique_ptr<Module>, std::string &),
-                          const std::vector<Function *> &Fns)
+                          const std::vector<Function *> &Fns,
-      : BD(bd), TestFn(F), FunctionsBeingTested(Fns) {}
+                          std::string &Error)
+      : BD(bd), TestFn(F), FunctionsBeingTested(Fns), Error(Error) {}
 
   TestResult doTest(std::vector<BasicBlock *> &Prefix,
-                    std::vector<BasicBlock *> &Suffix,
+                    std::vector<BasicBlock *> &Suffix) override {
-                    std::string &Error) override {
     if (!Suffix.empty()) {
-      bool Ret = TestFuncs(Suffix, Error);
+      bool Ret = TestFuncs(Suffix);
       if (!Error.empty())
         return InternalError;
       if (Ret)
         return KeepSuffix;
     }
     if (!Prefix.empty()) {
-      bool Ret = TestFuncs(Prefix, Error);
+      bool Ret = TestFuncs(Prefix);
       if (!Error.empty())
         return InternalError;
       if (Ret)
@@ -472,15 +474,14 @@ public:
     return NoFailure;
   }
 
-  bool TestFuncs(const std::vector<BasicBlock *> &BBs, std::string &Error);
+  bool TestFuncs(const std::vector<BasicBlock *> &BBs);
 };
 } // end anonymous namespace
 
 /// TestFuncs - Extract all blocks for the miscompiled functions except for the
 /// specified blocks.  If the problem still exists, return true.
 ///
-bool ReduceMiscompiledBlocks::TestFuncs(const std::vector<BasicBlock *> &BBs,
+bool ReduceMiscompiledBlocks::TestFuncs(const std::vector<BasicBlock *> &BBs) {
-                                        std::string &Error) {
   // Test to see if the function is misoptimized if we ONLY run it on the
   // functions listed in Funcs.
   outs() << "Checking to see if the program is misoptimized when all ";
@@ -550,15 +551,15 @@ static bool ExtractBlocks(BugDriver &BD,
   unsigned OldSize = Blocks.size();
 
   // Check to see if all blocks are extractible first.
-  bool Ret = ReduceMiscompiledBlocks(BD, TestFn, MiscompiledFunctions)
+  bool Ret = ReduceMiscompiledBlocks(BD, TestFn, MiscompiledFunctions, Error)
-                 .TestFuncs(std::vector<BasicBlock *>(), Error);
+                 .TestFuncs(std::vector<BasicBlock *>());
   if (!Error.empty())
     return false;
   if (Ret) {
     Blocks.clear();
   } else {
-    ReduceMiscompiledBlocks(BD, TestFn, MiscompiledFunctions)
+    ReduceMiscompiledBlocks(BD, TestFn, MiscompiledFunctions, Error)
-        .reduceList(Blocks, Error);
+        .reduceList(Blocks);
     if (!Error.empty())
       return false;
     if (Blocks.size() == OldSize)
@@ -628,8 +629,8 @@ DebugAMiscompilation(BugDriver &BD,
 
   // Do the reduction...
   if (!BugpointIsInterrupted)
-    ReduceMiscompilingFunctions(BD, TestFn)
+    ReduceMiscompilingFunctions(BD, TestFn, Error)
-        .reduceList(MiscompiledFunctions, Error);
+        .reduceList(MiscompiledFunctions);
   if (!Error.empty()) {
     errs() << "\n***Cannot reduce functions: ";
     return MiscompiledFunctions;
@@ -654,8 +655,8 @@ DebugAMiscompilation(BugDriver &BD,
 
       // Do the reduction...
       if (!BugpointIsInterrupted)
-        ReduceMiscompilingFunctions(BD, TestFn)
+        ReduceMiscompilingFunctions(BD, TestFn, Error)
-            .reduceList(MiscompiledFunctions, Error);
+            .reduceList(MiscompiledFunctions);
       if (!Error.empty())
         return MiscompiledFunctions;
 
@@ -677,8 +678,8 @@ DebugAMiscompilation(BugDriver &BD,
       DisambiguateGlobalSymbols(BD.getProgram());
 
       // Do the reduction...
-      ReduceMiscompilingFunctions(BD, TestFn)
+      ReduceMiscompilingFunctions(BD, TestFn, Error)
-          .reduceList(MiscompiledFunctions, Error);
+          .reduceList(MiscompiledFunctions);
       if (!Error.empty())
         return MiscompiledFunctions;
 
@@ -732,7 +733,7 @@ static bool TestOptimizer(BugDriver &BD, std::unique_ptr<Module> Test,
 void BugDriver::debugMiscompilation(std::string *Error) {
   // Make sure something was miscompiled...
   if (!BugpointIsInterrupted)
-    if (!ReduceMiscompilingPasses(*this).reduceList(PassesToRun, *Error)) {
+    if (!ReduceMiscompilingPasses(*this, *Error).reduceList(PassesToRun)) {
       if (Error->empty())
         errs() << "*** Optimized program matches reference output!  No problem"
                << " detected...\nbugpoint can't help you with your problem!\n";

llvm/tools/bugpoint/bugpoint.cpp

@@ -197,11 +197,5 @@ int main(int argc, char **argv) {
   sys::Process::PreventCoreFiles();
 #endif
 
-  std::string Error;
+  return D.run();
-  bool Failure = D.run(Error);
-  if (!Error.empty()) {
-    errs() << Error;
-    return 1;
-  }
-  return Failure;
 }