[TextAPI][elfabi] Make TBE handlers functions that return Errors

[amontanez]

Summary

Since TBEHandler doesn't maintain state or otherwise have any need to be a class right now, the read and write functions have been moved out and turned into standalone functions. Additionally, the TBE read function has been updated to return an Expected value for better error handling. Tests have been updated to reflect these changes.

[jakehehrlich]

Looks like a simple mechanical transformation. LGTM.

llvm/trunk/include/llvm/TextAPI/ELF/TBEHandler.h

 /// ELF) files.
 ///
 //===-----------------------------------------------------------------------===/
 
 #ifndef LLVM_TEXTAPI_ELF_TBEHANDLER_H
 #define LLVM_TEXTAPI_ELF_TBEHANDLER_H
 
 #include "llvm/Support/VersionTuple.h"
+#include "llvm/Support/Error.h"
 #include <memory>
 
 namespace llvm {
 
 class raw_ostream;
 class Error;
 class StringRef;
 class VersionTuple;
 
 namespace elfabi {
 
 class ELFStub;
 
 const VersionTuple TBEVersionCurrent(1, 0);
 
-class TBEHandler {
-public:
-  /// Attempts to read an ELF interface file from a StringRef buffer.
-  std::unique_ptr<ELFStub> readFile(StringRef Buf);
+Expected<std::unique_ptr<ELFStub>> readTBEFromBuffer(StringRef Buf);
 
-  /// Attempts to write an ELF interface file to a raw_ostream.
-  Error writeFile(raw_ostream &OS, const ELFStub &Stub);
-};
+Error writeTBEToOutputStream(raw_ostream &OS, const ELFStub &Stub);
+
 } // end namespace elfabi
 } // end namespace llvm
 
 #endif // LLVM_TEXTAPI_ELF_TBEHANDLER_H

llvm/trunk/lib/TextAPI/ELF/TBEHandler.cpp

static void mapping(IO &IO, ELFStub &Stub) {
     IO.mapOptional("NeededLibs", Stub.NeededLibs);
     IO.mapRequired("Symbols", Stub.Symbols);
   }
 };
 
 } // end namespace yaml
 } // end namespace llvm
 
