<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 6, 2017 at 5:30 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">LGTM<br>
<br>
BTW, as a followup you can even convert the macro to a template<br>
function, no?</blockquote><div><br></div><div>For something like CHECK(foo, this), yes, but in general, no. For example, we cannot lazy-evaluate the second argument in CHECK(foo, toString(this) + ": out of range")) using template.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> writes:<br>
<br>
> ruiu created this revision.<br>
> Herald added a subscriber: emaste.<br>
><br>
> Remove checkToString functions and use toString instead.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D40928" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D40928</a><br>
><br>
> Files:<br>
> lld/COFF/InputFiles.cpp<br>
> lld/ELF/InputFiles.cpp<br>
> lld/include/lld/Common/<wbr>ErrorHandler.h<br>
><br>
</div></div>> Index: lld/include/lld/Common/<wbr>ErrorHandler.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/include/lld/Common/<wbr>ErrorHandler.h<br>
> +++ lld/include/lld/Common/<wbr>ErrorHandler.h<br>
> @@ -102,12 +102,10 @@<br>
> return std::move(*E);<br>
> }<br>
><br>
> -inline std::string checkToString(const Twine &S) { return S.str(); }<br>
> -inline std::string checkToString(std::string S) { return S; }<br>
> -inline std::string checkToString(const char *S) { return S; }<br>
> +inline std::string toString(const Twine &S) { return S.str(); }<br>
><br>
> // To evaluate the second argument lazily, we use C macro.<br>
> -#define CHECK(E, S) check2(E, [&] { return checkToString(S); })<br>
> +#define CHECK(E, S) check2(E, [&] { return toString(S); })<br>
><br>
> } // namespace lld<br>
><br>
> Index: lld/ELF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.cpp<br>
> +++ lld/ELF/InputFiles.cpp<br>
> @@ -209,20 +209,19 @@<br>
><br>
> template <class ELFT><br>
> uint32_t ELFFileBase<ELFT>::<wbr>getSectionIndex(const Elf_Sym &Sym) const {<br>
> - return CHECK(getObj().<wbr>getSectionIndex(&Sym, ELFSyms, SymtabSHNDX),<br>
> - toString(this));<br>
> + return CHECK(getObj().<wbr>getSectionIndex(&Sym, ELFSyms, SymtabSHNDX), this);<br>
> }<br>
><br>
> template <class ELFT><br>
> void ELFFileBase<ELFT>::initSymtab(<wbr>ArrayRef<Elf_Shdr> Sections,<br>
> const Elf_Shdr *Symtab) {<br>
> FirstNonLocal = Symtab->sh_info;<br>
> - ELFSyms = CHECK(getObj().symbols(Symtab)<wbr>, toString(this));<br>
> + ELFSyms = CHECK(getObj().symbols(Symtab)<wbr>, this);<br>
> if (FirstNonLocal == 0 || FirstNonLocal > ELFSyms.size())<br>
> fatal(toString(this) + ": invalid sh_info in symbol table");<br>
><br>
> - StringTable = CHECK(getObj().<wbr>getStringTableForSymtab(*<wbr>Symtab, Sections),<br>
> - toString(this));<br>
> + StringTable =<br>
> + CHECK(getObj().<wbr>getStringTableForSymtab(*<wbr>Symtab, Sections), this);<br>
> }<br>
><br>
> template <class ELFT><br>
> @@ -254,12 +253,11 @@<br>
> // sh_info contains a symbol index, so we fetch a symbol and read its name.<br>
> if (this->ELFSyms.empty())<br>
> this->initSymtab(<br>
> - Sections,<br>
> - CHECK(object::getSection<ELFT><wbr>(Sections, Sec.sh_link), toString(this)));<br>
> + Sections, CHECK(object::getSection<ELFT><wbr>(Sections, Sec.sh_link), this));<br>
><br>
> - const Elf_Sym *Sym = CHECK(<br>
> - object::getSymbol<ELFT>(this-><wbr>ELFSyms, Sec.sh_info), toString(this));<br>
> - StringRef Signature = CHECK(Sym->getName(this-><wbr>StringTable), toString(this));<br>
> + const Elf_Sym *Sym =<br>
> + CHECK(object::getSymbol<ELFT>(<wbr>this->ELFSyms, Sec.sh_info), this);<br>
> + StringRef Signature = CHECK(Sym->getName(this-><wbr>StringTable), this);<br>
><br>
> // As a special case, if a symbol is a section symbol and has no name,<br>
> // we use a section name as a signature.<br>
> @@ -277,8 +275,8 @@<br>
> ArrayRef<typename ObjFile<ELFT>::Elf_Word><br>
> ObjFile<ELFT>::<wbr>getShtGroupEntries(const Elf_Shdr &Sec) {<br>
> const ELFFile<ELFT> &Obj = this->getObj();<br>
> - ArrayRef<Elf_Word> Entries = CHECK(<br>
> - Obj.template getSectionContentsAsArray<Elf_<wbr>Word>(&Sec), toString(this));<br>
> + ArrayRef<Elf_Word> Entries =<br>
> + CHECK(Obj.template getSectionContentsAsArray<Elf_<wbr>Word>(&Sec), this);<br>
> if (Entries.empty() || Entries[0] != GRP_COMDAT)<br>
> fatal(toString(this) + ": unsupported SHT_GROUP format");<br>
> return Entries.slice(1);<br>
> @@ -323,12 +321,11 @@<br>
> DenseSet<CachedHashStringRef> &ComdatGroups) {<br>
> const ELFFile<ELFT> &Obj = this->getObj();<br>
><br>
> - ArrayRef<Elf_Shdr> ObjSections =<br>
> - CHECK(this->getObj().sections(<wbr>), toString(this));<br>
> + ArrayRef<Elf_Shdr> ObjSections = CHECK(this->getObj().sections(<wbr>), this);<br>
> uint64_t Size = ObjSections.size();<br>
> this->Sections.resize(Size);<br>
> this->SectionStringTable =<br>
> - CHECK(Obj.<wbr>getSectionStringTable(<wbr>ObjSections), toString(this));<br>
> + CHECK(Obj.<wbr>getSectionStringTable(<wbr>ObjSections), this);<br>
><br>
> for (size_t I = 0, E = ObjSections.size(); I < E; I++) {<br>
> if (this->Sections[I] == &InputSection::Discarded)<br>
> @@ -374,8 +371,7 @@<br>
> this->initSymtab(ObjSections, &Sec);<br>
> break;<br>
> case SHT_SYMTAB_SHNDX:<br>
> - this->SymtabSHNDX =<br>
> - CHECK(Obj.getSHNDXTable(Sec, ObjSections), toString(this));<br>
> + this->SymtabSHNDX = CHECK(Obj.getSHNDXTable(Sec, ObjSections), this);<br>
> break;<br>
> case SHT_STRTAB:<br>
> case SHT_NULL:<br>
> @@ -520,13 +516,12 @@<br>
><br>
> size_t NumRelocations;<br>
> if (Sec.sh_type == SHT_RELA) {<br>
> - ArrayRef<Elf_Rela> Rels =<br>
> - CHECK(this->getObj().relas(&<wbr>Sec), toString(this));<br>
> + ArrayRef<Elf_Rela> Rels = CHECK(this->getObj().relas(&<wbr>Sec), this);<br>
> Target->FirstRelocation = Rels.begin();<br>
> NumRelocations = Rels.size();<br>
> Target->AreRelocsRela = true;<br>
> } else {<br>
> - ArrayRef<Elf_Rel> Rels = CHECK(this->getObj().rels(&<wbr>Sec), toString(this));<br>
> + ArrayRef<Elf_Rel> Rels = CHECK(this->getObj().rels(&<wbr>Sec), this);<br>
> Target->FirstRelocation = Rels.begin();<br>
> NumRelocations = Rels.size();<br>
> Target->AreRelocsRela = false;<br>
> @@ -595,8 +590,7 @@<br>
><br>
> template <class ELFT><br>
> StringRef ObjFile<ELFT>::getSectionName(<wbr>const Elf_Shdr &Sec) {<br>
> - return CHECK(this->getObj().<wbr>getSectionName(&Sec, SectionStringTable),<br>
> - toString(this));<br>
> + return CHECK(this->getObj().<wbr>getSectionName(&Sec, SectionStringTable), this);<br>
> }<br>
><br>
> template <class ELFT> void ObjFile<ELFT>::<wbr>initializeSymbols() {<br>
> @@ -628,7 +622,7 @@<br>
><br>
> if (Binding == STB_LOCAL) {<br>
> if (Sym->getType() == STT_FILE)<br>
> - SourceFile = CHECK(Sym->getName(this-><wbr>StringTable), toString(this));<br>
> + SourceFile = CHECK(Sym->getName(this-><wbr>StringTable), this);<br>
><br>
> if (this->StringTable.size() <= Sym->st_name)<br>
> fatal(toString(this) + ": invalid symbol name offset");<br>
> @@ -640,7 +634,7 @@<br>
> return make<Defined>(this, Name, Binding, StOther, Type, Value, Size, Sec);<br>
> }<br>
><br>
> - StringRef Name = CHECK(Sym->getName(this-><wbr>StringTable), toString(this));<br>
> + StringRef Name = CHECK(Sym->getName(this-><wbr>StringTable), this);<br>
><br>
> switch (Sym->st_shndx) {<br>
> case SHN_UNDEF:<br>
> @@ -695,8 +689,7 @@<br>
> Sym->getName());<br>
><br>
> if (C.getParent()->isThin() && Tar)<br>
> - Tar->append(relativeToRoot(<wbr>CHECK(C.getFullName(), toString(this))),<br>
> - Ret.getBuffer());<br>
> + Tar->append(relativeToRoot(<wbr>CHECK(C.getFullName(), this)), Ret.getBuffer());<br>
> if (C.getParent()->isThin())<br>
> return {Ret, 0};<br>
> return {Ret, C.getChildOffset()};<br>
> @@ -712,7 +705,7 @@<br>
> template <class ELFT> void SharedFile<ELFT>::parseSoName(<wbr>) {<br>
> const Elf_Shdr *DynamicSec = nullptr;<br>
> const ELFFile<ELFT> Obj = this->getObj();<br>
> - ArrayRef<Elf_Shdr> Sections = CHECK(Obj.sections(), toString(this));<br>
> + ArrayRef<Elf_Shdr> Sections = CHECK(Obj.sections(), this);<br>
><br>
> // Search for .dynsym, .dynamic, .symtab, .gnu.version and .gnu.version_d.<br>
> for (const Elf_Shdr &Sec : Sections) {<br>
> @@ -726,8 +719,7 @@<br>
> DynamicSec = &Sec;<br>
> break;<br>
> case SHT_SYMTAB_SHNDX:<br>
> - this->SymtabSHNDX =<br>
> - CHECK(Obj.getSHNDXTable(Sec, Sections), toString(this));<br>
> + this->SymtabSHNDX = CHECK(Obj.getSHNDXTable(Sec, Sections), this);<br>
> break;<br>
> case SHT_GNU_versym:<br>
> this->VersymSec = &Sec;<br>
> @@ -745,8 +737,7 @@<br>
> if (!DynamicSec)<br>
> return;<br>
> ArrayRef<Elf_Dyn> Arr =<br>
> - CHECK(Obj.template getSectionContentsAsArray<Elf_<wbr>Dyn>(DynamicSec),<br>
> - toString(this));<br>
> + CHECK(Obj.template getSectionContentsAsArray<Elf_<wbr>Dyn>(DynamicSec), this);<br>
> for (const Elf_Dyn &Dyn : Arr) {<br>
> if (Dyn.d_tag == DT_SONAME) {<br>
> uint64_t Val = Dyn.getVal();<br>
> @@ -805,8 +796,7 @@<br>
> const Elf_Versym *Versym = nullptr;<br>
> std::vector<const Elf_Verdef *> Verdefs = parseVerdefs(Versym);<br>
><br>
> - ArrayRef<Elf_Shdr> Sections =<br>
> - CHECK(this->getObj().sections(<wbr>), toString(this));<br>
> + ArrayRef<Elf_Shdr> Sections = CHECK(this->getObj().sections(<wbr>), this);<br>
><br>
> // Add symbols to the symbol table.<br>
> Elf_Sym_Range Syms = this->getGlobalELFSyms();<br>
> @@ -819,7 +809,7 @@<br>
> bool Hidden = VersymIndex & VERSYM_HIDDEN;<br>
> VersymIndex = VersymIndex & ~VERSYM_HIDDEN;<br>
><br>
> - StringRef Name = CHECK(Sym.getName(this-><wbr>StringTable), toString(this));<br>
> + StringRef Name = CHECK(Sym.getName(this-><wbr>StringTable), this);<br>
> if (Sym.isUndefined()) {<br>
> Undefs.push_back(Name);<br>
> continue;<br>
> @@ -916,7 +906,7 @@<br>
> MemoryBufferRef MBRef(MB.getBuffer(),<br>
> Saver.save(ArchiveName + MB.getBufferIdentifier() +<br>
> utostr(OffsetInArchive)));<br>
> - Obj = CHECK(lto::InputFile::create(<wbr>MBRef), toString(this));<br>
> + Obj = CHECK(lto::InputFile::create(<wbr>MBRef), this);<br>
><br>
> Triple T(Obj->getTargetTriple());<br>
> EKind = getBitcodeELFKind(T);<br>
> @@ -1081,28 +1071,28 @@<br>
> typedef typename ELFT::SymRange Elf_Sym_Range;<br>
><br>
> ELFFile<ELFT> Obj = check(ELFFile<ELFT>::create(<wbr>this->MB.getBuffer()));<br>
> - ArrayRef<Elf_Shdr> Sections = CHECK(Obj.sections(), toString(this));<br>
> + ArrayRef<Elf_Shdr> Sections = CHECK(Obj.sections(), this);<br>
> for (const Elf_Shdr &Sec : Sections) {<br>
> if (Sec.sh_type != SHT_SYMTAB)<br>
> continue;<br>
><br>
> - Elf_Sym_Range Syms = CHECK(Obj.symbols(&Sec), toString(this));<br>
> + Elf_Sym_Range Syms = CHECK(Obj.symbols(&Sec), this);<br>
> uint32_t FirstNonLocal = Sec.sh_info;<br>
> StringRef StringTable =<br>
> - CHECK(Obj.<wbr>getStringTableForSymtab(Sec, Sections), toString(this));<br>
> + CHECK(Obj.<wbr>getStringTableForSymtab(Sec, Sections), this);<br>
> std::vector<StringRef> V;<br>
><br>
> for (const Elf_Sym &Sym : Syms.slice(FirstNonLocal))<br>
> if (Sym.st_shndx != SHN_UNDEF)<br>
> - V.push_back(CHECK(Sym.getName(<wbr>StringTable), toString(this)));<br>
> + V.push_back(CHECK(Sym.getName(<wbr>StringTable), this));<br>
> return V;<br>
> }<br>
> return {};<br>
> }<br>
><br>
> std::vector<StringRef> LazyObjFile::<wbr>getBitcodeSymbols() {<br>
> std::unique_ptr<lto::<wbr>InputFile> Obj =<br>
> - CHECK(lto::InputFile::create(<wbr>this->MB), toString(this));<br>
> + CHECK(lto::InputFile::create(<wbr>this->MB), this);<br>
> std::vector<StringRef> V;<br>
> for (const lto::InputFile::Symbol &Sym : Obj->symbols())<br>
> if (!Sym.isUndefined())<br>
> Index: lld/COFF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/COFF/InputFiles.cpp<br>
> +++ lld/COFF/InputFiles.cpp<br>
> @@ -63,7 +63,7 @@<br>
><br>
> void ArchiveFile::parse() {<br>
> // Parse a MemoryBufferRef as an archive file.<br>
> - File = CHECK(Archive::create(MB), toString(this));<br>
> + File = CHECK(Archive::create(MB), this);<br>
><br>
> // Read the symbol table to construct Lazy objects.<br>
> for (const Archive::Symbol &Sym : File->symbols())<br>
> @@ -104,7 +104,7 @@<br>
><br>
> void ObjFile::parse() {<br>
> // Parse a memory buffer as a COFF file.<br>
> - std::unique_ptr<Binary> Bin = CHECK(createBinary(MB), toString(this));<br>
> + std::unique_ptr<Binary> Bin = CHECK(createBinary(MB), this);<br>
><br>
> if (auto *Obj = dyn_cast<COFFObjectFile>(Bin.<wbr>get())) {<br>
> Bin.release();<br>
</blockquote></div><br></div></div>