[llvm] adapt DWARFExpression.h for 6b9b86db9dd974585a5c71cf2e5231d1e3385f7c

[krasimir]

Summary

No functional changes intended.

Updated the iterator class for https://github.com/llvm/llvm-project/commit/6b9b86db9dd974585a5c71cf2e5231d1e3385f7c.

[dexonsmith]

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

106

Should this be const Operation?

110

Do callers really want/need to modify the Operation?

127

Wondering if this should be const class Operation &operator*.

[dexonsmith]

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

124

Huh... this is a bit strange. Usually ++ returns iterator&...

[krasimir]

Sorry for submitting quickly, I just wanted to unblock build bots. I still think that this patch is OK considering the weirdness of this Operation class.

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

110

I think yes,

110

I believe yes: none of the public member functions is const
https://github.com/llvm/llvm-project/blob/0b17336f793108a7b10c3fa913039144ef1d0f61/llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h#L89.
Some of the methods of Operation can be made const, but ultimately it's pretty mutable. For example, Operation::verify() sets Error (https://github.com/llvm/llvm-project/blob/fcd07f810781c1754c2e7e9641fd18b95fa1368c/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp#L376) and is used in a loop in DWARFExpression::verify: https://github.com/llvm/llvm-project/blob/fcd07f810781c1754c2e7e9641fd18b95fa1368c/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp#L387.

127

ditto, the problem is that all public members are non-const, so even checking for stuff needs non-const ref.

[dexonsmith]

I pushed fa39ec5786c08e78cc247a48c17aef66ca5985fb, a35226b9b606f4ea32e7db9e2f4de0d99ab6bef0, and 7a1b670f0f2f0a38a673b5554bb2c4d9d50447bf to fix most of the blockers. Then https://reviews.llvm.org/D113957 fixes Operation::verify() and https://reviews.llvm.org/D113958 makes it iterate over const Operation and removes the mutable.

In D113934#3132954, @krasimir wrote:

Sorry for submitting quickly, I just wanted to unblock build bots. I still think that this patch is OK considering the weirdness of this Operation class.

No worries; makes sense.

[krasimir]

In D113934#3133493, @dexonsmith wrote:

I pushed fa39ec5786c08e78cc247a48c17aef66ca5985fb, a35226b9b606f4ea32e7db9e2f4de0d99ab6bef0, and 7a1b670f0f2f0a38a673b5554bb2c4d9d50447bf to fix most of the blockers. Then https://reviews.llvm.org/D113957 fixes Operation::verify() and https://reviews.llvm.org/D113958 makes it iterate over const Operation and removes the mutable.

Thank you! These look good!

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

bool print(raw_ostream &OS, DIDumpOptions DumpOpts,
                const DWARFExpression *Expr, const MCRegisterInfo *RegInfo,
                DWARFUnit *U, bool isEH);
     bool verify(DWARFUnit *U);
   };
 
   /// An iterator to go through the expression operations.
   class iterator
       : public iterator_facade_base<iterator, std::forward_iterator_tag,
                                     Operation> {
     friend class DWARFExpression;
     const DWARFExpression *Expr;
     uint64_t Offset;
-    Operation Op;
+    mutable 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:
     class Operation &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 Op;
     }
 
-    class Operation &operator*() {
+    class Operation &operator*() const {
       return Op;
     }
 
     iterator skipBytes(uint64_t Add) {
       return iterator(Expr, Op.EndOffset + Add);
     }
 
     // Comparison operators are provided out of line.