Switch functions that returned bool and filled in a DWARFFormValue arg with ones that return Optional<DWARFFormValue>

[clayborg]

Summary

First step in switching the functions to return optionals where it makes sense.

[aprantl]

LGTM (small comments inline).

include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h

82

Could you also update the comment please?

lib/DebugInfo/DWARF/DWARFDie.cpp

138

This looks reasonable, but should probably be a separate NFC commit.

include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h

public:
   /// Extract an attribute value for a DWARFUnit given the DIE offset and the
   /// attribute.
   ///
   /// \param DIEOffset the DIE offset that points to the ULEB128 abbreviation
   /// code in the .debug_info data.
   /// \param Attr DWARF attribute to search for.
   /// \param U the DWARFUnit the contains the DIE.
   /// \param FormValue the form value that will be filled in.
   /// \returns true if the attribute was extracted into \p FormValue.
-  bool getAttributeValue(const uint32_t DIEOffset, const dwarf::Attribute Attr,
-                         const DWARFUnit &U, DWARFFormValue &FormValue) const;
+  Optional<DWARFFormValue> getAttributeValue(const uint32_t DIEOffset,
+                                             const dwarf::Attribute Attr,
+                                             const DWARFUnit &U) const;
+
   bool extract(DataExtractor Data, uint32_t* OffsetPtr);
   void dump(raw_ostream &OS) const;
 
   // Return an optional byte size of all attribute data in this abbreviation
   // if a constant byte size can be calculated given a DWARFUnit. This allows
   // DWARF parsing to be faster as many DWARF DIEs have a fixed byte size.
   Optional<size_t> getFixedAttributesByteSize(const DWARFUnit &U) const;
 

