DebugInfo: Make DWARFExpression::iterator a const iterator

[dexonsmith]

Summary

3d1d8c767be5537eb5510ee0522e2f3475fe7c04 made
DWARFExpression::iterator's Operation member mutable. After a few prep
commits, the iterator can instead be made a const iterator since no
caller can change the Operation.

Depends on https://reviews.llvm.org/D113957.

[dexonsmith]

[No change; just re-triggering builds after telling Phabricator this depends on the other patch.]

[jhenderson]

Clang-format nit, as per linter. Otherwise, seems reasonable to me (if it can be const it probably should be!). My only question is should it be renamed to const_iterator to avoid any confusion?

[dblaikie]

Looks good to me - I don't mind it still being called iterator rather than const_iterator - if it's the only iterator over the range anyway. (ArrayRef's iterator isn't called const_iterator for instance, even though it is only constant)

llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h

132–133

Side note: the class keyword here could be removed.

[dexonsmith]

Thanks for the reviews! Pushed in fd6018072ace7d5cdf537fd63a44c050a98e52fc.

In D113958#3133946, @jhenderson wrote:

Clang-format nit, as per linter.

Fixed!

My only question is should it be renamed to const_iterator to avoid any confusion?

Tend to agree with @dblaikie that iterator is clear/simple so I left this as is.

llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h

132–133

Fixed.

llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h

public:
   private:
     bool extract(DataExtractor Data, uint8_t AddressSize, uint64_t Offset,
                  Optional<dwarf::DwarfFormat> Format);
   };
 
   /// An iterator to go through the expression operations.
   class iterator
       : public iterator_facade_base<iterator, std::forward_iterator_tag,
-                                    Operation> {
+                                    const Operation> {
     friend class DWARFExpression;
     const DWARFExpression *Expr;
     uint64_t Offset;
-    mutable Operation Op;
+    Operation Op;
     iterator(const DWARFExpression *Expr, uint64_t Offset)
         : Expr(Expr), Offset(Offset) {
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
           !Op.extract(Expr->Data, Expr->AddressSize, Offset, Expr->Format);
     }
 
   public:
     iterator &operator++() {
       Offset = Op.isError() ? Expr->Data.getData().size() : Op.EndOffset;
       Op.Error =
           Offset >= Expr->Data.getData().size() ||
           !Op.extract(Expr->Data, Expr->AddressSize, Offset, Expr->Format);
       return *this;
     }
 
-    class Operation &operator*() const {
-      return Op;
-    }
+    const Operation &operator*() const { return Op; }
 
     iterator skipBytes(uint64_t Add) const {
       return iterator(Expr, Op.EndOffset + Add);
     }
 
     // Comparison operators are provided out of line.
     friend bool operator==(const iterator &, const iterator &);
   };

llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp

 };
 
 static bool printCompactDWARFExpr(raw_ostream &OS, DWARFExpression::iterator I,
                                   const DWARFExpression::iterator E,
                                   const MCRegisterInfo &MRI) {
   SmallVector<PrintedExpr, 4> Stack;
 
   while (I != E) {
-    DWARFExpression::Operation &Op = *I;
+    const DWARFExpression::Operation &Op = *I;
     uint8_t Opcode = Op.getCode();
     switch (Opcode) {
     case dwarf::DW_OP_regx: {
       // DW_OP_regx: A register, with the register num given as an operand.
       // Printed as the plain register name.
       uint64_t DwarfRegNum = Op.getRawOperand(0);
       Optional<unsigned> LLVMRegNum = MRI.getLLVMRegNum(DwarfRegNum, false);
       if (!LLVMRegNum) {

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die,
   case DW_AT_location: {
     if (Expected<std::vector<DWARFLocationExpression>> Loc =
             Die.getLocations(DW_AT_location)) {
       DWARFUnit *U = Die.getDwarfUnit();
       for (const auto &Entry : *Loc) {
         DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(), 0);
         DWARFExpression Expression(Data, U->getAddressByteSize(),
                                    U->getFormParams().Format);
-        bool Error = any_of(Expression, [](DWARFExpression::Operation &Op) {
+        bool Error =
+            any_of(Expression, [](const DWARFExpression::Operation &Op) {
               return Op.isError();
             });
         if (Error || !Expression.verify(U))
           ReportError("DIE contains invalid DWARF expression:");
       }
     } else
       ReportError(toString(Loc.takeError()));
     break;
   }
   case DW_AT_specification:
if (!Loc) {
     return false;
   }
   DWARFUnit *U = Die.getDwarfUnit();
   for (const auto &Entry : *Loc) {
     DataExtractor Data(toStringRef(Entry.Expr), DCtx.isLittleEndian(),
                        U->getAddressByteSize());
     DWARFExpression Expression(Data, U->getAddressByteSize(),
                                U->getFormParams().Format);
-    bool IsInteresting = any_of(Expression, [](DWARFExpression::Operation &Op) {
+    bool IsInteresting =
+        any_of(Expression, [](const DWARFExpression::Operation &Op) {
           return !Op.isError() && (Op.getCode() == DW_OP_addr ||
                                    Op.getCode() == DW_OP_form_tls_address ||
                                    Op.getCode() == DW_OP_GNU_push_tls_address);
         });
     if (IsInteresting)
       return true;
   }
   return false;
 }
 
 unsigned DWARFVerifier::verifyNameIndexCompleteness(
     const DWARFDie &Die, const DWARFDebugNames::NameIndex &NI) {

llvm/tools/llvm-dwarfdump/Statistics.cpp

static void collectStatsForDie(DWARFDie Die, const std::string &FnPrefix,
   auto IsEntryValue = [&](ArrayRef<uint8_t> D) -> bool {
     DWARFUnit *U = Die.getDwarfUnit();
     DataExtractor Data(toStringRef(D),
                        Die.getDwarfUnit()->getContext().isLittleEndian(), 0);
     DWARFExpression Expression(Data, U->getAddressByteSize(),
                                U->getFormParams().Format);
     // Consider the expression containing the DW_OP_entry_value as
     // an entry value.
-    return llvm::any_of(Expression, [](DWARFExpression::Operation &Op) {
+    return llvm::any_of(Expression, [](const DWARFExpression::Operation &Op) {
       return Op.getCode() == dwarf::DW_OP_entry_value ||
              Op.getCode() == dwarf::DW_OP_GNU_entry_value;
     });
   };
 
   if (Die.find(dwarf::DW_AT_const_value)) {
     // This catches constant members *and* variables.
     HasLoc = true;