Skip to content

Commit

Permalink
[Object] Revert r275316, Archive::child_iterator changes, while I upd…
Browse files Browse the repository at this point in the history
…ate lld.

Should fix the bots broken by r275316.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@275353 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
lhames committed Jul 14, 2016
1 parent 3dc0c3e commit 5daf897
Show file tree
Hide file tree
Showing 15 changed files with 197 additions and 236 deletions.
41 changes: 17 additions & 24 deletions include/llvm/Object/Archive.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,21 @@ class Archive : public Binary {
};

class child_iterator {
Child C;
Error *E;
ErrorOr<Child> child;

public:
child_iterator() : C(Child(nullptr, nullptr, nullptr)), E(nullptr) {}
child_iterator(const Child &C, Error *E) : C(C), E(E) {}
const Child *operator->() const { return &C; }
const Child &operator*() const { return C; }
child_iterator() : child(Child(nullptr, nullptr, nullptr)) {}
child_iterator(const Child &c) : child(c) {}
child_iterator(std::error_code EC) : child(EC) {}
const ErrorOr<Child> *operator->() const { return &child; }
const ErrorOr<Child> &operator*() const { return child; }

bool operator==(const child_iterator &other) const {
// Ignore errors here: If an error occurred during increment then getNext
// will have been set to child_end(), and the following comparison should
// do the right thing.
return C == other.C;
// We ignore error states so that comparisions with end() work, which
// allows range loops.
if (child.getError() || other.child.getError())
return false;
return *child == *other.child;
}

bool operator!=(const child_iterator &other) const {
Expand All @@ -129,15 +130,8 @@ class Archive : public Binary {
// Code in loops with child_iterators must check for errors on each loop
// iteration. And if there is an error break out of the loop.
child_iterator &operator++() { // Preincrement
assert(E && "Can't increment iterator with no Error attached");
if (auto ChildOrErr = C.getNext())
C = *ChildOrErr;
else {
ErrorAsOutParameter ErrAsOutParam(*E);
C = C.getParent()->child_end().C;
*E = errorCodeToError(ChildOrErr.getError());
E = nullptr;
}
assert(child && "Can't increment iterator with error");
child = child->getNext();
return *this;
}
};
Expand Down Expand Up @@ -196,11 +190,10 @@ class Archive : public Binary {
Kind kind() const { return (Kind)Format; }
bool isThin() const { return IsThin; }

child_iterator child_begin(Error &Err, bool SkipInternal = true) const;
child_iterator child_begin(bool SkipInternal = true) const;
child_iterator child_end() const;
iterator_range<child_iterator> children(Error &Err,
bool SkipInternal = true) const {
return make_range(child_begin(Err, SkipInternal), child_end());
iterator_range<child_iterator> children(bool SkipInternal = true) const {
return make_range(child_begin(SkipInternal), child_end());
}

symbol_iterator symbol_begin() const;
Expand All @@ -215,7 +208,7 @@ class Archive : public Binary {
}

// check if a symbol is in the archive
child_iterator findSym(Error &Err, StringRef name) const;
child_iterator findSym(StringRef name) const;

bool hasSymbolTable() const;
StringRef getSymbolTable() const { return SymbolTable; }
Expand Down
5 changes: 0 additions & 5 deletions include/llvm/Support/Error.h
Original file line number Diff line number Diff line change
Expand Up @@ -940,11 +940,6 @@ class ExitOnError {
std::function<int(const Error &)> GetExitCode;
};

/// Report a serious error, calling any installed error handler. See
/// ErrorHandling.h.
LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
bool gen_crash_diag = true);

} // namespace llvm

#endif // LLVM_SUPPORT_ERROR_H
9 changes: 4 additions & 5 deletions lib/ExecutionEngine/MCJIT/MCJIT.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,13 @@ RuntimeDyld::SymbolInfo MCJIT::findSymbol(const std::string &Name,
for (object::OwningBinary<object::Archive> &OB : Archives) {
object::Archive *A = OB.getBinary();
// Look for our symbols in each Archive
Error Err;
object::Archive::child_iterator ChildIt = A->findSym(Err, Name);
if (Err)
report_fatal_error(std::move(Err));
object::Archive::child_iterator ChildIt = A->findSym(Name);
if (std::error_code EC = ChildIt->getError())
report_fatal_error(EC.message());
if (ChildIt != A->child_end()) {
// FIXME: Support nested archives?
Expected<std::unique_ptr<object::Binary>> ChildBinOrErr =
ChildIt->getAsBinary();
(*ChildIt)->getAsBinary();
if (!ChildBinOrErr) {
// TODO: Actually report errors helpfully.
consumeError(ChildBinOrErr.takeError());
Expand Down
9 changes: 4 additions & 5 deletions lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,13 @@ class OrcMCJITReplacement : public ExecutionEngine {
for (object::OwningBinary<object::Archive> &OB : Archives) {
object::Archive *A = OB.getBinary();
// Look for our symbols in each Archive
Error Err;
object::Archive::child_iterator ChildIt = A->findSym(Err, Name);
if (Err)
report_fatal_error(std::move(Err));
object::Archive::child_iterator ChildIt = A->findSym(Name);
if (std::error_code EC = ChildIt->getError())
report_fatal_error(EC.message());
if (ChildIt != A->child_end()) {
// FIXME: Support nested archives?
Expected<std::unique_ptr<object::Binary>> ChildBinOrErr =
ChildIt->getAsBinary();
(*ChildIt)->getAsBinary();
if (!ChildBinOrErr) {
// TODO: Actually report errors helpfully.
consumeError(ChildBinOrErr.takeError());
Expand Down
48 changes: 22 additions & 26 deletions lib/Object/Archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,12 @@ Archive::Archive(MemoryBufferRef Source, Error &Err)
}

// Get the special members.
child_iterator I = child_begin(Err, false);
if (Err)
child_iterator I = child_begin(false);
std::error_code ec;
if ((ec = I->getError())) {
Err = errorCodeToError(ec);
return;
}
child_iterator E = child_end();

// This is at least a valid empty archive. Since an empty archive is the
Expand All @@ -312,13 +315,13 @@ Archive::Archive(MemoryBufferRef Source, Error &Err)
Err = Error::success();
return;
}
const Child *C = &*I;
const Child *C = &**I;

auto Increment = [&]() {
++I;
if (Err)
if ((Err = errorCodeToError(I->getError())))
return true;
C = &*I;
C = &**I;
return false;
};

Expand Down Expand Up @@ -363,7 +366,8 @@ Archive::Archive(MemoryBufferRef Source, Error &Err)
Format = K_BSD;
// We know this is BSD, so getName will work since there is no string table.
ErrorOr<StringRef> NameOrErr = C->getName();
if (auto ec = NameOrErr.getError()) {
ec = NameOrErr.getError();
if (ec) {
Err = errorCodeToError(ec);
return;
}
Expand Down Expand Up @@ -461,29 +465,23 @@ Archive::Archive(MemoryBufferRef Source, Error &Err)
Err = Error::success();
}

Archive::child_iterator Archive::child_begin(Error &Err,
bool SkipInternal) const {
Archive::child_iterator Archive::child_begin(bool SkipInternal) const {
if (Data.getBufferSize() == 8) // empty archive.
return child_end();

if (SkipInternal)
return child_iterator(Child(this, FirstRegularData,
FirstRegularStartOfFile),
&Err);
return Child(this, FirstRegularData, FirstRegularStartOfFile);

const char *Loc = Data.getBufferStart() + strlen(Magic);
std::error_code EC;
Child C(this, Loc, &EC);
if (EC) {
ErrorAsOutParameter ErrAsOutParam(Err);
Err = errorCodeToError(EC);
return child_end();
}
return child_iterator(C, &Err);
Child c(this, Loc, &EC);
if (EC)
return child_iterator(EC);
return child_iterator(c);
}

Archive::child_iterator Archive::child_end() const {
return child_iterator(Child(this, nullptr, nullptr), nullptr);
return Child(this, nullptr, nullptr);
}

StringRef Archive::Symbol::getName() const {
Expand Down Expand Up @@ -667,20 +665,18 @@ uint32_t Archive::getNumberOfSymbols() const {
return read32le(buf);
}

Archive::child_iterator Archive::findSym(Error &Err, StringRef name) const {
Archive::child_iterator Archive::findSym(StringRef name) const {
Archive::symbol_iterator bs = symbol_begin();
Archive::symbol_iterator es = symbol_end();

for (; bs != es; ++bs) {
StringRef SymName = bs->getName();
if (SymName == name) {
if (auto MemberOrErr = bs->getMember()) {
return child_iterator(*MemberOrErr, &Err);
} else {
ErrorAsOutParameter ErrAsOutParam(Err);
Err = errorCodeToError(MemberOrErr.getError());
ErrorOr<Archive::child_iterator> ResultOrErr = bs->getMember();
// FIXME: Should we really eat the error?
if (ResultOrErr.getError())
return child_end();
}
return ResultOrErr.get();
}
}
return child_end();
Expand Down
11 changes: 0 additions & 11 deletions lib/Support/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner) {
});
}


std::error_code ErrorList::convertToErrorCode() const {
return std::error_code(static_cast<int>(ErrorErrorCode::MultipleErrors),
*ErrorErrorCat);
Expand Down Expand Up @@ -100,14 +99,4 @@ std::error_code StringError::convertToErrorCode() const {
return EC;
}

void report_fatal_error(Error Err, bool GenCrashDiag) {
assert(Err && "report_fatal_error called with success value");
std::string ErrMsg;
{
raw_string_ostream ErrStream(ErrMsg);
logAllUnhandledErrors(std::move(Err), ErrStream, "");
}
report_fatal_error(ErrMsg);
}

}
8 changes: 4 additions & 4 deletions tools/dsymutil/BinaryHolder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename,
Buffers.reserve(CurrentArchives.size());

for (const auto &CurrentArchive : CurrentArchives) {
Error Err;
for (auto Child : CurrentArchive->children(Err)) {
for (auto ChildOrErr : CurrentArchive->children()) {
if (std::error_code Err = ChildOrErr.getError())
return Err;
const auto &Child = *ChildOrErr;
if (auto NameOrErr = Child.getName()) {
if (*NameOrErr == Filename) {
if (Timestamp != sys::TimeValue::PosixZeroTime() &&
Expand All @@ -121,8 +123,6 @@ BinaryHolder::GetArchiveMemberBuffers(StringRef Filename,
}
}
}
if (Err)
return errorToErrorCode(std::move(Err));
}

if (Buffers.empty())
Expand Down
68 changes: 32 additions & 36 deletions tools/llvm-ar/llvm-ar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,37 +405,35 @@ static void performReadOperation(ArchiveOperation Operation,
fail("extracting from a thin archive is not supported");

bool Filter = !Members.empty();
{
Error Err;
for (auto &C : OldArchive->children(Err)) {
ErrorOr<StringRef> NameOrErr = C.getName();
failIfError(NameOrErr.getError());
StringRef Name = NameOrErr.get();

if (Filter) {
auto I = std::find(Members.begin(), Members.end(), Name);
if (I == Members.end())
continue;
Members.erase(I);
}
for (auto &ChildOrErr : OldArchive->children()) {
failIfError(ChildOrErr.getError());
const object::Archive::Child &C = *ChildOrErr;

ErrorOr<StringRef> NameOrErr = C.getName();
failIfError(NameOrErr.getError());
StringRef Name = NameOrErr.get();

if (Filter) {
auto I = std::find(Members.begin(), Members.end(), Name);
if (I == Members.end())
continue;
Members.erase(I);
}

switch (Operation) {
default:
llvm_unreachable("Not a read operation");
case Print:
doPrint(Name, C);
break;
case DisplayTable:
doDisplayTable(Name, C);
break;
case Extract:
doExtract(Name, C);
break;
}
switch (Operation) {
default:
llvm_unreachable("Not a read operation");
case Print:
doPrint(Name, C);
break;
case DisplayTable:
doDisplayTable(Name, C);
break;
case Extract:
doExtract(Name, C);
break;
}
failIfError(std::move(Err));
}

if (Members.empty())
return;
for (StringRef Name : Members)
Expand Down Expand Up @@ -533,8 +531,9 @@ computeNewArchiveMembers(ArchiveOperation Operation,
int InsertPos = -1;
StringRef PosName = sys::path::filename(RelPos);
if (OldArchive) {
Error Err;
for (auto &Child : OldArchive->children(Err)) {
for (auto &ChildOrErr : OldArchive->children()) {
failIfError(ChildOrErr.getError());
auto &Child = ChildOrErr.get();
int Pos = Ret.size();
ErrorOr<StringRef> NameOrErr = Child.getName();
failIfError(NameOrErr.getError());
Expand Down Expand Up @@ -569,7 +568,6 @@ computeNewArchiveMembers(ArchiveOperation Operation,
if (MemberI != Members.end())
Members.erase(MemberI);
}
failIfError(std::move(Err));
}

if (Operation == Delete)
Expand Down Expand Up @@ -766,11 +764,9 @@ static void runMRIScript() {
"Could not parse library");
Archives.push_back(std::move(*LibOrErr));
object::Archive &Lib = *Archives.back();
{
Error Err;
for (auto &Member : Lib.children(Err))
addMember(NewMembers, Member);
failIfError(std::move(Err));
for (auto &MemberOrErr : Lib.children()) {
failIfError(MemberOrErr.getError());
addMember(NewMembers, *MemberOrErr);
}
break;
}
Expand Down
Loading

0 comments on commit 5daf897

Please sign in to comment.