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

Jakub Kuderski via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 17:59:44 PDT 2023


Author: Jakub Kuderski
Date: 2023-03-13T20:59:06-04:00
New Revision: b9db89fbcfdaece8656159a2a0f0a2f09cdd7db7

URL: https://github.com/llvm/llvm-project/commit/b9db89fbcfdaece8656159a2a0f0a2f09cdd7db7
DIFF: https://github.com/llvm/llvm-project/commit/b9db89fbcfdaece8656159a2a0f0a2f09cdd7db7.diff

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

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.

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D145987

Added: 
    

Modified: 
    clang/tools/clang-refactor/TestSupport.cpp
    llvm/lib/ObjectYAML/DWARFYAML.cpp
    llvm/lib/ObjectYAML/MinidumpEmitter.cpp
    llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
    llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp
    llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
    llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp

Removed: 
    


################################################################################
diff  --git a/clang/tools/clang-refactor/TestSupport.cpp b/clang/tools/clang-refactor/TestSupport.cpp
index 400313eeab5e5..3fae18c2109a6 100644
--- a/clang/tools/clang-refactor/TestSupport.cpp
+++ b/clang/tools/clang-refactor/TestSupport.cpp
@@ -176,11 +176,11 @@ std::pair<unsigned, unsigned> getLineColumn(StringRef Filename,
 
 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;

diff  --git a/llvm/lib/ObjectYAML/DWARFYAML.cpp b/llvm/lib/ObjectYAML/DWARFYAML.cpp
index 37116ada99015..2bddeed464135 100644
--- a/llvm/lib/ObjectYAML/DWARFYAML.cpp
+++ b/llvm/lib/ObjectYAML/DWARFYAML.cpp
@@ -59,22 +59,20 @@ 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();
     }
   }
 

diff  --git a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
index 1bda6f364b1bd..24b521a9925c7 100644
--- a/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpEmitter.cpp
@@ -236,8 +236,8 @@ bool yaml2minidump(MinidumpYAML::Object &Obj, raw_ostream &Out,
   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;

diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 1d479bb111b13..8256832b64e87 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -349,16 +349,16 @@ enum class UseMask {
 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;
 }
@@ -3959,7 +3959,7 @@ bool clusterSortPtrAccesses(ArrayRef<Value *> VL, Type *ElemTy,
         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;
       });
     }

diff  --git a/llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp b/llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp
index 10bbc64340325..d3572e2f5abf7 100644
--- a/llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp
+++ b/llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp
@@ -101,9 +101,9 @@ static void extractArgumentsFromModule(Oracle &O, ReducerWorkItem &WorkItem) {
       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

diff  --git a/llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp b/llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
index 135e7f854ecef..0686b47c69d52 100644
--- a/llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
+++ b/llvm/unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp
@@ -222,7 +222,7 @@ TEST_F(RandomAccessVisitorTest, MultipleVisits) {
 
   // 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()));
 }
 
@@ -251,7 +251,7 @@ TEST_F(RandomAccessVisitorTest, DescendingWithinChunk) {
 
   // 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()));
 }
 
@@ -280,7 +280,7 @@ TEST_F(RandomAccessVisitorTest, AscendingWithinChunk) {
 
   // 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()));
 }
 
@@ -311,7 +311,7 @@ TEST_F(RandomAccessVisitorTest, StopPrematurelyInChunk) {
 
   // [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()));
 }
 
@@ -341,7 +341,7 @@ TEST_F(RandomAccessVisitorTest, InnerChunk) {
 
   // 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()));
 }
 

diff  --git a/llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp b/llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp
index d98884493e840..493c6b35c8406 100644
--- a/llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp
+++ b/llvm/utils/TableGen/GlobalISel/GIMatchTree.cpp
@@ -89,20 +89,20 @@ GIMatchTreeBuilderLeafInfo::GIMatchTreeBuilderLeafInfo(
       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);
   }
 }
 
@@ -134,10 +134,10 @@ void GIMatchTreeBuilderLeafInfo::declareInstr(const GIMatchDagInstr *Instr, unsi
   // 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());
@@ -157,10 +157,9 @@ void GIMatchTreeBuilderLeafInfo::declareOperand(unsigned InstrID,
   // 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);
     }
   }
 
@@ -168,10 +167,10 @@ void GIMatchTreeBuilderLeafInfo::declareOperand(unsigned InstrID,
   // 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());
@@ -536,22 +535,22 @@ void GIMatchTreeOpcodePartitioner::applyForPartition(
 
   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
@@ -571,11 +570,11 @@ void GIMatchTreeOpcodePartitioner::applyForPartition(
     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());
       }
     }
   }
@@ -715,16 +714,16 @@ void GIMatchTreeVRegDefPartitioner::applyForPartition(
 
   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.


        


More information about the llvm-commits mailing list