[PATCH] Add iterator for PHINode value/BB pair

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Jul 22 12:11:39 PDT 2015


> On 2015-Jul-21, at 21:20, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> Thanks for all the feedback.  This is a patch which addresses all of it.
> 
> <phinode.diff>
> 

> +
> +    const PHINodeEdge operator*() const {

No reason for the const in `const PHINodeEdge` here.

> +      return { *Values, *Blocks };
> +    }
> +  };
> +

Otherwise LGTM.  Might want to pass it through clang-format; I noticed
some minor whitespace oddities.

> *snip*
>> On Jul 21, 2015, at 8:21 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> 
>> 
>> If you drop the requirements from `forward_iterator` to
>> `input_iterator`, then you're allowed to return a `PHINodeEdge` by-value
>> here instead of by-reference (unfortunately this makes it illegal to use
>> a bunch of STL algorithms; STL iterator traits are completely broken
>> IMO).
> I’m fine with this.  David, Chandler, please let me know how you feel about this.
> 
> Also, i forgot to say that I considered doing a zip iterator and inheriting this from it.  This is something I think Chandler or David mentioned a few months ago.  If there’s been any progress in the C++ committee on that then i’m happy to try implement something better.  If not, then i don’t think what I have here should be difficult to change in future.

I guess a generalized version would return a
`std::tuple<Value *const &, BasicBlock *const &>` or some such.  Not
sure how to actually make zip iterators work well without something
like N1550 though.



More information about the llvm-commits mailing list