[ADT][NFCI] Do not use non-const lvalue-refs with enumerate in llvm/

[kuhar]

Summary

Replace references to enumerate results with either const lvalue
rerences or structured bindings. I did not use structured bindings
everywhere as it wasn't clear to me it would improve readability.

This is in preparation to the switch to zip semantics which won't
support non-const lvalue reference to elements:
https://reviews.llvm.org/D144503.

[kuhar]

Also include clang/ since there's only one function to update

[dblaikie]

Looks alright - thanks! :)

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3962

I'd probably pass by const ref here - the forwarding ref seems a bit more general than needed/looks like there could be ownership transfer, etc.

[kuhar]

Update lambda signature

clang/tools/clang-refactor/TestSupport.cpp

std::pair<unsigned, unsigned> getLineColumn(StringRef Filename,
   return {Source.count('\n') + 1,
           (LastLine == StringRef::npos ? Offset : Offset - LastLine) + 1};
 }
 
 } // end anonymous namespace
 
 bool TestRefactoringResultConsumer::handleAllResults() {
   bool Failed = false;
-  for (auto &Group : llvm::enumerate(Results)) {
+  for (const auto &Group : llvm::enumerate(Results)) {
     // All ranges in the group must produce the same result.
     std::optional<tooling::AtomicChanges> CanonicalResult;
     std::optional<std::string> CanonicalErrorMessage;
-    for (auto &I : llvm::enumerate(Group.value())) {
+    for (const auto &I : llvm::enumerate(Group.value())) {
       Expected<tooling::AtomicChanges> &Result = I.value();
       std::string ErrorMessage;
       bool HasResult = !!Result;
       if (!HasResult) {
         handleAllErrors(
             Result.takeError(),
             [&](StringError &Err) { ErrorMessage = Err.getMessage(); },
             [&](DiagnosticError &Err) {

llvm/lib/ObjectYAML/DWARFYAML.cpp

if (DebugLoclists)
     SecNames.insert("debug_loclists");
   return SecNames;
 }
 
 Expected<DWARFYAML::Data::AbbrevTableInfo>
 DWARFYAML::Data::getAbbrevTableInfoByID(uint64_t ID) const {
   if (AbbrevTableInfoMap.empty()) {
     uint64_t AbbrevTableOffset = 0;
-    for (auto &AbbrevTable : enumerate(DebugAbbrev)) {
+    for (const auto &[Index, AbbrevTable] : enumerate(DebugAbbrev)) {
       // If the abbrev table's ID isn't specified, we use the index as its ID.
-      uint64_t AbbrevTableID =
-          AbbrevTable.value().ID.value_or(AbbrevTable.index());
+      uint64_t AbbrevTableID = AbbrevTable.ID.value_or(Index);
       auto It = AbbrevTableInfoMap.insert(
-          {AbbrevTableID, AbbrevTableInfo{/*Index=*/AbbrevTable.index(),
+          {AbbrevTableID, AbbrevTableInfo{/*Index=*/Index,
                                           /*Offset=*/AbbrevTableOffset}});
       if (!It.second)
         return createStringError(
             errc::invalid_argument,
             "the ID (%" PRIu64 ") of abbrev table with index %zu has been used "
             "by abbrev table with index %" PRIu64,
-            AbbrevTableID, AbbrevTable.index(), It.first->second.Index);
+            AbbrevTableID, Index, It.first->second.Index);
 
-      AbbrevTableOffset +=
-          getAbbrevTableContentByIndex(AbbrevTable.index()).size();
+      AbbrevTableOffset += getAbbrevTableContentByIndex(Index).size();
     }
   }
 
   auto It = AbbrevTableInfoMap.find(ID);
   if (It == AbbrevTableInfoMap.end())
     return createStringError(errc::invalid_argument,
                              "cannot find abbrev table whose ID is %" PRIu64,
                              ID);

llvm/lib/ObjectYAML/MinidumpEmitter.cpp

bool yaml2minidump(MinidumpYAML::Object &Obj, raw_ostream &Out,
                    ErrorHandler /*EH*/) {
   BlobAllocator File;
   File.allocateObject(Obj.Header);
 
   std::vector<Directory> StreamDirectory(Obj.Streams.size());
   Obj.Header.StreamDirectoryRVA = File.allocateArray(ArrayRef(StreamDirectory));
   Obj.Header.NumberOfStreams = StreamDirectory.size();
 
-  for (auto &Stream : enumerate(Obj.Streams))
-    StreamDirectory[Stream.index()] = layout(File, *Stream.value());
+  for (const auto &[Index, Stream] : enumerate(Obj.Streams))
+    StreamDirectory[Index] = layout(File, *Stream);
 
   File.writeTo(Out);
   return true;
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

 };
 } // namespace
 
 /// Prepares a use bitset for the given mask either for the first argument or
 /// for the second.
 static SmallBitVector buildUseMask(int VF, ArrayRef<int> Mask,
                                    UseMask MaskArg) {
   SmallBitVector UseMask(VF, true);
-  for (auto P : enumerate(Mask)) {
-    if (P.value() == UndefMaskElem) {
+  for (auto [Idx, Value] : enumerate(Mask)) {
+    if (Value == UndefMaskElem) {
       if (MaskArg == UseMask::UndefsAsMask)
-        UseMask.reset(P.index());
+        UseMask.reset(Idx);
       continue;
     }
-    if (MaskArg == UseMask::FirstArg && P.value() < VF)
-      UseMask.reset(P.value());
-    else if (MaskArg == UseMask::SecondArg && P.value() >= VF)
-      UseMask.reset(P.value() - VF);
+    if (MaskArg == UseMask::FirstArg && Value < VF)
+      UseMask.reset(Value);
+    else if (MaskArg == UseMask::SecondArg && Value >= VF)
+      UseMask.reset(Value - VF);
   }
   return UseMask;
 }
 
 /// Checks if the given value is actually an undefined constant vector.
 /// Also, if the \p UseMask is not empty, tries to check if the non-masked
 /// elements actually mask the insertelement buildvector, if any.
 template <bool IsPoisonOnly = false>
bool clusterSortPtrAccesses(ArrayRef<Value *> VL, Type *ElemTy,
   for (auto &Base : Bases) {
     auto &Vec = Base.second;
     if (Vec.size() > 1) {
       llvm::stable_sort(Vec, [](const std::tuple<Value *, int, unsigned> &X,
                                 const std::tuple<Value *, int, unsigned> &Y) {
         return std::get<1>(X) < std::get<1>(Y);
       });
       int InitialOffset = std::get<1>(Vec[0]);
-      AnyConsecutive |= all_of(enumerate(Vec), [InitialOffset](auto &P) {
+      AnyConsecutive |= all_of(enumerate(Vec), [InitialOffset](const auto &P) {
         return std::get<1>(P.value()) == int(P.index()) + InitialOffset;
       });
     }
   }
 
   // Fill SortedIndices array only if it looks worth-while to sort the ptrs.
   SortedIndices.clear();
   if (!AnyConsecutive)

llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp

for (Value *V : InstToDelete) {
         I->eraseFromParent();
     }
 
     // No arguments to reduce
     if (VMap.empty())
       continue;
 
     std::set<int> ArgIndexesToKeep;
-    for (auto &Arg : enumerate(F->args()))
-      if (ArgsToKeep.count(&Arg.value()))
-        ArgIndexesToKeep.insert(Arg.index());
+    for (const auto &[Index, Arg] : enumerate(F->args()))
+      if (ArgsToKeep.count(&Arg))
+        ArgIndexesToKeep.insert(Index);
 
     auto *ClonedFunc = CloneFunction(F, VMap);
     // In order to preserve function order, we move Clone after old Function
     ClonedFunc->removeFromParent();
     Program.getFunctionList().insertAfter(F->getIterator(), ClonedFunc);
 
     replaceFunctionCalls(*F, *ClonedFunc, ArgIndexesToKeep);
     // Rename Cloned Function to Old's name

llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

TEST_F(RandomAccessVisitorTest, MultipleVisits) {
 
   // [0,8) should be present
   EXPECT_EQ(8u, Types.size());
   for (uint32_t I = 0; I < 8; ++I)
     EXPECT_TRUE(ValidateDatabaseRecord(Types, I));
 
   // 5, 5, 5
   EXPECT_EQ(3u, TestState->Callbacks.count());
-  for (auto I : enumerate(IndicesToVisit))
+  for (const auto &I : enumerate(IndicesToVisit))
     EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
 }
 
 TEST_F(RandomAccessVisitorTest, DescendingWithinChunk) {
   // Visit multiple items from the same "chunk" in reverse order.  In this
   // example, it's 7 then 4 then 2.  At the end, all records from 0 to 7 should
   // be known by the database, but only 2, 4, and 7 should have been visited.
   TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
TEST_F(RandomAccessVisitorTest, DescendingWithinChunk) {
 
   // [0, 7]
   EXPECT_EQ(8u, Types.size());
   for (uint32_t I = 0; I < 8; ++I)
     EXPECT_TRUE(ValidateDatabaseRecord(Types, I));
 
   // 2, 4, 7
   EXPECT_EQ(3u, TestState->Callbacks.count());
-  for (auto I : enumerate(IndicesToVisit))
+  for (const auto &I : enumerate(IndicesToVisit))
     EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
 }
 
 TEST_F(RandomAccessVisitorTest, AscendingWithinChunk) {
   // * Visit multiple items from the same chunk in ascending order, ensuring
   //   that intermediate items are not visited.  In the below example, it's
   //   5 -> 6 -> 7 which come from the [4,8) chunk.
   TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
TEST_F(RandomAccessVisitorTest, AscendingWithinChunk) {
 
   // [0, 7]
   EXPECT_EQ(8u, Types.size());
   for (uint32_t I = 0; I < 8; ++I)
     EXPECT_TRUE(ValidateDatabaseRecord(Types, I));
 
   // 2, 4, 7
   EXPECT_EQ(3u, TestState->Callbacks.count());
-  for (auto &I : enumerate(IndicesToVisit))
+  for (const auto &I : enumerate(IndicesToVisit))
     EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
 }
 
 TEST_F(RandomAccessVisitorTest, StopPrematurelyInChunk) {
   // * Don't visit the last item in one chunk, ensuring that visitation stops
   //   at the record you specify, and the chunk is only partially visited.
   //   In the below example, this is tested by visiting 0 and 1 but not 2,
   //   all from the [0,3) chunk.
TEST_F(RandomAccessVisitorTest, StopPrematurelyInChunk) {
 
   // [0, 8) should be visited.
   EXPECT_EQ(8u, Types.size());
   for (uint32_t I = 0; I < 8; ++I)
     EXPECT_TRUE(ValidateDatabaseRecord(Types, I));
 
   // [0, 2]
   EXPECT_EQ(3u, TestState->Callbacks.count());
-  for (auto I : enumerate(IndicesToVisit))
+  for (const auto &I : enumerate(IndicesToVisit))
     EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
 }
 
 TEST_F(RandomAccessVisitorTest, InnerChunk) {
   // Test that when a request comes from a chunk in the middle of the partial
   // offsets array, that items from surrounding chunks are not visited or
   // added to the database.
   TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 4, 9});
TEST_F(RandomAccessVisitorTest, InnerChunk) {
 
   // [4, 9)
   EXPECT_EQ(5u, Types.size());
   for (uint32_t I = 4; I < 9; ++I)
     EXPECT_TRUE(ValidateDatabaseRecord(Types, I));
 
   // 5, 7
   EXPECT_EQ(2u, TestState->Callbacks.count());
-  for (auto &I : enumerate(IndicesToVisit))
+  for (const auto &I : enumerate(IndicesToVisit))
     EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
 }
 
 TEST_F(RandomAccessVisitorTest, CrossChunkName) {
   AppendingTypeTableBuilder Builder(GlobalState->Allocator);
 
   // TypeIndex 0
   ClassRecord Class(TypeRecordKind::Class);

llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp

GIMatchTreeBuilderLeafInfo::GIMatchTreeBuilderLeafInfo(
     const GIMatchDag &MatchDag, void *Data)
     : Builder(Builder), Info(Name, RootIdx, Data), MatchDag(MatchDag),
       RemainingInstrNodes(BitVector(MatchDag.getNumInstrNodes(), true)),
       RemainingEdges(BitVector(MatchDag.getNumEdges(), true)),
       RemainingPredicates(BitVector(MatchDag.getNumPredicates(), true)),
       TraversableEdges(MatchDag.getNumEdges()),
       TestablePredicates(MatchDag.getNumPredicates()) {
   // Number all the predicates in this DAG
-  for (auto &P : enumerate(MatchDag.predicates())) {
-    PredicateIDs.insert(std::make_pair(P.value(), P.index()));
+  for (const auto &[Idx, P] : enumerate(MatchDag.predicates())) {
+    PredicateIDs.insert(std::make_pair(P, Idx));
   }
 
   // Number all the predicate dependencies in this DAG and set up a bitvector
   // for each predicate indicating the unsatisfied dependencies.
-  for (auto &Dep : enumerate(MatchDag.predicate_edges())) {
-    PredicateDepIDs.insert(std::make_pair(Dep.value(), Dep.index()));
+  for (const auto &[Idx, Dep] : enumerate(MatchDag.predicate_edges())) {
+    PredicateDepIDs.insert(std::make_pair(Dep, Idx));
   }
   UnsatisfiedPredDepsForPred.resize(MatchDag.getNumPredicates(),
                                     BitVector(PredicateDepIDs.size()));
-  for (auto &Dep : enumerate(MatchDag.predicate_edges())) {
-    unsigned ID = PredicateIDs.lookup(Dep.value()->getPredicate());
-    UnsatisfiedPredDepsForPred[ID].set(Dep.index());
+  for (const auto &[Idx, Dep] : enumerate(MatchDag.predicate_edges())) {
+    unsigned ID = PredicateIDs.lookup(Dep->getPredicate());
+    UnsatisfiedPredDepsForPred[ID].set(Idx);
   }
 }
 
 void GIMatchTreeBuilderLeafInfo::declareInstr(const GIMatchDagInstr *Instr, unsigned ID) {
   // Record the assignment of this instr to the given ID.
   auto InfoI = InstrNodeToInfo.insert(std::make_pair(
       Instr, GIMatchTreeInstrInfo(ID, Instr)));
   InstrIDToInfo.insert(std::make_pair(ID, &InfoI.first->second));
void GIMatchTreeBuilderLeafInfo::declareInstr(const GIMatchDagInstr *Instr, unsigned ID) {
   // When we declare an instruction, we don't expose any traversable edges just
   // yet. A partitioner has to check they exist and are registers before they
   // are traversable.
 
   // When we declare an instruction, we potentially activate some predicates.
   // Mark the dependencies that are now satisfied as a result of this
   // instruction and mark any predicates whose dependencies are fully
   // satisfied.
-  for (auto &Dep : enumerate(MatchDag.predicate_edges())) {
+  for (const auto &Dep : enumerate(MatchDag.predicate_edges())) {
     if (Dep.value()->getRequiredMI() == Instr &&
         Dep.value()->getRequiredMO() == nullptr) {
-      for (auto &DepsFor : enumerate(UnsatisfiedPredDepsForPred)) {
+      for (const auto &DepsFor : enumerate(UnsatisfiedPredDepsForPred)) {
         DepsFor.value().reset(Dep.index());
         if (DepsFor.value().none())
           TestablePredicates.set(DepsFor.index());
       }
     }
   }
 }
 
 void GIMatchTreeBuilderLeafInfo::declareOperand(unsigned InstrID,
                                                 unsigned OpIdx) {
   const GIMatchDagInstr *Instr = InstrIDToInfo.lookup(InstrID)->getInstrNode();
 
   OperandIDToInfo.insert(std::make_pair(
       std::make_pair(InstrID, OpIdx),
       GIMatchTreeOperandInfo(Instr, OpIdx)));
 
   // When an operand becomes reachable, we potentially activate some traversals.
   // Record the edges that can now be followed as a result of this
   // instruction.
-  for (auto &E : enumerate(MatchDag.edges())) {
-    if (E.value()->getFromMI() == Instr &&
-        E.value()->getFromMO()->getIdx() == OpIdx) {
-      TraversableEdges.set(E.index());
+  for (const auto &[Idx, E] : enumerate(MatchDag.edges())) {
+    if (E->getFromMI() == Instr && E->getFromMO()->getIdx() == OpIdx) {
+      TraversableEdges.set(Idx);
     }
   }
 
   // When an operand becomes reachable, we potentially activate some predicates.
   // Clear the dependencies that are now satisfied as a result of this
   // operand and activate any predicates whose dependencies are fully
   // satisfied.
-  for (auto &Dep : enumerate(MatchDag.predicate_edges())) {
+  for (const auto &Dep : enumerate(MatchDag.predicate_edges())) {
     if (Dep.value()->getRequiredMI() == Instr && Dep.value()->getRequiredMO() &&
         Dep.value()->getRequiredMO()->getIdx() == OpIdx) {
-      for (auto &DepsFor : enumerate(UnsatisfiedPredDepsForPred)) {
+      for (const auto &DepsFor : enumerate(UnsatisfiedPredDepsForPred)) {
         DepsFor.value().reset(Dep.index());
         if (DepsFor.value().none())
           TestablePredicates.set(DepsFor.index());
       }
     }
   }
 }
 

 
 void GIMatchTreeOpcodePartitioner::applyForPartition(
     unsigned PartitionIdx, GIMatchTreeBuilder &Builder, GIMatchTreeBuilder &SubBuilder) {
   LLVM_DEBUG(dbgs() << "  Making partition " << PartitionIdx << "\n");
   const CodeGenInstruction *CGI = PartitionToInstr[PartitionIdx];
 
   BitVector PossibleLeaves = getPossibleLeavesForPartition(PartitionIdx);
   // Consume any predicates we handled.
-  for (auto &EnumeratedLeaf : enumerate(Builder.getPossibleLeaves())) {
-    if (!PossibleLeaves[EnumeratedLeaf.index()])
+  for (const auto &[Index, EnumeratedLeaf] :
+       enumerate(Builder.getPossibleLeaves())) {
+    if (!PossibleLeaves[Index])
       continue;
 
-    auto &Leaf = EnumeratedLeaf.value();
-    const auto &TestedPredicatesForLeaf =
-        TestedPredicates[EnumeratedLeaf.index()];
+    const auto &TestedPredicatesForLeaf = TestedPredicates[Index];
 
     for (unsigned PredIdx : TestedPredicatesForLeaf.set_bits()) {
-      LLVM_DEBUG(dbgs() << "    " << Leaf.getName() << " tested predicate #"
-                        << PredIdx << " of " << TestedPredicatesForLeaf.size()
-                        << " " << *Leaf.getPredicate(PredIdx) << "\n");
-      Leaf.RemainingPredicates.reset(PredIdx);
-      Leaf.TestablePredicates.reset(PredIdx);
+      LLVM_DEBUG(dbgs() << "    " << EnumeratedLeaf.getName()
+                        << " tested predicate #" << PredIdx << " of "
+                        << TestedPredicatesForLeaf.size() << " "
+                        << *EnumeratedLeaf.getPredicate(PredIdx) << "\n");
+      EnumeratedLeaf.RemainingPredicates.reset(PredIdx);
+      EnumeratedLeaf.TestablePredicates.reset(PredIdx);
     }
-    SubBuilder.addLeaf(Leaf);
+    SubBuilder.addLeaf(EnumeratedLeaf);
   }
 
   // Nothing to do, we don't know anything about this instruction as a result
   // of this partitioner.
   if (CGI == nullptr)
     return;
 
   GIMatchTreeBuilder::LeafVec &NewLeaves = SubBuilder.getPossibleLeaves();
   // Find all the operands we know to exist and are referenced. This will
   // usually be all the referenced operands but there are some cases where
   // instructions are variadic. Such operands must be handled by partitioners
   // that check the number of operands.
   BitVector ReferencedOperands(1);
   for (auto &Leaf : NewLeaves) {
     GIMatchTreeInstrInfo *InstrInfo = Leaf.getInstrInfo(InstrID);
     // Skip any leaves that don't care about this instruction.
     if (!InstrInfo)
       continue;
     const GIMatchDagInstr *Instr = InstrInfo->getInstrNode();
-    for (auto &E : enumerate(Leaf.getMatchDag().edges())) {
-      if (E.value()->getFromMI() == Instr &&
-          E.value()->getFromMO()->getIdx() < CGI->Operands.size()) {
-        ReferencedOperands.resize(E.value()->getFromMO()->getIdx() + 1);
-        ReferencedOperands.set(E.value()->getFromMO()->getIdx());
+    for (const auto &E : Leaf.getMatchDag().edges()) {
+      if (E->getFromMI() == Instr &&
+          E->getFromMO()->getIdx() < CGI->Operands.size()) {
+        ReferencedOperands.resize(E->getFromMO()->getIdx() + 1);
+        ReferencedOperands.set(E->getFromMO()->getIdx());
       }
     }
   }
   for (auto &Leaf : NewLeaves) {
     // Skip any leaves that don't care about this instruction.
     if (!Leaf.getInstrInfo(InstrID))
       continue;
 

 
 void GIMatchTreeVRegDefPartitioner::applyForPartition(
     unsigned PartitionIdx, GIMatchTreeBuilder &Builder,
     GIMatchTreeBuilder &SubBuilder) {
   BitVector PossibleLeaves = getPossibleLeavesForPartition(PartitionIdx);
 
   std::vector<BitVector> TraversedEdgesByNewLeaves;
   // Consume any edges we handled.
-  for (auto &EnumeratedLeaf : enumerate(Builder.getPossibleLeaves())) {
-    if (!PossibleLeaves[EnumeratedLeaf.index()])
+  for (const auto &[Index, EnumeratedLeaf] :
+       enumerate(Builder.getPossibleLeaves())) {
+    if (!PossibleLeaves[Index])
       continue;
 
-    auto &Leaf = EnumeratedLeaf.value();
-    const auto &TraversedEdgesForLeaf = TraversedEdges[EnumeratedLeaf.index()];
+    const auto &TraversedEdgesForLeaf = TraversedEdges[Index];
     TraversedEdgesByNewLeaves.push_back(TraversedEdgesForLeaf);
-    Leaf.RemainingEdges.reset(TraversedEdgesForLeaf);
-    Leaf.TraversableEdges.reset(TraversedEdgesForLeaf);
-    SubBuilder.addLeaf(Leaf);
+    EnumeratedLeaf.RemainingEdges.reset(TraversedEdgesForLeaf);
+    EnumeratedLeaf.TraversableEdges.reset(TraversedEdgesForLeaf);
+    SubBuilder.addLeaf(EnumeratedLeaf);
   }
 
   // Nothing to do. The only thing we know is that it isn't a vreg-def.
   if (PartitionToResult[PartitionIdx] == false)
     return;
 
   NewInstrID = SubBuilder.allocInstrID();