[cxx-interop] Conform to UnsafeCxxContiguousIterator based on iterator_concept nested type
#77049
Conversation
egorzhdan
requested review from
zoecarver,
hyp,
Xazax-hun,
beccadax and
ian-twilightcoder
as code owners
|
@swift-ci please smoke test |
| // `iterator_concept`. It is not possible to detect a contiguous iterator | ||
| // based on its `iterator_category`. The type might not have an | ||
| // `iterator_concept` defined. | ||
| auto iteratorConcept = getIteratorConceptDecl(clangDecl); |
There was a problem hiding this comment.
Would it make sense to move this down, closer to its use?
| auto unwrapUnderlyingTypeDecl = | ||
| [](clang::TypeDecl *typeDecl) -> clang::CXXRecordDecl * { | ||
| clang::CXXRecordDecl *underlyingDecl = nullptr; | ||
| if (auto typedefDecl = dyn_cast<clang::TypedefNameDecl>(typeDecl)) { |
There was a problem hiding this comment.
Could we have multiple layers of TypedefNameDecls? Would that be a problem?
E.g.:
struct MyIterator {
using iterator_concept = std::vector<int>::iterator::iterator_concept;
// ...
private:
std::vector<int>::iterator it;
};
There was a problem hiding this comment.
Good catch! Let's make sure this works. I'll add a test.
There was a problem hiding this comment.
Actually this already works fine, I'll add a test in a separate PR.
| }; | ||
|
|
||
| // Traverse all transitive bases of `underlyingDecl` to check if | ||
| // it inherits from `std::input_iterator_tag`. | ||
| bool isInputIterator = isInputIteratorDecl(underlyingCategoryDecl); | ||
| bool isRandomAccessIterator = | ||
| isRandomAccessIteratorDecl(underlyingCategoryDecl); | ||
| bool isContiguousIterator = isContiguousIteratorDecl(underlyingCategoryDecl); | ||
| underlyingCategoryDecl->forallBases([&](const clang::CXXRecordDecl *base) { | ||
| if (isInputIteratorDecl(base)) { |
There was a problem hiding this comment.
We do not shortcut the search if we only found an input iterator but we do when we found a random access one. Is this intended?
There was a problem hiding this comment.
Yeap, this is intended. If we found an input_iterator_tag, we might still find a random_access_iterator_tag later during traversal, which gives us stronger guarantees. This isn't very likely to happen in practice, but it's valid to have a complex inheritance tree of the tags.
…ator_concept` nested type 3a200de has a logic bug where we tried to conform C++ iterator types to `UnsafeCxxContiguousIterator` protocol based on their nested type called `iterator_category`. The C++20 standard says we should rely on `iterator_concept` instead. https://en.cppreference.com/w/cpp/iterator/iterator_tags#Iterator_concept Despite what the name suggests, we are not actually using C++ concepts in this change. rdar://137877849
egorzhdan
force-pushed
the
egorzhdan/cxx-contiguous-fix
branch
from
42b3db7 to
34f6cd3
Compare
|
@swift-ci please smoke test |
|
@swift-ci please build toolchain Amazon Linux 2 |
3a200de has a logic bug where we tried to conform C++ iterator types to
UnsafeCxxContiguousIteratorprotocol based on their nested type callediterator_category. The C++20 standard says we should rely oniterator_conceptinstead.https://en.cppreference.com/w/cpp/iterator/iterator_tags#Iterator_concept
Despite what the name suggests, we are not actually using C++ concepts in this change.
rdar://137877849