[PATCH] D66976: Small update to Expected<StringRef>

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D66976#1660407 <https://reviews.llvm.org/D66976#1660407>, @grimar wrote:

> In D66976#1659909 <https://reviews.llvm.org/D66976#1659909>, @dblaikie wrote:
>
> > In D66976#1658737 <https://reviews.llvm.org/D66976#1658737>, @MaskRay wrote:
> >
> > > In D66976#1658669 <https://reviews.llvm.org/D66976#1658669>, @dblaikie wrote:
> > >
> > > > Any chance of a test case? This looks like it fixed a bug/difference in behavior.
> > >
> > >
> > > rL370669 <https://reviews.llvm.org/rL370669> added a test
> >
> >
> > That doesn't seem to test this patch, though - if I revert this patch no tests fail.
>
>
> Did you set `LLVM_ENABLE_ABI_BREAKING_CHECKS` to `1`?
>
> Before this patch `NameOrErr` was not consumed/ignored by `consumeError`.
>  If you revert it and run the test from rL370669 <https://reviews.llvm.org/rL370669>, you should get a
>  "Expected<T> must be checked before access or destruction" failture.


Must've screwed it up somehow - maybe I used lit to run the test directly without rebuilding llvm-nm.

In any case - got it now - sorry for the runaround!
(also, can be helpful in the future to note that a patch is fixing a buildbot failure/existing test suite failure - otherwise it's harder to tell if it's missing test coverage)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66976/new/

https://reviews.llvm.org/D66976