[PATCH] D63518: BitStream reader: propagate errors

Lang Hames via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 26 09:16:10 PDT 2019


lhames accepted this revision.
lhames added a comment.

> Indeed! That's a pretty terrifying thing... but I'm not signing up to address *that* particular issue :)

Yep. I don't think you need to address that in this patch.

LGTM!



================
Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:611-618
+    Expected<unsigned> MaybeCode = Stream.ReadCode();
+    if (!MaybeCode) {
+      // FIXME this drops the error on the floor.
+      consumeError(MaybeCode.takeError());
+    }
+    
+    // FIXME check that the enum is in range.
----------------
jfb wrote:
> jfb wrote:
> > lhames wrote:
> > > Bigcheese wrote:
> > > > Does `consumeError` return?  `MaybeCode.get()` will crash if `!MaybeCode`.
> > > Yes: consumeError returns. This should probably be:
> > > 
> > >   if (!MaybeCode)
> > >     return consumeError(MaybeCode.takeError()), Cursor::Badness;
> > > 
> > Good point, fixed.
> You tempt me so with this comma operator... but no, I must not!
;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63518





More information about the cfe-commits mailing list