[PATCH] D51836: [bugpoint] Revert r318459

[Don Hinton via Phabricator via llvm-commits]

hintonda added inline comments.


================
Comment at: tools/bugpoint/ExecutionDriver.cpp:439
   Expected<std::string> Output =
       executeProgram(Program, "", BitcodeFile, SharedObject, nullptr);
   if (Error E = Output.takeError())
----------------
Meinersbur wrote:
> hintonda wrote:
> > Meinersbur wrote:
> > > hintonda wrote:
> > > > Meinersbur wrote:
> > > > > [serious] Here is another call to `executeProgram`. `BitcodeFile` can be empty as well (e.g. `BugDriver.cpp:218`) which will fail if only `executeProgramSafely` handles creating the bitcode file.
> > > > Yes, you're correct.  Looks like just rolling back r318459 with a note that you can't use TempFile in this case.  Does that sound like the correct fix?
> > > Have you considered wrapping `executeProgram` inside a function that creates (and discards) the bitfile (instead of hoisting that code into `executeProgramSafely`)?
> > I could do that, but we should revert r318459 first -- that change introduced a bug.
> What purpose does the revert serve? We know how to fix the problem immediately, as shown here. 
> 
> If you want to revert r318459 to fix the bug, why did you even upload this differential?
r318459 introduced a bug 10 months ago by an author who no longer works on the project.  

If it had been noticed earlier, I'd still consult with the author before reverting.  In this case, I initially wanted to just fix it, but as you pointed out, there were multiple code paths that would have needed the same fix.  So, in retrospect, reverting the commit the caused the problem seems like a more reasonable course.

If I was wrong to submit a diff for this, I'll just revert the change.


Repository:
  rL LLVM

https://reviews.llvm.org/D51836