[PATCH] D66089: [llvm/Object] - Convert SectionRef::getName() to return Expected<>

[Sam Clegg via Phabricator via llvm-commits]

sbc100 accepted this revision.
sbc100 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/llvm/Object/ObjectFile.h:438
+inline Expected<StringRef> SectionRef::getName() const {
   Expected<StringRef> NameOrErr = OwningObject->getSectionName(SectionPimpl);
   if (!NameOrErr)
----------------
Can you return the result of `getSectionName` here?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:1512
+      else
+        consumeError(NameOrErr.takeError());
+
----------------
Add TODOs here and below to handle errors?


================
Comment at: lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFX86_64.h:287
       const object::SectionRef &Section = SectionPair.first;
-      StringRef Name;
-      if (auto EC = Section.getName(Name))
-        return errorCodeToError(EC);
+      Expected<StringRef> ErrorOrName = Section.getName();
+      if (!ErrorOrName)
----------------
You use `ErrorOrName` here but `NameOrErr` here.  Presumably we should prefer one or at least be consistent?


================
Comment at: lib/Object/ELFObjectFile.cpp:386
   for (const SectionRef &Section : sections()) {
-    StringRef Name;
-    if (Section.getName(Name))
+    auto NameOrErr = Section.getName();
+    if (!NameOrErr) {
----------------
In most other places you use `Expected<StringRef>` rather than auto.   Is there a reason for auto here and not elsewhere?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66089/new/

https://reviews.llvm.org/D66089