llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Paul Kirth (ilovepi) <details> <summary>Changes</summary> In BitcodeReader, we were using consumeError(), which drops the error and hides it from normal usage. To avoid that, we can just slightly tweak the API to return an Expected<T>, and propagate the error accordingly. --- Full diff: https://github.com/llvm/llvm-project/pull/168759.diff 2 Files Affected: - (modified) clang-tools-extra/clang-doc/BitcodeReader.cpp (+20-26) - (modified) clang-tools-extra/clang-doc/BitcodeReader.h (+1-1) ``````````diff diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 4efbbd34730cf..0af7aeb1d3500 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -847,9 +847,11 @@ llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) { while (true) { unsigned BlockOrCode = 0; - Cursor Res = skipUntilRecordOrBlock(BlockOrCode); + llvm::Expected<Cursor> C = skipUntilRecordOrBlock(BlockOrCode); + if (!C) + return C.takeError(); - switch (Res) { + switch (*C) { case Cursor::BadBlock: return llvm::createStringError(llvm::inconvertibleErrorCode(), "bad block found"); @@ -985,45 +987,39 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) { } } -ClangDocBitcodeReader::Cursor +llvm::Expected<ClangDocBitcodeReader::Cursor> ClangDocBitcodeReader::skipUntilRecordOrBlock(unsigned &BlockOrRecordID) { llvm::TimeTraceScope("Reducing infos", "skipUntilRecordOrBlock"); BlockOrRecordID = 0; while (!Stream.AtEndOfStream()) { - Expected<unsigned> MaybeCode = Stream.ReadCode(); - if (!MaybeCode) { - // FIXME this drops the error on the floor. - consumeError(MaybeCode.takeError()); - return Cursor::BadBlock; - } + Expected<unsigned> Code = Stream.ReadCode(); + if (!Code) + return Code.takeError(); - unsigned Code = MaybeCode.get(); - if (Code >= static_cast<unsigned>(llvm::bitc::FIRST_APPLICATION_ABBREV)) { + if (*Code >= static_cast<unsigned>(llvm::bitc::FIRST_APPLICATION_ABBREV)) { BlockOrRecordID = Code; return Cursor::Record; } - switch (static_cast<llvm::bitc::FixedAbbrevIDs>(Code)) { + switch (static_cast<llvm::bitc::FixedAbbrevIDs>(*Code)) { case llvm::bitc::ENTER_SUBBLOCK: if (Expected<unsigned> MaybeID = Stream.ReadSubBlockID()) BlockOrRecordID = MaybeID.get(); - else { - // FIXME this drops the error on the floor. - consumeError(MaybeID.takeError()); - } + else + return MaybeID.takeError(); return Cursor::BlockBegin; case llvm::bitc::END_BLOCK: if (Stream.ReadBlockEnd()) - return Cursor::BadBlock; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "error at end of block"); return Cursor::BlockEnd; case llvm::bitc::DEFINE_ABBREV: - if (llvm::Error Err = Stream.ReadAbbrevRecord()) { - // FIXME this drops the error on the floor. - consumeError(std::move(Err)); - } + if (llvm::Error Err = Stream.ReadAbbrevRecord()) + return std::move(Err); continue; case llvm::bitc::UNABBREV_RECORD: - return Cursor::BadBlock; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "found unabbreviated record"); case llvm::bitc::FIRST_APPLICATION_ABBREV: llvm_unreachable("Unexpected abbrev id."); } @@ -1149,10 +1145,8 @@ ClangDocBitcodeReader::readBitcode() { return std::move(Err); continue; default: - if (llvm::Error Err = Stream.SkipBlock()) { - // FIXME this drops the error on the floor. - consumeError(std::move(Err)); - } + if (llvm::Error Err = Stream.SkipBlock()) + return std::move(Err); continue; } } diff --git a/clang-tools-extra/clang-doc/BitcodeReader.h b/clang-tools-extra/clang-doc/BitcodeReader.h index 4947721f0a06d..0f7f4f256f15b 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.h +++ b/clang-tools-extra/clang-doc/BitcodeReader.h @@ -59,7 +59,7 @@ class ClangDocBitcodeReader { // Helper function to step through blocks to find and dispatch the next record // or block to be read. - Cursor skipUntilRecordOrBlock(unsigned &BlockOrRecordID); + llvm::Expected<Cursor> skipUntilRecordOrBlock(unsigned &BlockOrRecordID); // Helper function to set up the appropriate type of Info. llvm::Expected<std::unique_ptr<Info>> readBlockToInfo(unsigned ID); `````````` </details> https://github.com/llvm/llvm-project/pull/168759 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
