[llvm] r289611 - Switch functions that returned bool and filled in a DWARFFormValue arg with ones that return Optional<DWARFFormValue>

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 15:40:13 PST 2016


Looks like you missed my inline comments in the review?
> On Dec 13, 2016, at 3:20 PM, Greg Clayton via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: gclayton
> Date: Tue Dec 13 17:20:56 2016
> New Revision: 289611
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=289611&view=rev
> Log:
> Switch functions that returned bool and filled in a DWARFFormValue arg with ones that return Optional<DWARFFormValue>
> 
> Differential Revision: https://reviews.llvm.org/D27737
> 
> 
> Modified:
>    llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h
>    llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDie.h
>    llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
>    llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
>    llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp
>    llvm/trunk/tools/dsymutil/DwarfLinker.cpp
>    llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
> 
> Modified: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h (original)
> +++ llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h Tue Dec 13 17:20:56 2016
> @@ -80,8 +80,10 @@ public:
>   /// \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.

Comment needs to be updated, too.

-- adrian

> -  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;
> 
> 
> Modified: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDie.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDie.h?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDie.h (original)
> +++ llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFDie.h Tue Dec 13 17:20:56 2016
> @@ -10,6 +10,7 @@
> #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 {
> @@ -117,8 +118,7 @@ public:
>   /// \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.
>   ///
> 
> Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp Tue Dec 13 17:20:56 2016
> @@ -143,12 +143,12 @@ DWARFAbbreviationDeclaration::findAttrib
>   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();
> 
> @@ -159,8 +159,9 @@ bool DWARFAbbreviationDeclaration::getAt
>   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))
> @@ -169,7 +170,7 @@ bool DWARFAbbreviationDeclaration::getAt
>       DWARFFormValue::skipValue(Spec.Form, DebugInfoData, &Offset, &U);
>     ++AttrIndex;
>   }
> -  return false;
> +  return None;
> }
> 
> size_t DWARFAbbreviationDeclaration::FixedSizeInfo::getByteSize(
> 
> Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp Tue Dec 13 17:20:56 2016
> @@ -133,68 +133,68 @@ bool DWARFDie::isSubroutineDIE() const {
>   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;
> }
> 
> 
> Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp Tue Dec 13 17:20:56 2016
> @@ -26,11 +26,7 @@ bool DWARFTypeUnit::extractImpl(DataExtr
> 
> 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 << "'"
> 
> Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
> +++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Tue Dec 13 17:20:56 2016
> @@ -2136,19 +2136,19 @@ unsigned DwarfLinker::shouldKeepSubprogr
> 
>   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.
> 
> Modified: llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp?rev=289611&r1=289610&r2=289611&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp (original)
> +++ llvm/trunk/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp Tue Dec 13 17:20:56 2016
> @@ -227,33 +227,37 @@ void TestAllForms() {
>   //----------------------------------------------------------------------
>   // 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);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list