-std::unique_ptr<ELFStub> TBEHandler::readFile(StringRef Buf) {
+Expected<std::unique_ptr<ELFStub>> elfabi::readTBEFromBuffer(StringRef Buf) {
   yaml::Input YamlIn(Buf);
   std::unique_ptr<ELFStub> Stub(new ELFStub());
   YamlIn >> *Stub;
-  if (YamlIn.error())
-    return nullptr;
+  if (std::error_code Err = YamlIn.error())
+    return createStringError(Err, "YAML failed reading as TBE");
+
   return Stub;
 }
 
-Error TBEHandler::writeFile(raw_ostream &OS, const ELFStub &Stub) {
+Error elfabi::writeTBEToOutputStream(raw_ostream &OS, const ELFStub &Stub) {
   yaml::Output YamlOut(OS, NULL, /*WrapColumn =*/0);
 
   YamlOut << const_cast<ELFStub &>(Stub);
   return Error::success();
 }

llvm/trunk/unittests/TextAPI/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
+  TestingSupport
   TextAPI
 )
 
 add_llvm_unittest(TapiTests
   ELFYAMLTest.cpp
 )

llvm/trunk/unittests/TextAPI/ELFYAMLTest.cpp

 //===- llvm/unittests/TextAPI/YAMLTest.cpp --------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===-----------------------------------------------------------------------===/
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/TextAPI/ELF/ELFStub.h"
 #include "llvm/TextAPI/ELF/TBEHandler.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <string>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::elfabi;
 
-std::unique_ptr<ELFStub> readFromBuffer(const char Data[]) {
-  TBEHandler Handler;
-
-  StringRef Buf(Data);
-
-  std::unique_ptr<ELFStub> Stub = Handler.readFile(Buf);
-  EXPECT_NE(Stub.get(), nullptr);
-  return Stub;
-}
-
 void compareByLine(StringRef LHS, StringRef RHS) {
   StringRef Line1;
   StringRef Line2;
   while (LHS.size() > 0 && RHS.size() > 0) {
     std::tie(Line1, LHS) = LHS.split('\n');
     std::tie(Line2, RHS) = RHS.split('\n');
     // Comparing StringRef objects works, but has messy output when not equal.
     // Using STREQ on StringRef.data() doesn't work since these substrings are
     // not null terminated.
     // This is inefficient, but forces null terminated strings that can be
     // cleanly compared.
     EXPECT_STREQ(Line1.str().data(), Line2.str().data());
   }
 }
 
 TEST(ElfYamlTextAPI, YAMLReadableTBE) {
   const char Data[] = "--- !tapi-tbe\n"
                       "TbeVersion: 1.0\n"
                       "SoName: test.so\n"
                       "Arch: x86_64\n"
                       "NeededLibs: [libc.so, libfoo.so, libbar.so]\n"
                       "Symbols:\n"
                       "  foo: { Type: Func, Undefined: true }\n"
                       "...\n";
-  StringRef Buf = StringRef(Data);
-  TBEHandler Handler;
-  std::unique_ptr<ELFStub> Stub = Handler.readFile(Buf);
+  Expected<std::unique_ptr<ELFStub>> StubOrErr = readTBEFromBuffer(Data);
+  ASSERT_THAT_ERROR(StubOrErr.takeError(), Succeeded());
+  std::unique_ptr<ELFStub> Stub = std::move(StubOrErr.get());
   EXPECT_NE(Stub.get(), nullptr);
   EXPECT_EQ(Stub->Arch, (uint16_t)llvm::ELF::EM_X86_64);
   EXPECT_STREQ(Stub->SoName.c_str(), "test.so");
   EXPECT_EQ(Stub->NeededLibs.size(), 3u);
   EXPECT_STREQ(Stub->NeededLibs[0].c_str(), "libc.so");
   EXPECT_STREQ(Stub->NeededLibs[1].c_str(), "libfoo.so");
   EXPECT_STREQ(Stub->NeededLibs[2].c_str(), "libbar.so");
 }
 
 TEST(ElfYamlTextAPI, YAMLReadsTBESymbols) {
   const char Data[] = "--- !tapi-tbe\n"
                       "TbeVersion: 1.0\n"
                       "SoName: test.so\n"
                       "Arch: x86_64\n"
                       "Symbols:\n"
                       "  bar: { Type: Object, Size: 42 }\n"
                       "  baz: { Type: TLS, Size: 3 }\n"
                       "  foo: { Type: Func, Warning: \"Deprecated!\" }\n"
                       "  nor: { Type: NoType, Undefined: true }\n"
                       "  not: { Type: File, Undefined: true, Size: 111, "
                       "Warning: \'All fields populated!\' }\n"
                       "...\n";
-  std::unique_ptr<ELFStub> Stub = readFromBuffer(Data);
+  Expected<std::unique_ptr<ELFStub>> StubOrErr = readTBEFromBuffer(Data);
+  ASSERT_THAT_ERROR(StubOrErr.takeError(), Succeeded());
+  std::unique_ptr<ELFStub> Stub = std::move(StubOrErr.get());
+  EXPECT_NE(Stub.get(), nullptr);
   EXPECT_EQ(Stub->Symbols.size(), 5u);
 
   auto Iterator = Stub->Symbols.begin();
   ELFSymbol const &SymBar = *Iterator++;
   EXPECT_STREQ(SymBar.Name.c_str(), "bar");
   EXPECT_EQ(SymBar.Size, 42u);
   EXPECT_EQ(SymBar.Type, ELFSymbolType::Object);
   EXPECT_FALSE(SymBar.Undefined);

 
 TEST(ElfYamlTextAPI, YAMLReadsNoTBESyms) {
   const char Data[] = "--- !tapi-tbe\n"
                       "TbeVersion: 1.0\n"
                       "SoName: test.so\n"
                       "Arch: x86_64\n"
                       "Symbols: {}\n"
                       "...\n";
-  std::unique_ptr<ELFStub> Stub = readFromBuffer(Data);
+  Expected<std::unique_ptr<ELFStub>> StubOrErr = readTBEFromBuffer(Data);
+  ASSERT_THAT_ERROR(StubOrErr.takeError(), Succeeded());
+  std::unique_ptr<ELFStub> Stub = std::move(StubOrErr.get());
+  EXPECT_NE(Stub.get(), nullptr);
   EXPECT_EQ(0u, Stub->Symbols.size());
 }
 
 TEST(ElfYamlTextAPI, YAMLUnreadableTBE) {
-  TBEHandler Handler;
   // Can't read: wrong format/version.
   const char Data[] = "--- !tapi-tbz\n"
                       "TbeVersion: z.3\n"
                       "SoName: test.so\n"
                       "Arch: x86_64\n"
                       "Symbols:\n"
                       "  foo: { Type: Func, Undefined: true }\n";
-  StringRef Buf = StringRef(Data);
-  std::unique_ptr<ELFStub> Stub = Handler.readFile(Buf);
-  EXPECT_EQ(Stub.get(), nullptr);
+  Expected<std::unique_ptr<ELFStub>> StubOrErr = readTBEFromBuffer(Data);
+  ASSERT_THAT_ERROR(StubOrErr.takeError(), Failed());
 }
 
 TEST(ElfYamlTextAPI, YAMLWritesTBESymbols) {
   const char Expected[] =
       "--- !tapi-tbe\n"
       "TbeVersion:      1.0\n"
       "SoName:          test.so\n"
       "Arch:            AArch64\n"
TEST(ElfYamlTextAPI, YAMLWritesTBESymbols) {
   Stub.Symbols.insert(SymFoo);
   Stub.Symbols.insert(SymNor);
 
   // Ensure move constructor works as expected.
   ELFStub Moved = std::move(Stub);
 
   std::string Result;
   raw_string_ostream OS(Result);
-  TBEHandler Handler;
-  EXPECT_FALSE(Handler.writeFile(OS, Moved));
+  ASSERT_THAT_ERROR(writeTBEToOutputStream(OS, Moved), Succeeded());
   Result = OS.str();
   compareByLine(Result.c_str(), Expected);
 }
 
 TEST(ElfYamlTextAPI, YAMLWritesNoTBESyms) {
   const char Expected[] = "--- !tapi-tbe\n"
                           "TbeVersion:      1.0\n"
                           "SoName:          nosyms.so\n"
TEST(ElfYamlTextAPI, YAMLWritesNoTBESyms) {
   Stub.SoName = "nosyms.so";
   Stub.Arch = ELF::EM_X86_64;
   Stub.NeededLibs.push_back("libc.so");
   Stub.NeededLibs.push_back("libfoo.so");
   Stub.NeededLibs.push_back("libbar.so");
 
   std::string Result;
   raw_string_ostream OS(Result);
-  TBEHandler Handler;
-  EXPECT_FALSE(Handler.writeFile(OS, Stub));
+  ASSERT_THAT_ERROR(writeTBEToOutputStream(OS, Stub), Succeeded());
   Result = OS.str();
   compareByLine(Result.c_str(), Expected);
 }