diff --git a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp index 36b6007b58a51..4382f9df5336e 100644 --- a/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp +++ b/clang-tools-extra/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp @@ -39,7 +39,7 @@ bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node, bool &IsInterface) const { assert(Node->getIdentifier()); StringRef Name = Node->getIdentifier()->getName(); - llvm::StringMapConstIterator Pair = InterfaceMap.find(Name); + auto Pair = InterfaceMap.find(Name); if (Pair == InterfaceMap.end()) return false; IsInterface = Pair->second; diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index e83aee01b75cb..f69ac41dddb3e 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -1936,8 +1936,7 @@ Error OffloadBundler::UnbundleArchive() { /// Write out an archive for each target for (auto &Target : BundlerConfig.TargetNames) { StringRef FileName = TargetOutputFileNameMap[Target]; - StringMapIterator> CurArchiveMembers = - OutputArchivesMap.find(Target); + auto CurArchiveMembers = OutputArchivesMap.find(Target); if (CurArchiveMembers != OutputArchivesMap.end()) { if (Error WriteErr = writeArchive(FileName, CurArchiveMembers->getValue(), SymtabWritingMode::NormalSymtab, diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h index dbc0485967ac6..6dfbf4f4618c1 100644 --- a/llvm/include/llvm/ADT/StringMap.h +++ b/llvm/include/llvm/ADT/StringMap.h @@ -21,11 +21,11 @@ #include "llvm/Support/PointerLikeTypeTraits.h" #include #include +#include namespace llvm { -template class StringMapConstIterator; -template class StringMapIterator; +template class StringMapIterBase; template class StringMapKeyIterator; /// StringMapImpl - This is the base class of StringMap that is shared among @@ -217,8 +217,8 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap using value_type = StringMapEntry; using size_type = size_t; - using const_iterator = StringMapConstIterator; - using iterator = StringMapIterator; + using const_iterator = StringMapIterBase; + using iterator = StringMapIterBase; iterator begin() { return iterator(TheTable, NumBuckets == 0); } iterator end() { return iterator(TheTable + NumBuckets, true); } @@ -431,14 +431,17 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap } }; -template -class StringMapIterBase - : public iterator_facade_base { -protected: +template class StringMapIterBase { StringMapEntryBase **Ptr = nullptr; public: + using iterator_category = std::forward_iterator_tag; + using value_type = StringMapEntry; + using difference_type = std::ptrdiff_t; + using pointer = std::conditional_t; + using reference = + std::conditional_t; + StringMapIterBase() = default; explicit StringMapIterBase(StringMapEntryBase **Bucket, @@ -448,85 +451,56 @@ class StringMapIterBase AdvancePastEmptyBuckets(); } - DerivedTy &operator=(const DerivedTy &Other) { - Ptr = Other.Ptr; - return static_cast(*this); - } - - friend bool operator==(const DerivedTy &LHS, const DerivedTy &RHS) { - return LHS.Ptr == RHS.Ptr; - } + reference operator*() const { return *static_cast(*Ptr); } + pointer operator->() const { return static_cast(*Ptr); } - DerivedTy &operator++() { // Preincrement + StringMapIterBase &operator++() { // Preincrement ++Ptr; AdvancePastEmptyBuckets(); - return static_cast(*this); + return *this; } - DerivedTy operator++(int) { // Post-increment - DerivedTy Tmp(Ptr); + StringMapIterBase operator++(int) { // Post-increment + StringMapIterBase Tmp(*this); ++*this; return Tmp; } -private: - void AdvancePastEmptyBuckets() { - while (*Ptr == nullptr || *Ptr == StringMapImpl::getTombstoneVal()) - ++Ptr; + template ::type> + operator StringMapIterBase() const { + return StringMapIterBase(Ptr, true); } -}; - -template -class StringMapConstIterator - : public StringMapIterBase, - const StringMapEntry> { - using base = StringMapIterBase, - const StringMapEntry>; - -public: - StringMapConstIterator() = default; - explicit StringMapConstIterator(StringMapEntryBase **Bucket, - bool NoAdvance = false) - : base(Bucket, NoAdvance) {} - const StringMapEntry &operator*() const { - return *static_cast *>(*this->Ptr); + friend bool operator==(const StringMapIterBase &LHS, + const StringMapIterBase &RHS) { + return LHS.Ptr == RHS.Ptr; } -}; - -template -class StringMapIterator : public StringMapIterBase, - StringMapEntry> { - using base = - StringMapIterBase, StringMapEntry>; -public: - StringMapIterator() = default; - explicit StringMapIterator(StringMapEntryBase **Bucket, - bool NoAdvance = false) - : base(Bucket, NoAdvance) {} - - StringMapEntry &operator*() const { - return *static_cast *>(*this->Ptr); + friend bool operator!=(const StringMapIterBase &LHS, + const StringMapIterBase &RHS) { + return !(LHS == RHS); } - operator StringMapConstIterator() const { - return StringMapConstIterator(this->Ptr, true); +private: + void AdvancePastEmptyBuckets() { + while (*Ptr == nullptr || *Ptr == StringMapImpl::getTombstoneVal()) + ++Ptr; } }; template class StringMapKeyIterator : public iterator_adaptor_base, - StringMapConstIterator, + StringMapIterBase, std::forward_iterator_tag, StringRef> { using base = iterator_adaptor_base, - StringMapConstIterator, + StringMapIterBase, std::forward_iterator_tag, StringRef>; public: StringMapKeyIterator() = default; - explicit StringMapKeyIterator(StringMapConstIterator Iter) + explicit StringMapKeyIterator(StringMapIterBase Iter) : base(std::move(Iter)) {} StringRef operator*() const { return this->wrapped()->getKey(); } diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp index 35135cdb297e7..92ae364d45d5e 100644 --- a/llvm/unittests/ADT/StringMapTest.cpp +++ b/llvm/unittests/ADT/StringMapTest.cpp @@ -693,4 +693,58 @@ TEST(StringMapCustomTest, StringMapEntrySize) { EXPECT_EQ(LargeValue, Key.size()); } +TEST(StringMapCustomTest, NonConstIterator) { + StringMap Map; + Map["key"] = 1; + + // Check that Map.begin() returns a non-const iterator. + static_assert( + std::is_same_v::iterator>); + + // Check that the value_type of a non-const iterator is not a const type. + static_assert( + !std::is_const_v::iterator::value_type>, + "The value_type of a non-const iterator should not be a const type."); + + // Check that pointer and reference types are not const. + static_assert(std::is_same_v::iterator::pointer, + StringMap::iterator::value_type *>); + static_assert(std::is_same_v::iterator::reference, + StringMap::iterator::value_type &>); + + // Check that we can construct a const_iterator from an iterator. + static_assert(std::is_constructible_v::const_iterator, + StringMap::iterator>); + + // Double check that we can actually construct a const_iterator. + StringMap::const_iterator const_it = Map.begin(); + (void)const_it; +} + +TEST(StringMapCustomTest, ConstIterator) { + StringMap Map; + const StringMap &ConstMap = Map; + + // Check that ConstMap.begin() returns a const_iterator. + static_assert(std::is_same_v::const_iterator>); + + // Check that the value_type of a const iterator is not a const type. + static_assert( + !std::is_const_v::const_iterator::value_type>, + "The value_type of a const iterator should not be a const type."); + + // Check that pointer and reference types are const. + static_assert( + std::is_same_v::const_iterator::pointer, + const StringMap::const_iterator::value_type *>); + static_assert( + std::is_same_v::const_iterator::reference, + const StringMap::const_iterator::value_type &>); + + // Check that we cannot construct an iterator from a const_iterator. + static_assert(!std::is_constructible_v::iterator, + StringMap::const_iterator>); +} + } // end anonymous namespace