[llvm] r289017 - Unbreak buildbots where the debug info test was crashing due to unchecked error.

[Greg Clayton via llvm-commits]

> On Dec 12, 2016, at 2:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Mon, Dec 12, 2016 at 2:05 PM Greg Clayton <gclayton at apple.com> wrote:
> 
> > On Dec 12, 2016, at 1:59 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Wed, Dec 7, 2016 at 6:21 PM Greg Clayton via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> > Author: gclayton
> > Date: Wed Dec  7 20:11:03 2016
> > New Revision: 289017
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=289017&view=rev
> > Log:
> > Unbreak buildbots where the debug info test was crashing due to unchecked error.
> >
> >
> > Modified:
> >     llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
> >
> > Modified: llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp?rev=289017&r1=289016&r2=289017&view=diff
> > ==============================================================================
> > --- llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp (original)
> > +++ llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp Wed Dec  7 20:11:03 2016
> > @@ -51,14 +51,15 @@ Triple getHostTripleForAddrSize(uint8_t
> >  /// \returns true if there were errors, false otherwise.
> >  template <typename T>
> >  static bool HandleExpectedError(T &Expected) {
> > -  if (!Expected)
> > -    return false;
> >    std::string ErrorMsg;
> >    handleAllErrors(Expected.takeError(), [&](const llvm::ErrorInfoBase &EI) {
> >      ErrorMsg = EI.message();
> >    });
> > -  ::testing::AssertionFailure() << "error: " << ErrorMsg;
> > -  return true;
> > +  if (!ErrorMsg.empty()) {
> > +    ::testing::AssertionFailure() << "error: " << ErrorMsg;
> > +    return true;
> > +  }
> > +  return false;
> >
> > I think this could be written a bit simpler and without relying on the error string being non-empty, etc.
> >
> > Oh, huh - Lang, any ideas?
> >
> > It'd be nice if handleAllErrors could return a value (if all handlers returned the same/compatibible) - though I guess then it'd need a default value.
> >
> > Lang, would it be more idiomatic here to test the error for non-failure ,return false, otherwise then handleAllErrors and return true.
> >
> 
> As you can see I was testing the error and returning false, but that was crashing some buildbots with some extra API macro defined.
> 
> Got a link/quote of a buildbot failure - that sounds like a rather different problem from what this code would address, so I'm probably not picturing what sort of failure you're describing.

without the extra API define, the initial code works fine. With the extra API define, we get an error stating no one checked the error.

>  
> I have been looking for Lang around here but he is on vacation. As soon as he gets back I will check with him and make this work, but for now I didn't want buildbots to be broken.
> 
> Greg