include/llvm/DebugInfo/DWARF/DWARFDie.h

 //===-- DWARFDie.h --------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_DEBUGINFO_DWARFDIE_H
 #define LLVM_LIB_DEBUGINFO_DWARFDIE_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h"
 
 namespace llvm {
     
 class DWARFUnit;
 class DWARFDebugInfoEntry;
 class raw_ostream;
   
public:
   ///
   /// Extract an attribute value from this DIE only. This call doesn't look
   /// for the attribute value in any DW_AT_specification or
   /// DW_AT_abstract_origin referenced DIEs.
   ///
   /// \param Attr the attribute to extract.
   /// \param FormValue contains the attribute value if true is returned.
   /// \returns true if the attribute was extracted from this DIE.
-  bool getAttributeValue(dwarf::Attribute Attr,
-                         DWARFFormValue &FormValue) const;
+  Optional<DWARFFormValue> getAttributeValue(dwarf::Attribute Attr) const;
   
   /// Extract the specified attribute from this DIE as a C string.
   ///
   /// Extract an attribute value from this DIE only. This call doesn't look
   /// for the attribute value in any DW_AT_specification or
   /// DW_AT_abstract_origin referenced DIEs.
   ///
   /// \param Attr the attribute to extract.

lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp

 DWARFAbbreviationDeclaration::findAttributeIndex(dwarf::Attribute Attr) const {
   for (uint32_t i = 0, e = AttributeSpecs.size(); i != e; ++i) {
     if (AttributeSpecs[i].Attr == Attr)
       return i;
   }
   return None;
 }
 
-bool DWARFAbbreviationDeclaration::getAttributeValue(
-    const uint32_t DIEOffset, const dwarf::Attribute Attr, const DWARFUnit &U,
-    DWARFFormValue &FormValue) const {
+Optional<DWARFFormValue> DWARFAbbreviationDeclaration::getAttributeValue(
+    const uint32_t DIEOffset, const dwarf::Attribute Attr,
+    const DWARFUnit &U) const {
   Optional<uint32_t> MatchAttrIndex = findAttributeIndex(Attr);
   if (!MatchAttrIndex)
-    return false;
+    return None;
 
   auto DebugInfoData = U.getDebugInfoExtractor();
 
   // Add the byte size of ULEB that for the abbrev Code so we can start
   // skipping the attribute data.
   uint32_t Offset = DIEOffset + CodeByteSize;
   uint32_t AttrIndex = 0;
   for (const auto &Spec : AttributeSpecs) {
     if (*MatchAttrIndex == AttrIndex) {
       // We have arrived at the attribute to extract, extract if from Offset.
-      FormValue.setForm(Spec.Form);
-      return FormValue.extractValue(DebugInfoData, &Offset, &U);
+      DWARFFormValue FormValue(Spec.Form);
+      if (FormValue.extractValue(DebugInfoData, &Offset, &U))
+        return FormValue;
     }
     // March Offset along until we get to the attribute we want.
     if (Optional<uint8_t> FixedSize = Spec.getByteSize(U))
       Offset += *FixedSize;
     else
       DWARFFormValue::skipValue(Spec.Form, DebugInfoData, &Offset, &U);
     ++AttrIndex;
   }
-  return false;
+  return None;
 }
 
 size_t DWARFAbbreviationDeclaration::FixedSizeInfo::getByteSize(
     const DWARFUnit &U) const {
   size_t ByteSize = NumBytes;
   if (NumAddrs)
     ByteSize += NumAddrs * U.getAddressByteSize();
   if (NumRefAddrs)

lib/DebugInfo/DWARF/DWARFDie.cpp

bool DWARFDie::isSubprogramDIE() const {
   return getTag() == DW_TAG_subprogram;
 }
 
 bool DWARFDie::isSubroutineDIE() const {
   auto Tag = getTag();
   return Tag == DW_TAG_subprogram || Tag == DW_TAG_inlined_subroutine;
 }
 
-bool DWARFDie::getAttributeValue(dwarf::Attribute Attr,
-                                 DWARFFormValue &FormValue) const {
-  if (!U)
-    return false;
+Optional<DWARFFormValue>
+DWARFDie::getAttributeValue(dwarf::Attribute Attr) const {
+  if (!isValid())
+    return None;
   auto AbbrevDecl = getAbbreviationDeclarationPtr();
   if (AbbrevDecl)
-    return AbbrevDecl->getAttributeValue(getOffset(), Attr, *U, FormValue);
-  return false;
+    return AbbrevDecl->getAttributeValue(getOffset(), Attr, *U);
+  return None;
 }
 
 const char *DWARFDie::getAttributeValueAsString(dwarf::Attribute Attr,
                                                 const char *FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<const char *> Result = FormValue.getAsCString();
+  Optional<const char *> Result = FormValue->getAsCString();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 uint64_t DWARFDie::getAttributeValueAsAddress(dwarf::Attribute Attr,
                                               uint64_t FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<uint64_t> Result = FormValue.getAsAddress();
+  Optional<uint64_t> Result = FormValue->getAsAddress();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 int64_t DWARFDie::getAttributeValueAsSignedConstant(dwarf::Attribute Attr,
                                                     int64_t FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<int64_t> Result = FormValue.getAsSignedConstant();
+  Optional<int64_t> Result = FormValue->getAsSignedConstant();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 uint64_t
 DWARFDie::getAttributeValueAsUnsignedConstant(dwarf::Attribute Attr,
                                               uint64_t FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<uint64_t> Result = FormValue.getAsUnsignedConstant();
+  Optional<uint64_t> Result = FormValue->getAsUnsignedConstant();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 uint64_t DWARFDie::getAttributeValueAsReference(dwarf::Attribute Attr,
                                                 uint64_t FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<uint64_t> Result = FormValue.getAsReference();
+  Optional<uint64_t> Result = FormValue->getAsReference();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 uint64_t DWARFDie::getAttributeValueAsSectionOffset(dwarf::Attribute Attr,
                                                     uint64_t FailValue) const {
-  DWARFFormValue FormValue;
-  if (!getAttributeValue(Attr, FormValue))
+  auto FormValue = getAttributeValue(Attr);
+  if (!FormValue)
     return FailValue;
-  Optional<uint64_t> Result = FormValue.getAsSectionOffset();
+  Optional<uint64_t> Result = FormValue->getAsSectionOffset();
   return Result.hasValue() ? Result.getValue() : FailValue;
 }
 
 DWARFDie
 DWARFDie::getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const {
   uint32_t SpecRef = getAttributeValueAsReference(Attr, -1U);
   if (SpecRef != -1U) {
     auto SpecUnit = U->getUnitSection().getUnitForOffset(SpecRef);

lib/DebugInfo/DWARF/DWARFTypeUnit.cpp

if (!DWARFUnit::extractImpl(debug_info, offset_ptr))
     return false;
   TypeHash = debug_info.getU64(offset_ptr);
   TypeOffset = debug_info.getU32(offset_ptr);
   return TypeOffset < getLength();
 }
 
 void DWARFTypeUnit::dump(raw_ostream &OS, bool SummarizeTypes) {
   DWARFDie TD = getDIEForOffset(TypeOffset + getOffset());
-  DWARFFormValue NameVal;
-  const char *Name = "";
-  if (TD.getAttributeValue(llvm::dwarf::DW_AT_name, NameVal))
-    if (auto ON = NameVal.getAsCString())
-      Name = *ON;
+  const char *Name = TD.getAttributeValueAsString(llvm::dwarf::DW_AT_name, "");
 
   if (SummarizeTypes) {
     OS << "name = '" << Name << "'"
        << " type_signature = " << format("0x%16" PRIx64, TypeHash)
        << " length = " << format("0x%08x", getLength()) << '\n';
     return;
   }
 

tools/dsymutil/DwarfLinker.cpp

if (LowPc == -1ULL ||
       !RelocMgr.hasValidRelocation(LowPcOffset, LowPcEndOffset, MyInfo))
     return Flags;
 
   if (Options.Verbose)
     DIE.dump(outs(), 0, 8 /* Indent */);
 
   Flags |= TF_Keep;
 
-  DWARFFormValue HighPcValue;
-  if (!DIE.getAttributeValue(dwarf::DW_AT_high_pc, HighPcValue)) {
+  Optional<DWARFFormValue> HighPcValue;
+  if (!(HighPcValue = DIE.getAttributeValue(dwarf::DW_AT_high_pc))) {
     reportWarning("Function without high_pc. Range will be discarded.\n",
                   &DIE);
     return Flags;
   }
 
   uint64_t HighPc;
-  if (HighPcValue.isFormClass(DWARFFormValue::FC_Address)) {
-    HighPc = *HighPcValue.getAsAddress();
+  if (HighPcValue->isFormClass(DWARFFormValue::FC_Address)) {
+    HighPc = *HighPcValue->getAsAddress();
   } else {
-    assert(HighPcValue.isFormClass(DWARFFormValue::FC_Constant));
-    HighPc = LowPc + *HighPcValue.getAsUnsignedConstant();
+    assert(HighPcValue->isFormClass(DWARFFormValue::FC_Constant));
+    HighPc = LowPc + *HighPcValue->getAsUnsignedConstant();
   }
 
   // Replace the debug map range with a more accurate one.
   Ranges[LowPc] = std::make_pair(HighPc, MyInfo.AddrAdjust);
   Unit.addFunctionRange(LowPc, HighPc, MyInfo.AddrAdjust);
   return Flags;
 }
 

unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

#define UDATA_1 18446744073709551614ULL
   // Test address forms
   //----------------------------------------------------------------------
   EXPECT_EQ(DieDG.getAttributeValueAsAddress(Attr_DW_FORM_addr, 0),
             AddrValue);
 
   //----------------------------------------------------------------------
   // Test block forms
   //----------------------------------------------------------------------
-  DWARFFormValue FormValue;
+  Optional<DWARFFormValue> FormValue;
   ArrayRef<uint8_t> ExtractedBlockData;
   Optional<ArrayRef<uint8_t>> BlockDataOpt;
 
-  EXPECT_TRUE(DieDG.getAttributeValue(Attr_DW_FORM_block, FormValue));
-  BlockDataOpt = FormValue.getAsBlock();
+  FormValue = DieDG.getAttributeValue(Attr_DW_FORM_block);
+  EXPECT_TRUE((bool)FormValue);
+  BlockDataOpt = FormValue->getAsBlock();
   EXPECT_TRUE(BlockDataOpt.hasValue());
   ExtractedBlockData = BlockDataOpt.getValue();
   EXPECT_EQ(ExtractedBlockData.size(), BlockSize);
   EXPECT_TRUE(memcmp(ExtractedBlockData.data(), BlockData, BlockSize) == 0);
 
-  EXPECT_TRUE(DieDG.getAttributeValue(Attr_DW_FORM_block1, FormValue));
-  BlockDataOpt = FormValue.getAsBlock();
+  FormValue = DieDG.getAttributeValue(Attr_DW_FORM_block1);
+  EXPECT_TRUE((bool)FormValue);
+  BlockDataOpt = FormValue->getAsBlock();
   EXPECT_TRUE(BlockDataOpt.hasValue());
   ExtractedBlockData = BlockDataOpt.getValue();
   EXPECT_EQ(ExtractedBlockData.size(), BlockSize);
   EXPECT_TRUE(memcmp(ExtractedBlockData.data(), BlockData, BlockSize) == 0);
 
-  EXPECT_TRUE(DieDG.getAttributeValue(Attr_DW_FORM_block2, FormValue));
-  BlockDataOpt = FormValue.getAsBlock();
+  FormValue = DieDG.getAttributeValue(Attr_DW_FORM_block2);
+  EXPECT_TRUE((bool)FormValue);
+  BlockDataOpt = FormValue->getAsBlock();
   EXPECT_TRUE(BlockDataOpt.hasValue());
   ExtractedBlockData = BlockDataOpt.getValue();
   EXPECT_EQ(ExtractedBlockData.size(), BlockSize);
   EXPECT_TRUE(memcmp(ExtractedBlockData.data(), BlockData, BlockSize) == 0);
 
-  EXPECT_TRUE(DieDG.getAttributeValue(Attr_DW_FORM_block4, FormValue));
-  BlockDataOpt = FormValue.getAsBlock();
+  FormValue = DieDG.getAttributeValue(Attr_DW_FORM_block4);
+  EXPECT_TRUE((bool)FormValue);
+  BlockDataOpt = FormValue->getAsBlock();
   EXPECT_TRUE(BlockDataOpt.hasValue());
   ExtractedBlockData = BlockDataOpt.getValue();
   EXPECT_EQ(ExtractedBlockData.size(), BlockSize);
   EXPECT_TRUE(memcmp(ExtractedBlockData.data(), BlockData, BlockSize) == 0);
 
   //----------------------------------------------------------------------
   // Test data forms
   //----------------------------------------------------------------------