https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/191030
append()/assign() iterate over the iterator twice -- once to get the length and once to actually append the content. This is only permitted with forward iterators, not input iterators. Fix the check to test for the forward iterator tag instead and deal with the fallout. Originally introduced in https://reviews.llvm.org/D33919. >From 554c5b139755b2057b1f276d9d86d1ec29e63fb9 Mon Sep 17 00:00:00 2001 From: Alexis Engelke <[email protected]> Date: Wed, 8 Apr 2026 18:49:57 +0000 Subject: [PATCH] [spr] initial version Created using spr 1.3.8-wip --- clang/lib/Lex/PPDirectives.cpp | 5 ++-- clang/lib/Sema/SemaCodeComplete.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 3 ++- llvm/include/llvm/ADT/SmallVector.h | 19 +++++++++----- llvm/include/llvm/IR/Metadata.h | 2 +- llvm/include/llvm/SandboxIR/User.h | 2 +- llvm/tools/llvm-cov/CoverageReport.cpp | 5 ++-- llvm/unittests/ADT/IteratorTest.cpp | 10 +++---- .../DWARF/DWARFAcceleratorTableTest.cpp | 26 ++++++++++--------- 9 files changed, 42 insertions(+), 32 deletions(-) diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 172c2fa57db00..153ddc944cad3 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2652,8 +2652,9 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( NameWithoriginalSlashes.consume_front("\\\\?\\"); #endif StringRef RealPathName = File->getFileEntry().tryGetRealPathName(); - SmallVector<StringRef, 16> Components(llvm::sys::path::begin(Name), - llvm::sys::path::end(Name)); + SmallVector<StringRef, 16> Components; + std::copy(llvm::sys::path::begin(Name), llvm::sys::path::end(Name), + std::back_inserter(Components)); #if defined(_WIN32) // -Wnonportable-include-path is designed to diagnose includes using // case even on systems with a case-insensitive file system. diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 0cd9819dc2964..e9f856b891361 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -10053,7 +10053,7 @@ void SemaCodeCompletion::CodeCompleteObjCMethodDecl( IFace = Category->getClassInterface(); if (IFace) - llvm::append_range(Containers, IFace->visible_categories()); + llvm::copy(IFace->visible_categories(), std::back_inserter(Containers)); if (IsInstanceMethod) { for (unsigned I = 0, N = Containers.size(); I != N; ++I) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 4b3adce07f10c..8100d6dfd0d3d 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3982,7 +3982,8 @@ class ASTIdentifierTableTrait { // "stat"), but the ASTReader adds declarations to the end of the list // (so we need to see the struct "stat" before the function "stat"). // Only emit declarations that aren't from a chained PCH, though. - SmallVector<NamedDecl *, 16> Decls(IdResolver->decls(II)); + SmallVector<NamedDecl *, 16> Decls; + llvm::copy(IdResolver->decls(II), std::back_inserter(Decls)); for (NamedDecl *D : llvm::reverse(Decls)) LE.write<DeclID>((DeclID)Writer.getDeclID( getDeclForLocalLookup(PP.getLangOpts(), D))); diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h index 23d40c3e07675..c8ea0654f0a04 100644 --- a/llvm/include/llvm/ADT/SmallVector.h +++ b/llvm/include/llvm/ADT/SmallVector.h @@ -39,9 +39,10 @@ template <typename T> class ArrayRef; template <typename IteratorT> class iterator_range; template <class Iterator> -using EnableIfConvertibleToInputIterator = std::enable_if_t<std::is_convertible< - typename std::iterator_traits<Iterator>::iterator_category, - std::input_iterator_tag>::value>; +using EnableIfConvertibleToForwardIterator = + std::enable_if_t<std::is_convertible< + typename std::iterator_traits<Iterator>::iterator_category, + std::forward_iterator_tag>::value>; /// This is all the stuff common to all SmallVectors. /// @@ -682,7 +683,8 @@ class SmallVectorImpl : public SmallVectorTemplateBase<T> { void swap(SmallVectorImpl &RHS); /// Add the specified range to the end of the SmallVector. - template <typename ItTy, typename = EnableIfConvertibleToInputIterator<ItTy>> + template <typename ItTy, + typename = EnableIfConvertibleToForwardIterator<ItTy>> void append(ItTy in_start, ItTy in_end) { this->assertSafeToAddRange(in_start, in_end); size_type NumInputs = std::distance(in_start, in_end); @@ -723,7 +725,8 @@ class SmallVectorImpl : public SmallVectorTemplateBase<T> { // FIXME: Consider assigning over existing elements, rather than clearing & // re-initializing them - for all assign(...) variants. - template <typename ItTy, typename = EnableIfConvertibleToInputIterator<ItTy>> + template <typename ItTy, + typename = EnableIfConvertibleToForwardIterator<ItTy>> void assign(ItTy in_start, ItTy in_end) { this->assertSafeToReferenceAfterClear(in_start, in_end); clear(); @@ -880,7 +883,8 @@ class SmallVectorImpl : public SmallVectorTemplateBase<T> { return I; } - template <typename ItTy, typename = EnableIfConvertibleToInputIterator<ItTy>> + template <typename ItTy, + typename = EnableIfConvertibleToForwardIterator<ItTy>> iterator insert(iterator I, ItTy From, ItTy To) { // Convert iterator to elt# to avoid invalidating iterator when we reserve() size_t InsertElt = I - this->begin(); @@ -1221,7 +1225,8 @@ class LLVM_GSL_OWNER SmallVector : public SmallVectorImpl<T>, this->assign(Size, Value); } - template <typename ItTy, typename = EnableIfConvertibleToInputIterator<ItTy>> + template <typename ItTy, + typename = EnableIfConvertibleToForwardIterator<ItTy>> SmallVector(ItTy S, ItTy E) : SmallVectorImpl<T>(N) { this->append(S, E); } diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h index f6c393c1d8c67..09bdb79a4dfda 100644 --- a/llvm/include/llvm/IR/Metadata.h +++ b/llvm/include/llvm/IR/Metadata.h @@ -1625,7 +1625,7 @@ template <class T> class TypedMDOperandIterator { MDNode::op_iterator I = nullptr; public: - using iterator_category = std::input_iterator_tag; + using iterator_category = std::forward_iterator_tag; using value_type = T *; using difference_type = std::ptrdiff_t; using pointer = void; diff --git a/llvm/include/llvm/SandboxIR/User.h b/llvm/include/llvm/SandboxIR/User.h index c552e2e3378be..2404ca40a72ae 100644 --- a/llvm/include/llvm/SandboxIR/User.h +++ b/llvm/include/llvm/SandboxIR/User.h @@ -34,7 +34,7 @@ class OperandUseIterator { using value_type = sandboxir::Use; using pointer = value_type *; using reference = value_type &; - using iterator_category = std::input_iterator_tag; + using iterator_category = std::forward_iterator_tag; OperandUseIterator() = default; LLVM_ABI value_type operator*() const; diff --git a/llvm/tools/llvm-cov/CoverageReport.cpp b/llvm/tools/llvm-cov/CoverageReport.cpp index 9b754d613370c..b243854793579 100644 --- a/llvm/tools/llvm-cov/CoverageReport.cpp +++ b/llvm/tools/llvm-cov/CoverageReport.cpp @@ -139,8 +139,9 @@ raw_ostream::Colors determineCoveragePercentageColor(const T &Info) { unsigned getNumRedundantPathComponents(ArrayRef<std::string> Paths) { // To start, set the number of redundant path components to the maximum // possible value. - SmallVector<StringRef, 8> FirstPathComponents{sys::path::begin(Paths[0]), - sys::path::end(Paths[0])}; + SmallVector<StringRef, 8> FirstPathComponents; + std::copy(sys::path::begin(Paths[0]), sys::path::end(Paths[0]), + std::back_inserter(FirstPathComponents)); unsigned NumRedundant = FirstPathComponents.size(); for (unsigned I = 1, E = Paths.size(); NumRedundant > 0 && I < E; ++I) { diff --git a/llvm/unittests/ADT/IteratorTest.cpp b/llvm/unittests/ADT/IteratorTest.cpp index 9dd8c1a84f44a..6f4bc4ca8bffe 100644 --- a/llvm/unittests/ADT/IteratorTest.cpp +++ b/llvm/unittests/ADT/IteratorTest.cpp @@ -336,16 +336,16 @@ TEST(FilterIteratorTest, Composition) { EXPECT_EQ((SmallVector<int, 3>{1, 3, 5}), Actual); } -TEST(FilterIteratorTest, InputIterator) { - struct InputIterator - : iterator_adaptor_base<InputIterator, int *, std::input_iterator_tag> { - InputIterator(int *It) : InputIterator::iterator_adaptor_base(It) {} +TEST(FilterIteratorTest, ForwardIterator) { + struct ForwardIterator : iterator_adaptor_base<ForwardIterator, int *, + std::forward_iterator_tag> { + ForwardIterator(int *It) : ForwardIterator::iterator_adaptor_base(It) {} }; auto IsOdd = [](int N) { return N % 2 == 1; }; int A[] = {0, 1, 2, 3, 4, 5, 6}; auto Range = make_filter_range( - make_range(InputIterator(std::begin(A)), InputIterator(std::end(A))), + make_range(ForwardIterator(std::begin(A)), ForwardIterator(std::end(A))), IsOdd); SmallVector<int, 3> Actual(Range.begin(), Range.end()); EXPECT_EQ((SmallVector<int, 3>{1, 3, 5}), Actual); diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp index 82cc02c921d15..76bc93cf970ac 100644 --- a/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp +++ b/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp @@ -106,16 +106,18 @@ TEST(DWARFDebugNames, BasicTestEntries) { DWARFDebugNames::NameTableEntry SecondEntry = NameIndex.getNameTableEntry(2); ASSERT_EQ(SecondEntry.getString(), StringRef("NameType2")); - SmallVector<DWARFDebugNames::Entry> FirstNameEntries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("NameType1")); + SmallVector<DWARFDebugNames::Entry> FirstNameEntries; + llvm::copy(NameIndex.equal_range("NameType1"), + std::back_inserter(FirstNameEntries)); ASSERT_EQ(FirstNameEntries.size(), 2u); ASSERT_EQ(FirstNameEntries[0].getCUIndex(), 0u); ASSERT_EQ(FirstNameEntries[1].getCUIndex(), 0x2); ASSERT_EQ(FirstNameEntries[0].getDIEUnitOffset(), 0x0); ASSERT_EQ(FirstNameEntries[1].getDIEUnitOffset(), 0x2); - SmallVector<DWARFDebugNames::Entry> SecondNameEntries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("NameType2")); + SmallVector<DWARFDebugNames::Entry> SecondNameEntries; + llvm::copy(NameIndex.equal_range("NameType2"), + std::back_inserter(SecondNameEntries)); ASSERT_EQ(SecondNameEntries.size(), 1u); ASSERT_EQ(SecondNameEntries[0].getCUIndex(), 0x1); ASSERT_EQ(SecondNameEntries[0].getDIEUnitOffset(), 0x1); @@ -181,32 +183,32 @@ TEST(DWARFDebugNames, ParentEntries) { ASSERT_NE(DebugNames.begin(), DebugNames.end()); const DWARFDebugNames::NameIndex &NameIndex = *DebugNames.begin(); - SmallVector<DWARFDebugNames::Entry> Name1Entries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("Name1")); + SmallVector<DWARFDebugNames::Entry> Name1Entries; + llvm::copy(NameIndex.equal_range("Name1"), std::back_inserter(Name1Entries)); ASSERT_EQ(Name1Entries.size(), 1u); Expected<std::optional<DWARFDebugNames::Entry>> Name1Parent = Name1Entries[0].getParentDIEEntry(); ASSERT_THAT_EXPECTED(Name1Parent, Succeeded()); ASSERT_EQ(*Name1Parent, std::nullopt); // Name1 has no parent - SmallVector<DWARFDebugNames::Entry> Name2Entries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("Name2")); + SmallVector<DWARFDebugNames::Entry> Name2Entries; + llvm::copy(NameIndex.equal_range("Name2"), std::back_inserter(Name2Entries)); ASSERT_EQ(Name2Entries.size(), 1u); Expected<std::optional<DWARFDebugNames::Entry>> Name2Parent = Name2Entries[0].getParentDIEEntry(); ASSERT_THAT_EXPECTED(Name2Parent, Succeeded()); ASSERT_EQ((**Name2Parent).getDIEUnitOffset(), 0x0); // Name2 parent == Name1 - SmallVector<DWARFDebugNames::Entry> Name3Entries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("Name3")); + SmallVector<DWARFDebugNames::Entry> Name3Entries; + llvm::copy(NameIndex.equal_range("Name3"), std::back_inserter(Name3Entries)); ASSERT_EQ(Name3Entries.size(), 1u); Expected<std::optional<DWARFDebugNames::Entry>> Name3Parent = Name3Entries[0].getParentDIEEntry(); ASSERT_THAT_EXPECTED(Name3Parent, Succeeded()); ASSERT_EQ((**Name3Parent).getDIEUnitOffset(), 0x1); // Name3 parent == Name2 - SmallVector<DWARFDebugNames::Entry> Name4Entries = - to_vector_of<DWARFDebugNames::Entry>(NameIndex.equal_range("Name4")); + SmallVector<DWARFDebugNames::Entry> Name4Entries; + llvm::copy(NameIndex.equal_range("Name4"), std::back_inserter(Name4Entries)); ASSERT_EQ(Name4Entries.size(), 1u); ASSERT_FALSE(Name4Entries[0].hasParentInformation()); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
