jfb added inline comments.
================ 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: > 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! ================ Comment at: clang/lib/Frontend/CompilerInstance.cpp:2051-2053 + // FIXME this drops the error on the floor. This code is only used for + // typo correction and drops more than just this one source of errors + // (such as the directory creation failure above). ---------------- lhames wrote: > This will crash if writeIndex ever generates an error. For that reason I > would suggest writing this as: > > // FIXME: Can actually fail! <fixme goes here> > cantFail(GlobalModuleIndex::writeIndex(...)); > > It has the same effect, but without the control flow. > I think this one shouldn't crash, so I've made it consume the error instead (and it still needs to get fixed). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits