Remove IteratorRange in favor of llvm::iterator_range #27574
Conversation
Now that llvm::iterator_range has 'empty', there's not enough reason to keep our own version of it in the Swift repo. No functionality change.
|
apple/swift-llvm#174 |
|
|
||
| /// An iterator that transforms the result of an underlying bidirectional | ||
| /// iterator with a given operation. | ||
| /// | ||
| /// Slightly different semantics from llvm::map_iterator, but we should |
There was a problem hiding this comment.
What are the different semantics here? Just seems out of place with the rest of the PR. Can you elaborate in the code?
There was a problem hiding this comment.
It's something about whether the underlying iterator returns a reference or not. I don't actually remember the details, but I tried to remove it and failed and I want to leave a warning for others to come.
There was a problem hiding this comment.
It is not the purpose of the PR, as you note, so if you object to it going in in this form I'll just drop it.
include/swift/AST/DeclContext.h
Outdated
| @@ -667,7 +667,7 @@ class DeclIterator { | |||
|
|
|||
| /// The range of declarations stored within an iterable declaration | |||
| /// context. | |||
| typedef IteratorRange<DeclIterator> DeclRange; | |||
| typedef llvm::iterator_range<DeclIterator> DeclRange; | |||
There was a problem hiding this comment.
I mostly wasn't updating existing code, but sure.
Sure, will do. |
If we're going to get rid of swift::IteratorRange, let's make llvm::iterator_range easy to use. No functionality change.
|
apple/swift-llvm#174 |
|
Hm, guess I have to wait until the LLVM change propagates into |
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
Now that
llvm::iterator_rangehasempty, there's not enough reason to keep our own version of it in the Swift repo.No functionality change.