[llvm] r226939 - Move the accessor functions from DIExpression::iterator into a wrapper

[Duncan P. N. Exon Smith]

> On 2015-Jan-23, at 14:32, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Jan 23, 2015 at 1:24 PM, Adrian Prantl <aprantl at apple.com> wrote:
> Author: adrian
> Date: Fri Jan 23 15:24:41 2015
> New Revision: 226939
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=226939&view=rev
> Log:
> Move the accessor functions  from DIExpression::iterator into a wrapper
> DIExpression::Operand, so we can write range-based for loops.
> 
> Thanks to David Blaikie for the idea.
> 
> Modified:
>     llvm/trunk/include/llvm/IR/DebugInfo.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
>     llvm/trunk/lib/IR/DebugInfo.cpp
> 
> Modified: llvm/trunk/include/llvm/IR/DebugInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=226939&r1=226938&r2=226939&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
> +++ llvm/trunk/include/llvm/IR/DebugInfo.h Fri Jan 23 15:24:41 2015
> @@ -867,16 +867,17 @@ public:
>    /// \brief Return the size of this piece in bytes.
>    uint64_t getPieceSize() const;
> 
> +  class Operand;
> +
>    /// \brief An iterator for DIExpression elements.
>    class iterator : public std::iterator<std::forward_iterator_tag, StringRef,
>                                          unsigned, const uint64_t *, uint64_t> {
> 
> I'm guessing these types passed to std::iterator need to be updated now that the value is an Operand, not a uint64_t.
>  

Also, this should be a input_iterator, shouldn't it?

>      DIHeaderFieldIterator I;
> 
> Really vaguely curious - should the expression be encoded as a raw byte sequence, rather than bytes in strings in a DIHeader? (bytes in strings seems inefficient?)
>  

I think this is fine as is as a stop-gap.  When my work is finished
(I'm stalling because I'm having trouble figuring out how to stage
everything), DIExpression won't be backed by a string: it'll store an
array of `uint64_t` directly.

>      iterator(DIHeaderFieldIterator I) : I(I) {}
> -
>    public:
>      iterator() {}
>      iterator(const DIExpression &Expr) : I(++Expr.header_begin()) {}
> -    uint64_t operator*() const { return I.getNumber<uint64_t>(); }
> +    Operand operator*() const { return Operand(I); }
> 
> I /think/ you might need to return by reference than by value here (& have an Operand value as a member of the iterator) - I think that the iterator requirements require a reference to be returned or some other feature that makes returning by value not viable - but I could totally be wrong here, my memory is very vague.

Ah, right.  This is what I was thinking of above.

If we switch to input_iterator, returning by value is fine.

(Anyone know what happened to the "new iterator concepts" [1] that
fixed this nonsense of conflating traversal with value access?

[1]: http://www.boost.org/doc/libs/1_57_0/libs/iterator/doc/new-iter-concepts.html
)