Index: clang/include/clang/Serialization/ASTReader.h =================================================================== --- clang/include/clang/Serialization/ASTReader.h +++ clang/include/clang/Serialization/ASTReader.h @@ -1320,18 +1320,18 @@ ASTReaderListener *Listener, bool ValidateDiagnosticOptions); - ASTReadResult ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities); - ASTReadResult ReadExtensionBlock(ModuleFile &F); + llvm::Error ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities); + llvm::Error ReadExtensionBlock(ModuleFile &F); void ReadModuleOffsetMap(ModuleFile &F) const; - bool ParseLineTable(ModuleFile &F, const RecordData &Record); - bool ReadSourceManagerBlock(ModuleFile &F); + void ParseLineTable(ModuleFile &F, const RecordData &Record); + llvm::Error ReadSourceManagerBlock(ModuleFile &F); llvm::BitstreamCursor &SLocCursorForID(int ID); SourceLocation getImportLocation(ModuleFile *F); ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const ModuleFile *ImportedBy, unsigned ClientLoadCapabilities); - ASTReadResult ReadSubmoduleBlock(ModuleFile &F, - unsigned ClientLoadCapabilities); + llvm::Error ReadSubmoduleBlock(ModuleFile &F, + unsigned ClientLoadCapabilities); static bool ParseLanguageOptions(const RecordData &Record, bool Complain, ASTReaderListener &Listener, bool AllowCompatibleDifferences); @@ -1904,8 +1904,9 @@ /// ReadBlockAbbrevs - Enter a subblock of the specified BlockID with the /// specified cursor. Read the abbreviations that are at the top of the block /// and then leave the cursor pointing into the block. - static bool ReadBlockAbbrevs(llvm::BitstreamCursor &Cursor, unsigned BlockID, - uint64_t *StartOfBlockOffset = nullptr); + static llvm::Error ReadBlockAbbrevs(llvm::BitstreamCursor &Cursor, + unsigned BlockID, + uint64_t *StartOfBlockOffset = nullptr); /// Finds all the visible declarations with a given name. /// The current implementation of this method just loads the entire Index: clang/lib/Serialization/ASTReader.cpp =================================================================== --- clang/lib/Serialization/ASTReader.cpp +++ clang/lib/Serialization/ASTReader.cpp @@ -815,6 +815,23 @@ // AST reader implementation //===----------------------------------------------------------------------===// +namespace { +/// An already-diagnosed error +class DiagnosedError : public llvm::ErrorInfo { +public: + static char ID; + + DiagnosedError() {} + + void log(llvm::raw_ostream &OS) const override {} + + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); + } +}; +char DiagnosedError::ID; +} // namespace + static uint64_t readULEB(const unsigned char *&P) { unsigned Length = 0; const char *Error = nullptr; @@ -1263,7 +1280,14 @@ } void ASTReader::Error(llvm::Error &&Err) const { - Error(toString(std::move(Err))); + SmallVector Errors; + handleAllErrors( + std::move(Err), [](const DiagnosedError &E) {}, + [&Errors](const llvm::ErrorInfoBase &EI) { + Errors.push_back(EI.message()); + }); + if (!Errors.empty()) + Error(llvm::join(Errors.begin(), Errors.end(), "\n")); } //===----------------------------------------------------------------------===// @@ -1271,9 +1295,7 @@ //===----------------------------------------------------------------------===// /// Read the line table in the source manager block. -/// \returns true if there was an error. -bool ASTReader::ParseLineTable(ModuleFile &F, - const RecordData &Record) { +void ASTReader::ParseLineTable(ModuleFile &F, const RecordData &Record) { unsigned Idx = 0; LineTableInfo &LineTable = SourceMgr.getLineTable(); @@ -1312,12 +1334,10 @@ } LineTable.AddEntry(FileID::get(FID), Entries); } - - return false; } /// Read a source manager block -bool ASTReader::ReadSourceManagerBlock(ModuleFile &F) { +llvm::Error ASTReader::ReadSourceManagerBlock(ModuleFile &F) { using namespace SrcMgr; BitstreamCursor &SLocEntryCursor = F.SLocEntryCursor; @@ -1329,36 +1349,29 @@ SLocEntryCursor = F.Stream; // The stream itself is going to skip over the source manager block. - if (llvm::Error Err = F.Stream.SkipBlock()) { - Error(std::move(Err)); - return true; - } + if (llvm::Error Err = F.Stream.SkipBlock()) + return Err; // Enter the source manager block. - if (llvm::Error Err = - SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) { - Error(std::move(Err)); - return true; - } + if (llvm::Error Err = SLocEntryCursor.EnterSubBlock(SOURCE_MANAGER_BLOCK_ID)) + return Err; F.SourceManagerBlockStartOffset = SLocEntryCursor.GetCurrentBitNo(); RecordData Record; while (true) { Expected MaybeE = SLocEntryCursor.advanceSkippingSubblocks(); - if (!MaybeE) { - Error(MaybeE.takeError()); - return true; - } + if (!MaybeE) + return MaybeE.takeError(); llvm::BitstreamEntry E = MaybeE.get(); switch (E.Kind) { case llvm::BitstreamEntry::SubBlock: // Handled for us already. case llvm::BitstreamEntry::Error: - Error("malformed block record in AST file"); - return true; + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::EndBlock: - return false; + return llvm::Error::success(); case llvm::BitstreamEntry::Record: // The interesting case. break; @@ -1369,10 +1382,8 @@ StringRef Blob; Expected MaybeRecord = SLocEntryCursor.readRecord(E.ID, Record, &Blob); - if (!MaybeRecord) { - Error(MaybeRecord.takeError()); - return true; - } + if (!MaybeRecord) + return MaybeRecord.takeError(); switch (MaybeRecord.get()) { default: // Default behavior: ignore. break; @@ -1381,7 +1392,7 @@ case SM_SLOC_BUFFER_ENTRY: case SM_SLOC_EXPANSION_ENTRY: // Once we hit one of the source location entries, we're done. - return false; + return llvm::Error::success(); } } } @@ -1632,13 +1643,11 @@ /// Enter a subblock of the specified BlockID with the specified cursor. Read /// the abbreviations that are at the top of the block and then leave the cursor /// pointing into the block. -bool ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor, unsigned BlockID, - uint64_t *StartOfBlockOffset) { - if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; - } +llvm::Error ASTReader::ReadBlockAbbrevs(BitstreamCursor &Cursor, + unsigned BlockID, + uint64_t *StartOfBlockOffset) { + if (llvm::Error Err = Cursor.EnterSubBlock(BlockID)) + return Err; if (StartOfBlockOffset) *StartOfBlockOffset = Cursor.GetCurrentBitNo(); @@ -1646,27 +1655,18 @@ while (true) { uint64_t Offset = Cursor.GetCurrentBitNo(); Expected MaybeCode = Cursor.ReadCode(); - if (!MaybeCode) { - // FIXME this drops errors on the floor. - consumeError(MaybeCode.takeError()); - return true; - } + if (!MaybeCode) + return MaybeCode.takeError(); unsigned Code = MaybeCode.get(); // We expect all abbrevs to be at the start of the block. if (Code != llvm::bitc::DEFINE_ABBREV) { - if (llvm::Error Err = Cursor.JumpToBit(Offset)) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; - } - return false; - } - if (llvm::Error Err = Cursor.ReadAbbrevRecord()) { - // FIXME this drops errors on the floor. - consumeError(std::move(Err)); - return true; + if (llvm::Error Err = Cursor.JumpToBit(Offset)) + return Err; + return llvm::Error::success(); } + if (llvm::Error Err = Cursor.ReadAbbrevRecord()) + return Err; } } @@ -2954,30 +2954,27 @@ } } -ASTReader::ASTReadResult -ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { +llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, + unsigned ClientLoadCapabilities) { BitstreamCursor &Stream = F.Stream; - if (llvm::Error Err = Stream.EnterSubBlock(AST_BLOCK_ID)) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.EnterSubBlock(AST_BLOCK_ID)) + return Err; F.ASTBlockStartOffset = Stream.GetCurrentBitNo(); // Read all of the records and blocks for the AST file. RecordData Record; while (true) { Expected MaybeEntry = Stream.advance(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::Error: - Error("error at end of module block in AST file"); - return Failure; + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "error at end of module block in AST file"); case llvm::BitstreamEntry::EndBlock: // Outside of C++, we do not store a lookup map for the translation unit. // Instead, mark it as needing a lookup map to be built if this module @@ -2990,7 +2987,7 @@ DC->setMustBuildLookupTable(); } - return Success; + return llvm::Error::success(); case llvm::BitstreamEntry::SubBlock: switch (Entry.ID) { case DECLTYPES_BLOCK_ID: @@ -2999,15 +2996,11 @@ // cursor to it, enter the block and read the abbrevs in that block. // With the main cursor, we just skip over it. F.DeclsCursor = Stream; - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } - if (ReadBlockAbbrevs(F.DeclsCursor, DECLTYPES_BLOCK_ID, - &F.DeclsBlockStartOffset)) { - Error("malformed block record in AST file"); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = ReadBlockAbbrevs( + F.DeclsCursor, DECLTYPES_BLOCK_ID, &F.DeclsBlockStartOffset)) + return Err; break; case PREPROCESSOR_BLOCK_ID: @@ -3015,14 +3008,11 @@ if (!PP.getExternalSource()) PP.setExternalSource(this); - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } - if (ReadBlockAbbrevs(F.MacroCursor, PREPROCESSOR_BLOCK_ID)) { - Error("malformed block record in AST file"); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = + ReadBlockAbbrevs(F.MacroCursor, PREPROCESSOR_BLOCK_ID)) + return Err; F.MacroStartOffset = F.MacroCursor.GetCurrentBitNo(); break; @@ -3030,14 +3020,11 @@ F.PreprocessorDetailCursor = Stream; if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } - if (ReadBlockAbbrevs(F.PreprocessorDetailCursor, - PREPROCESSOR_DETAIL_BLOCK_ID)) { - Error("malformed preprocessor detail record in AST file"); - return Failure; + return Err; } + if (llvm::Error Err = ReadBlockAbbrevs(F.PreprocessorDetailCursor, + PREPROCESSOR_DETAIL_BLOCK_ID)) + return Err; F.PreprocessorDetailStartOffset = F.PreprocessorDetailCursor.GetCurrentBitNo(); @@ -3048,36 +3035,29 @@ break; case SOURCE_MANAGER_BLOCK_ID: - if (ReadSourceManagerBlock(F)) - return Failure; + if (llvm::Error Err = ReadSourceManagerBlock(F)) + return Err; break; case SUBMODULE_BLOCK_ID: - if (ASTReadResult Result = - ReadSubmoduleBlock(F, ClientLoadCapabilities)) - return Result; + if (llvm::Error Err = ReadSubmoduleBlock(F, ClientLoadCapabilities)) + return Err; break; case COMMENTS_BLOCK_ID: { BitstreamCursor C = Stream; - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } - if (ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID)) { - Error("malformed comments block in AST file"); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; + if (llvm::Error Err = ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID)) + return Err; CommentsCursors.push_back(std::make_pair(C, &F)); break; } default: - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; break; } continue; @@ -3092,10 +3072,8 @@ StringRef Blob; Expected MaybeRecordType = Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeRecordType) { - Error(MaybeRecordType.takeError()); - return Failure; - } + if (!MaybeRecordType) + return MaybeRecordType.takeError(); ASTRecordTypes RecordType = (ASTRecordTypes)MaybeRecordType.get(); // If we're not loading an AST context, we don't care about most records. @@ -3126,10 +3104,10 @@ break; case TYPE_OFFSET: { - if (F.LocalNumTypes != 0) { - Error("duplicate TYPE_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumTypes != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate TYPE_OFFSET record in AST file"); F.TypeOffsets = reinterpret_cast(Blob.data()); F.LocalNumTypes = Record[0]; unsigned LocalBaseTypeIndex = Record[1]; @@ -3150,10 +3128,10 @@ } case DECL_OFFSET: { - if (F.LocalNumDecls != 0) { - Error("duplicate DECL_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumDecls != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate DECL_OFFSET record in AST file"); F.DeclOffsets = (const DeclOffset *)Blob.data(); F.LocalNumDecls = Record[0]; unsigned LocalBaseDeclID = Record[1]; @@ -3218,10 +3196,10 @@ break; case IDENTIFIER_OFFSET: { - if (F.LocalNumIdentifiers != 0) { - Error("duplicate IDENTIFIER_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumIdentifiers != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate IDENTIFIER_OFFSET record in AST file"); F.IdentifierOffsets = (const uint32_t *)Blob.data(); F.LocalNumIdentifiers = Record[0]; unsigned LocalBaseIdentifierID = Record[1]; @@ -3272,10 +3250,9 @@ break; } - if (SpecialTypes.size() != Record.size()) { - Error("invalid special-types record"); - return Failure; - } + if (SpecialTypes.size() != Record.size()) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid special-types record"); for (unsigned I = 0, N = Record.size(); I != N; ++I) { serialization::TypeID ID = getGlobalTypeID(F, Record[I]); @@ -3304,10 +3281,9 @@ break; case WEAK_UNDECLARED_IDENTIFIERS: - if (Record.size() % 4 != 0) { - Error("invalid weak identifiers record"); - return Failure; - } + if (Record.size() % 4 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid weak identifiers record"); // FIXME: Ignore weak undeclared identifiers from non-original PCH // files. This isn't the way to do it :) @@ -3414,10 +3390,9 @@ std::tie(F.SLocEntryBaseID, F.SLocEntryBaseOffset) = SourceMgr.AllocateLoadedSLocEntries(F.LocalNumSLocEntries, SLocSpaceSize); - if (!F.SLocEntryBaseID) { - Error("ran out of source locations"); - break; - } + if (!F.SLocEntryBaseID) + return llvm::createStringError(std::errc::invalid_argument, + "ran out of source locations"); // Make our entry in the range map. BaseID is negative and growing, so // we invert it. Because we invert it, though, we need the other end of // the range. @@ -3448,19 +3423,16 @@ break; case SOURCE_MANAGER_LINE_TABLE: - if (ParseLineTable(F, Record)) { - Error("malformed SOURCE_MANAGER_LINE_TABLE in AST file"); - return Failure; - } + ParseLineTable(F, Record); break; case SOURCE_LOCATION_PRELOADS: { // Need to transform from the local view (1-based IDs) to the global view, // which is based off F.SLocEntryBaseID. - if (!F.PreloadSLocEntries.empty()) { - Error("Multiple SOURCE_LOCATION_PRELOADS records in AST file"); - return Failure; - } + if (!F.PreloadSLocEntries.empty()) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Multiple SOURCE_LOCATION_PRELOADS records in AST file"); F.PreloadSLocEntries.swap(Record); break; @@ -3472,10 +3444,9 @@ break; case VTABLE_USES: - if (Record.size() % 3 != 0) { - Error("Invalid VTABLE_USES record"); - return Failure; - } + if (Record.size() % 3 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid VTABLE_USES record"); // Later tables overwrite earlier ones. // FIXME: Modules will have some trouble with this. This is clearly not @@ -3491,15 +3462,15 @@ break; case PENDING_IMPLICIT_INSTANTIATIONS: - if (PendingInstantiations.size() % 2 != 0) { - Error("Invalid existing PendingInstantiations"); - return Failure; - } + if (PendingInstantiations.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Invalid existing PendingInstantiations"); - if (Record.size() % 2 != 0) { - Error("Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Invalid PENDING_IMPLICIT_INSTANTIATIONS block"); for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { PendingInstantiations.push_back(getGlobalDeclID(F, Record[I++])); @@ -3509,10 +3480,9 @@ break; case SEMA_DECL_REFS: - if (Record.size() != 3) { - Error("Invalid SEMA_DECL_REFS block"); - return Failure; - } + if (Record.size() != 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid SEMA_DECL_REFS block"); for (unsigned I = 0, N = Record.size(); I != N; ++I) SemaDeclRefs.push_back(getGlobalDeclID(F, Record[I])); break; @@ -3568,10 +3538,10 @@ } case DECL_UPDATE_OFFSETS: - if (Record.size() % 2 != 0) { - Error("invalid DECL_UPDATE_OFFSETS block in AST file"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "invalid DECL_UPDATE_OFFSETS block in AST file"); for (unsigned I = 0, N = Record.size(); I != N; I += 2) { GlobalDeclID ID = getGlobalDeclID(F, Record[I]); DeclUpdateOffsets[ID].push_back(std::make_pair(&F, Record[I + 1])); @@ -3585,10 +3555,10 @@ break; case OBJC_CATEGORIES_MAP: - if (F.LocalNumObjCCategoriesInMap != 0) { - Error("duplicate OBJC_CATEGORIES_MAP record in AST file"); - return Failure; - } + if (F.LocalNumObjCCategoriesInMap != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate OBJC_CATEGORIES_MAP record in AST file"); F.LocalNumObjCCategoriesInMap = Record[0]; F.ObjCCategoriesMap = (const ObjCCategoriesInfo *)Blob.data(); @@ -3653,15 +3623,13 @@ break; case UNDEFINED_BUT_USED: - if (UndefinedButUsed.size() % 2 != 0) { - Error("Invalid existing UndefinedButUsed"); - return Failure; - } + if (UndefinedButUsed.size() % 2 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "Invalid existing UndefinedButUsed"); - if (Record.size() % 2 != 0) { - Error("invalid undefined-but-used record"); - return Failure; - } + if (Record.size() % 2 != 0) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid undefined-but-used record"); for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { UndefinedButUsed.push_back(getGlobalDeclID(F, Record[I++])); UndefinedButUsed.push_back( @@ -3700,10 +3668,10 @@ break; case MACRO_OFFSET: { - if (F.LocalNumMacros != 0) { - Error("duplicate MACRO_OFFSET record in AST file"); - return Failure; - } + if (F.LocalNumMacros != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "duplicate MACRO_OFFSET record in AST file"); F.MacroOffsets = (const uint32_t *)Blob.data(); F.LocalNumMacros = Record[0]; unsigned LocalBaseMacroID = Record[1]; @@ -3731,26 +3699,24 @@ break; case OPTIMIZE_PRAGMA_OPTIONS: - if (Record.size() != 1) { - Error("invalid pragma optimize record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma optimize record"); OptimizeOffPragmaLocation = ReadSourceLocation(F, Record[0]); break; case MSSTRUCT_PRAGMA_OPTIONS: - if (Record.size() != 1) { - Error("invalid pragma ms_struct record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma ms_struct record"); PragmaMSStructState = Record[0]; break; case POINTERS_TO_MEMBERS_PRAGMA_OPTIONS: - if (Record.size() != 2) { - Error("invalid pragma ms_struct record"); - return Failure; - } + if (Record.size() != 2) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "invalid pragma pointers to members record"); PragmaMSPointersToMembersState = Record[0]; PointersToMembersPragmaLocation = ReadSourceLocation(F, Record[1]); break; @@ -3762,18 +3728,16 @@ break; case CUDA_PRAGMA_FORCE_HOST_DEVICE_DEPTH: - if (Record.size() != 1) { - Error("invalid cuda pragma options record"); - return Failure; - } + if (Record.size() != 1) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid cuda pragma options record"); ForceCUDAHostDeviceDepth = Record[0]; break; case ALIGN_PACK_PRAGMA_OPTIONS: { - if (Record.size() < 3) { - Error("invalid pragma pack record"); - return Failure; - } + if (Record.size() < 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma pack record"); PragmaAlignPackCurrentValue = ReadAlignPackInfo(Record[0]); PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]); unsigned NumStackEntries = Record[2]; @@ -3793,10 +3757,9 @@ } case FLOAT_CONTROL_PRAGMA_OPTIONS: { - if (Record.size() < 3) { - Error("invalid pragma pack record"); - return Failure; - } + if (Record.size() < 3) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "invalid pragma float control record"); FpPragmaCurrentValue = FPOptionsOverride::getFromOpaqueInt(Record[0]); FpPragmaCurrentLocation = ReadSourceLocation(F, Record[1]); unsigned NumStackEntries = Record[2]; @@ -4257,16 +4220,23 @@ return ReadResult; } - // Here comes stuff that we only do once the entire chain is loaded. + // Here comes stuff that we only do once the entire chain is loaded. Do *not* + // remove modules from this point. Various fields are updated during reading + // the AST block and removing the modules would result in dangling pointers. + // They are generally only incidentally dereferenced, ie. a binary search + // runs over `GlobalSLocEntryMap`, which could cause an invalid module to + // be dereferenced but it wouldn't actually be used. - // Load the AST blocks of all of the modules that we loaded. We can still + // Load the AST blocks of all of the modules that we loaded. We can still // hit errors parsing the ASTs at this point. for (ImportedModule &M : Loaded) { ModuleFile &F = *M.Mod; // Read the AST block. - if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities)) + if (llvm::Error Err = ReadASTBlock(F, ClientLoadCapabilities)) { + Error(std::move(Err)); return Failure; + } // The AST block should always have a definition for the main module. if (F.isModule() && !F.DidReadTopLevelSubmodule) { @@ -4276,8 +4246,10 @@ // Read the extension blocks. while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) { - if (ASTReadResult Result = ReadExtensionBlock(F)) + if (llvm::Error Err = ReadExtensionBlock(F)) { + Error(std::move(Err)); return Failure; + } } // Once read, set the ModuleFile bit base offset and update the size in @@ -4798,32 +4770,26 @@ return false; } -ASTReader::ASTReadResult ASTReader::ReadExtensionBlock(ModuleFile &F) { +llvm::Error ASTReader::ReadExtensionBlock(ModuleFile &F) { BitstreamCursor &Stream = F.Stream; RecordData Record; while (true) { Expected MaybeEntry = Stream.advance(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::SubBlock: - if (llvm::Error Err = Stream.SkipBlock()) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = Stream.SkipBlock()) + return Err; continue; - case llvm::BitstreamEntry::EndBlock: - return Success; - + return llvm::Error::success(); case llvm::BitstreamEntry::Error: - return HadErrors; - + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::Record: break; } @@ -4832,17 +4798,15 @@ StringRef Blob; Expected MaybeRecCode = Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeRecCode) { - Error(MaybeRecCode.takeError()); - return Failure; - } + if (!MaybeRecCode) + return MaybeRecCode.takeError(); switch (MaybeRecCode.get()) { case EXTENSION_METADATA: { ModuleFileExtensionMetadata Metadata; - if (parseModuleFileExtensionMetadata(Record, Blob, Metadata)) { - Error("malformed EXTENSION_METADATA in AST file"); - return Failure; - } + if (parseModuleFileExtensionMetadata(Record, Blob, Metadata)) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "malformed EXTENSION_METADATA in AST file"); // Find a module file extension with this block name. auto Known = ModuleFileExtensions.find(Metadata.BlockName); @@ -4859,7 +4823,7 @@ } } - return Success; + return llvm::Error::success(); } void ASTReader::InitializeContext() { @@ -5439,13 +5403,11 @@ /*ValidateDiagnosticOptions=*/true); } -ASTReader::ASTReadResult -ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { +llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, + unsigned ClientLoadCapabilities) { // Enter the submodule block. - if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) { - Error(std::move(Err)); - return Failure; - } + if (llvm::Error Err = F.Stream.EnterSubBlock(SUBMODULE_BLOCK_ID)) + return Err; ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap(); bool First = true; @@ -5454,19 +5416,17 @@ while (true) { Expected MaybeEntry = F.Stream.advanceSkippingSubblocks(); - if (!MaybeEntry) { - Error(MaybeEntry.takeError()); - return Failure; - } + if (!MaybeEntry) + return MaybeEntry.takeError(); llvm::BitstreamEntry Entry = MaybeEntry.get(); switch (Entry.Kind) { case llvm::BitstreamEntry::SubBlock: // Handled for us already. case llvm::BitstreamEntry::Error: - Error("malformed block record in AST file"); - return Failure; + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed block record in AST file"); case llvm::BitstreamEntry::EndBlock: - return Success; + return llvm::Error::success(); case llvm::BitstreamEntry::Record: // The interesting case. break; @@ -5476,16 +5436,14 @@ StringRef Blob; Record.clear(); Expected MaybeKind = F.Stream.readRecord(Entry.ID, Record, &Blob); - if (!MaybeKind) { - Error(MaybeKind.takeError()); - return Failure; - } + if (!MaybeKind) + return MaybeKind.takeError(); unsigned Kind = MaybeKind.get(); - if ((Kind == SUBMODULE_METADATA) != First) { - Error("submodule metadata record should be at beginning of block"); - return Failure; - } + if ((Kind == SUBMODULE_METADATA) != First) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "submodule metadata record should be at beginning of block"); First = false; // Submodule information is only valid if we have a current module. @@ -5499,10 +5457,9 @@ break; case SUBMODULE_DEFINITION: { - if (Record.size() < 12) { - Error("malformed module definition"); - return Failure; - } + if (Record.size() < 12) + return llvm::createStringError(std::errc::illegal_byte_sequence, + "malformed module definition"); StringRef Name = Blob; unsigned Idx = 0; @@ -5534,10 +5491,9 @@ SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS; if (GlobalIndex >= SubmodulesLoaded.size() || - SubmodulesLoaded[GlobalIndex]) { - Error("too many submodules"); - return Failure; - } + SubmodulesLoaded[GlobalIndex]) + return llvm::createStringError(std::errc::invalid_argument, + "too many submodules"); if (!ParentModule) { if (const FileEntry *CurFile = CurrentModule->getASTFile()) { @@ -5548,7 +5504,7 @@ Error(diag::err_module_file_conflict, CurrentModule->getTopLevelModuleName(), CurFile->getName(), F.File->getName()); - return Failure; + return llvm::make_error(); } }