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

[David Blaikie via llvm-commits]

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.


> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161212/5bfeacba/attachment.html>