This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Add public iterator and const_iterator types to iterator_range
AbandonedPublic

Authored by abrachet on Jul 19 2019, 7:39 AM.

Details

Summary

iterator_range was originally modeled on a proposed std::iterator_range, I never found those papers but boost's iterator_range publicly exposes these types.

Diff Detail

Event Timeline

abrachet created this revision.Jul 19 2019, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2019, 7:39 AM

Just to note: there's a typo in your summary (cosnt_iterator -> const_iterator).

What's the context for this? Why do you need these iterator types defined rather than just using whatever was passed in? (I think I know the answer, but I just want you to explain for people reading this in the future).

abrachet retitled this revision from [ADT] Add public iterator and cosnt_iterator types to iterator_range to [ADT] Add public iterator and const_iterator types to iterator_range.Jul 19 2019, 8:14 AM

Defining iterator and const_iterator allows it to be used specifically for my use case in D64462 but also in other types I'm sure. Notably many function templates in iterator will not work, std::begin and std::end need one of these types to be publicly defined for example, but one would expect these function templates to work with llvm::iterator_range.

jhenderson accepted this revision.Jul 19 2019, 8:38 AM

Okay, I think this LGTM, but only put this in if this is actually needed in any final version of D64462.

This revision is now accepted and ready to land.Jul 19 2019, 8:38 AM
MaskRay added inline comments.Jul 19 2019, 9:27 AM
llvm/include/llvm/ADT/iterator_range.h
39

You can insert

using iterator = IteratorT;
using const_iterator = IteratorT;

here to avoid adding two access modifiers.

abrachet abandoned this revision.Jul 21 2019, 8:47 PM