[lld] r262626 - Simplify error handling.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 3 12:28:04 PST 2016
Maybe we should make all fatal functions noreturn functions, and rename
fatal to check if it may return?
On Thu, Mar 3, 2016 at 12:25 PM, Rui Ueyama <ruiu at google.com> wrote:
> Nice change. Do you think that fatal is an appropriate name for that
> function? I guess I'd probably name this "check" or something if we didn't
> have "fatal" function before.
>
> On Thu, Mar 3, 2016 at 8:21 AM, Rafael Espindola via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: rafael
>> Date: Thu Mar 3 10:21:44 2016
>> New Revision: 262626
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=262626&view=rev
>> Log:
>> Simplify error handling.
>>
>> This makes fatal return T when there is no error. This avoids the need
>> for quite a few temporaries.
>>
>> Modified:
>> lld/trunk/ELF/Driver.cpp
>> lld/trunk/ELF/Error.cpp
>> lld/trunk/ELF/Error.h
>> lld/trunk/ELF/InputFiles.cpp
>> lld/trunk/ELF/InputSection.cpp
>> lld/trunk/ELF/SymbolTable.cpp
>> lld/trunk/ELF/Writer.cpp
>>
>> Modified: lld/trunk/ELF/Driver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Driver.cpp (original)
>> +++ lld/trunk/ELF/Driver.cpp Thu Mar 3 10:21:44 2016
>> @@ -70,17 +70,18 @@ static std::pair<ELFKind, uint16_t> pars
>> // Returns slices of MB by parsing MB as an archive file.
>> // Each slice consists of a member file in the archive.
>> static std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef
>> MB) {
>> - ErrorOr<std::unique_ptr<Archive>> FileOrErr = Archive::create(MB);
>> - fatal(FileOrErr, "Failed to parse archive");
>> - std::unique_ptr<Archive> File = std::move(*FileOrErr);
>> + std::unique_ptr<Archive> File =
>> + fatal(Archive::create(MB), "Failed to parse archive");
>>
>> std::vector<MemoryBufferRef> V;
>> - for (const ErrorOr<Archive::Child> &C : File->children()) {
>> - fatal(C, "Could not get the child of the archive " +
>> File->getFileName());
>> - ErrorOr<MemoryBufferRef> MbOrErr = C->getMemoryBufferRef();
>> - fatal(MbOrErr, "Could not get the buffer for a child of the archive
>> " +
>> - File->getFileName());
>> - V.push_back(*MbOrErr);
>> + for (const ErrorOr<Archive::Child> &COrErr : File->children()) {
>> + Archive::Child C = fatal(COrErr, "Could not get the child of the
>> archive " +
>> + File->getFileName());
>> + MemoryBufferRef Mb =
>> + fatal(C.getMemoryBufferRef(),
>> + "Could not get the buffer for a child of the archive " +
>> + File->getFileName());
>> + V.push_back(Mb);
>> }
>> return V;
>> }
>>
>> Modified: lld/trunk/ELF/Error.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Error.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Error.cpp (original)
>> +++ lld/trunk/ELF/Error.cpp Thu Mar 3 10:21:44 2016
>> @@ -50,6 +50,10 @@ void fatal(const Twine &Msg) {
>> exit(1);
>> }
>>
>> +void fatal(const Twine &Msg, const Twine &Prefix) {
>> + fatal(Prefix + ": " + Msg);
>> +}
>> +
>>
>
> This new function seems to be used only once in Error.cpp. You can remove
> this if you update the caller to use +.
>
> void fatal(std::error_code EC, const Twine &Prefix) {
>> if (EC)
>> fatal(Prefix + ": " + EC.message());
>>
>> Modified: lld/trunk/ELF/Error.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Error.h?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Error.h (original)
>> +++ lld/trunk/ELF/Error.h Thu Mar 3 10:21:44 2016
>> @@ -38,14 +38,21 @@ template <typename T> bool error(const E
>> }
>>
>> LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg);
>> +LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg, const Twine
>> &Prefix);
>> void fatal(std::error_code EC, const Twine &Prefix);
>> void fatal(std::error_code EC);
>>
>> -template <typename T> void fatal(const ErrorOr<T> &V, const Twine
>> &Prefix) {
>> - fatal(V.getError(), Prefix);
>> +template <class T> T fatal(ErrorOr<T> EO) {
>> + if (EO)
>> + return std::move(*EO);
>> + fatal(EO.getError().message());
>> }
>>
>> -template <typename T> void fatal(const ErrorOr<T> &V) {
>> fatal(V.getError()); }
>> +template <class T> T fatal(ErrorOr<T> EO, const Twine &Prefix) {
>> + if (EO)
>> + return std::move(*EO);
>> + fatal(EO.getError().message(), Prefix);
>> +}
>>
>> } // namespace elf
>> } // namespace lld
>>
>> Modified: lld/trunk/ELF/InputFiles.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/InputFiles.cpp (original)
>> +++ lld/trunk/ELF/InputFiles.cpp Thu Mar 3 10:21:44 2016
>> @@ -73,9 +73,7 @@ uint32_t ELFFileBase<ELFT>::getSectionIn
>> template <class ELFT> void ELFFileBase<ELFT>::initStringTable() {
>> if (!Symtab)
>> return;
>> - ErrorOr<StringRef> StringTableOrErr =
>> ELFObj.getStringTableForSymtab(*Symtab);
>> - fatal(StringTableOrErr);
>> - StringTable = *StringTableOrErr;
>> + StringTable = fatal(ELFObj.getStringTableForSymtab(*Symtab));
>> }
>>
>> template <class ELFT>
>> @@ -124,26 +122,19 @@ template <class ELFT>
>> StringRef elf::ObjectFile<ELFT>::getShtGroupSignature(const Elf_Shdr
>> &Sec) {
>> const ELFFile<ELFT> &Obj = this->ELFObj;
>> uint32_t SymtabdSectionIndex = Sec.sh_link;
>> - ErrorOr<const Elf_Shdr *> SecOrErr =
>> Obj.getSection(SymtabdSectionIndex);
>> - fatal(SecOrErr);
>> - const Elf_Shdr *SymtabSec = *SecOrErr;
>> + const Elf_Shdr *SymtabSec = fatal(Obj.getSection(SymtabdSectionIndex));
>> uint32_t SymIndex = Sec.sh_info;
>> const Elf_Sym *Sym = Obj.getSymbol(SymtabSec, SymIndex);
>> - ErrorOr<StringRef> StringTableOrErr =
>> Obj.getStringTableForSymtab(*SymtabSec);
>> - fatal(StringTableOrErr);
>> - ErrorOr<StringRef> SignatureOrErr = Sym->getName(*StringTableOrErr);
>> - fatal(SignatureOrErr);
>> - return *SignatureOrErr;
>> + StringRef StringTable = fatal(Obj.getStringTableForSymtab(*SymtabSec));
>> + return fatal(Sym->getName(StringTable));
>> }
>>
>> template <class ELFT>
>> ArrayRef<typename elf::ObjectFile<ELFT>::uint32_X>
>> elf::ObjectFile<ELFT>::getShtGroupEntries(const Elf_Shdr &Sec) {
>> const ELFFile<ELFT> &Obj = this->ELFObj;
>> - ErrorOr<ArrayRef<uint32_X>> EntriesOrErr =
>> - Obj.template getSectionContentsAsArray<uint32_X>(&Sec);
>> - fatal(EntriesOrErr);
>> - ArrayRef<uint32_X> Entries = *EntriesOrErr;
>> + ArrayRef<uint32_X> Entries =
>> + fatal(Obj.template getSectionContentsAsArray<uint32_X>(&Sec));
>> if (Entries.empty() || Entries[0] != GRP_COMDAT)
>> fatal("Unsupported SHT_GROUP format");
>> return Entries.slice(1);
>> @@ -202,12 +193,9 @@ void elf::ObjectFile<ELFT>::initializeSe
>> case SHT_SYMTAB:
>> this->Symtab = &Sec;
>> break;
>> - case SHT_SYMTAB_SHNDX: {
>> - ErrorOr<ArrayRef<Elf_Word>> ErrorOrTable = Obj.getSHNDXTable(Sec);
>> - fatal(ErrorOrTable);
>> - this->SymtabSHNDX = *ErrorOrTable;
>> + case SHT_SYMTAB_SHNDX:
>> + this->SymtabSHNDX = fatal(Obj.getSHNDXTable(Sec));
>> break;
>> - }
>> case SHT_STRTAB:
>> case SHT_NULL:
>> break;
>> @@ -248,9 +236,7 @@ void elf::ObjectFile<ELFT>::initializeSe
>> template <class ELFT>
>> InputSectionBase<ELFT> *
>> elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
>> - ErrorOr<StringRef> NameOrErr = this->ELFObj.getSectionName(&Sec);
>> - fatal(NameOrErr);
>> - StringRef Name = *NameOrErr;
>> + StringRef Name = fatal(this->ELFObj.getSectionName(&Sec));
>>
>> // .note.GNU-stack is a marker section to control the presence of
>> // PT_GNU_STACK segment in outputs. Since the presence of the segment
>> @@ -300,9 +286,7 @@ elf::ObjectFile<ELFT>::getSection(const
>>
>> template <class ELFT>
>> SymbolBody *elf::ObjectFile<ELFT>::createSymbolBody(const Elf_Sym *Sym) {
>> - ErrorOr<StringRef> NameOrErr = Sym->getName(this->StringTable);
>> - fatal(NameOrErr);
>> - StringRef Name = *NameOrErr;
>> + StringRef Name = fatal(Sym->getName(this->StringTable));
>>
>> switch (Sym->st_shndx) {
>> case SHN_UNDEF:
>> @@ -328,9 +312,7 @@ SymbolBody *elf::ObjectFile<ELFT>::creat
>> }
>>
>> void ArchiveFile::parse() {
>> - ErrorOr<std::unique_ptr<Archive>> FileOrErr = Archive::create(MB);
>> - fatal(FileOrErr, "Failed to parse archive");
>> - File = std::move(*FileOrErr);
>> + File = fatal(Archive::create(MB), "Failed to parse archive");
>>
>> // Allocate a buffer for Lazy objects.
>> size_t NumSyms = File->getNumberOfSymbols();
>> @@ -343,18 +325,16 @@ void ArchiveFile::parse() {
>>
>> // Returns a buffer pointing to a member file containing a given symbol.
>> MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {
>> - ErrorOr<Archive::Child> COrErr = Sym->getMember();
>> - fatal(COrErr, "Could not get the member for symbol " + Sym->getName());
>> - const Archive::Child &C = *COrErr;
>> + Archive::Child C =
>> + fatal(Sym->getMember(),
>> + "Could not get the member for symbol " + Sym->getName());
>>
>> if (!Seen.insert(C.getChildOffset()).second)
>> return MemoryBufferRef();
>>
>> - ErrorOr<MemoryBufferRef> RefOrErr = C.getMemoryBufferRef();
>> - if (!RefOrErr)
>> - fatal(RefOrErr, "Could not get the buffer for the member defining
>> symbol " +
>> - Sym->getName());
>> - return *RefOrErr;
>> + return fatal(C.getMemoryBufferRef(),
>> + "Could not get the buffer for the member defining symbol
>> " +
>> + Sym->getName());
>> }
>>
>> template <class ELFT>
>> @@ -367,9 +347,7 @@ SharedFile<ELFT>::getSection(const Elf_S
>> uint32_t Index = this->getSectionIndex(Sym);
>> if (Index == 0)
>> return nullptr;
>> - ErrorOr<const Elf_Shdr *> Ret = this->ELFObj.getSection(Index);
>> - fatal(Ret);
>> - return *Ret;
>> + return fatal(this->ELFObj.getSection(Index));
>> }
>>
>> // Partially parse the shared object file so that we can call
>> @@ -390,13 +368,10 @@ template <class ELFT> void SharedFile<EL
>> case SHT_DYNAMIC:
>> DynamicSec = &Sec;
>> break;
>> - case SHT_SYMTAB_SHNDX: {
>> - ErrorOr<ArrayRef<Elf_Word>> ErrorOrTable = Obj.getSHNDXTable(Sec);
>> - fatal(ErrorOrTable);
>> - this->SymtabSHNDX = *ErrorOrTable;
>> + case SHT_SYMTAB_SHNDX:
>> + this->SymtabSHNDX = fatal(Obj.getSHNDXTable(Sec));
>> break;
>> }
>> - }
>> }
>>
>> this->initStringTable();
>> @@ -444,11 +419,8 @@ bool BitcodeFile::classof(const InputFil
>>
>> void BitcodeFile::parse(DenseSet<StringRef> &ComdatGroups) {
>> LLVMContext Context;
>> - ErrorOr<std::unique_ptr<IRObjectFile>> ObjOrErr =
>> - IRObjectFile::create(MB, Context);
>> - fatal(ObjOrErr);
>> - IRObjectFile &Obj = **ObjOrErr;
>> - const Module &M = Obj.getModule();
>> + std::unique_ptr<IRObjectFile> Obj = fatal(IRObjectFile::create(MB,
>> Context));
>> + const Module &M = Obj->getModule();
>>
>> DenseSet<const Comdat *> KeptComdats;
>> for (const auto &P : M.getComdatSymbolTable()) {
>> @@ -457,8 +429,8 @@ void BitcodeFile::parse(DenseSet<StringR
>> KeptComdats.insert(&P.second);
>> }
>>
>> - for (const BasicSymbolRef &Sym : Obj.symbols()) {
>> - if (const GlobalValue *GV = Obj.getSymbolGV(Sym.getRawDataRefImpl()))
>> + for (const BasicSymbolRef &Sym : Obj->symbols()) {
>> + if (const GlobalValue *GV =
>> Obj->getSymbolGV(Sym.getRawDataRefImpl()))
>> if (const Comdat *C = GV->getComdat())
>> if (!KeptComdats.count(C))
>> continue;
>>
>> Modified: lld/trunk/ELF/InputSection.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/InputSection.cpp (original)
>> +++ lld/trunk/ELF/InputSection.cpp Thu Mar 3 10:21:44 2016
>> @@ -39,17 +39,12 @@ InputSectionBase<ELFT>::InputSectionBase
>> }
>>
>> template <class ELFT> StringRef InputSectionBase<ELFT>::getSectionName()
>> const {
>> - ErrorOr<StringRef> Name = File->getObj().getSectionName(this->Header);
>> - fatal(Name);
>> - return *Name;
>> + return fatal(File->getObj().getSectionName(this->Header));
>> }
>>
>> template <class ELFT>
>> ArrayRef<uint8_t> InputSectionBase<ELFT>::getSectionData() const {
>> - ErrorOr<ArrayRef<uint8_t>> Ret =
>> - this->File->getObj().getSectionContents(this->Header);
>> - fatal(Ret);
>> - return *Ret;
>> + return fatal(this->File->getObj().getSectionContents(this->Header));
>> }
>>
>> template <class ELFT>
>>
>> Modified: lld/trunk/ELF/SymbolTable.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/SymbolTable.cpp (original)
>> +++ lld/trunk/ELF/SymbolTable.cpp Thu Mar 3 10:21:44 2016
>> @@ -136,11 +136,9 @@ ObjectFile<ELFT> *SymbolTable<ELFT>::cre
>> for (const std::unique_ptr<BitcodeFile> &F : BitcodeFiles) {
>> std::unique_ptr<MemoryBuffer> Buffer =
>> MemoryBuffer::getMemBuffer(F->MB, false);
>> - ErrorOr<std::unique_ptr<Module>> MOrErr =
>> - getLazyBitcodeModule(std::move(Buffer), Context,
>> - /*ShouldLazyLoadMetadata*/ true);
>> - fatal(MOrErr);
>> - std::unique_ptr<Module> &M = *MOrErr;
>> + std::unique_ptr<Module> M =
>> + fatal(getLazyBitcodeModule(std::move(Buffer), Context,
>> + /*ShouldLazyLoadMetadata*/ true));
>> L.linkInModule(std::move(M));
>> }
>> std::unique_ptr<InputFile> F = codegen(Combined);
>>
>> Modified: lld/trunk/ELF/Writer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=262626&r1=262625&r2=262626&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Writer.cpp (original)
>> +++ lld/trunk/ELF/Writer.cpp Thu Mar 3 10:21:44 2016
>> @@ -564,9 +564,7 @@ template <class ELFT> void Writer<ELFT>:
>> return;
>> for (const std::unique_ptr<ObjectFile<ELFT>> &F :
>> Symtab.getObjectFiles()) {
>> for (const Elf_Sym &Sym : F->getLocalSymbols()) {
>> - ErrorOr<StringRef> SymNameOrErr = Sym.getName(F->getStringTable());
>> - fatal(SymNameOrErr);
>> - StringRef SymName = *SymNameOrErr;
>> + StringRef SymName = fatal(Sym.getName(F->getStringTable()));
>> if (!shouldKeepInSymtab<ELFT>(*F, SymName, Sym))
>> continue;
>> if (Sym.st_shndx != SHN_ABS) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160303/3b734618/attachment.html>
More information about the llvm-commits
mailing list