[NFC][TableGen] Code cleanup in Record.h/cpp

[jurahul]

• Use range for loops in several places.
• Change some variable names to conform to LLVM coding standard.
• Use ListSeparator instead of manual code to generate comma interleaved strings.
• Remove unnecessary copies in SETDAGOP evaluation in BinOpInit::Fold.
• Eliminate duplicated code in the 2 overloads of RecordVal::setValue.
• Use explicit type in some range for loops.
• Tested by verifying that all the .inc files generated by building all the *CommonTableGen targets stay unchanged.

[jurahul]

I got a clean Windows but Linux has some flang failures (unrelated) so rerunning the checks but started the review as well.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

• Use range for loops in several places.
• Change some variable names to conform to LLVM coding standard.
• Use ListSeparator instead of manual code to generate comma interleaved strings.
• Remove unnecessary copies in SETDAGOP evaluation in BinOpInit::Fold.
• Eliminate duplicated code in the 2 overloads of RecordVal::setValue.
• Use explicit type in some range for loops.

---

Patch is 24.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138876.diff

2 Files Affected:

• (modified) llvm/include/llvm/TableGen/Record.h (+17-25)
• (modified) llvm/lib/TableGen/Record.cpp (+117-179)

diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index 982cc255553a2..d7d744aeec956 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -19,6 +19,7 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -244,7 +245,7 @@ class RecordRecTy final : public RecTy,
   RecordRecTy &operator=(const RecordRecTy &) = delete;
 
   // Do not use sized deallocation due to trailing objects.
-  void operator delete(void *p) { ::operator delete(p); }
+  void operator delete(void *Ptr) { ::operator delete(Ptr); }
 
   static bool classof(const RecTy *RT) {
     return RT->getRecTyKind() == RecordRecTyKind;
@@ -598,7 +599,7 @@ class BitsInit final : public TypedInit,
   BitsInit &operator=(const BitsInit &) = delete;
 
   // Do not use sized deallocation due to trailing objects.
-  void operator delete(void *p) { ::operator delete(p); }
+  void operator delete(void *Ptr) { ::operator delete(Ptr); }
 
   static bool classof(const Init *I) {
     return I->getKind() == IK_BitsInit;
@@ -615,18 +616,8 @@ class BitsInit final : public TypedInit,
   convertInitializerBitRange(ArrayRef<unsigned> Bits) const override;
   std::optional<int64_t> convertInitializerToInt() const;
 
-  bool isComplete() const override {
-    for (unsigned i = 0; i != getNumBits(); ++i)
-      if (!getBit(i)->isComplete()) return false;
-    return true;
-  }
-
-  bool allInComplete() const {
-    for (unsigned i = 0; i != getNumBits(); ++i)
-      if (getBit(i)->isComplete()) return false;
-    return true;
-  }
-
+  bool isComplete() const override;
+  bool allInComplete() const;
   bool isConcrete() const override;
   std::string getAsString() const override;
 
@@ -773,7 +764,7 @@ class ListInit final : public TypedInit,
   ListInit &operator=(const ListInit &) = delete;
 
   // Do not use sized deallocation due to trailing objects.
-  void operator delete(void *p) { ::operator delete(p); }
+  void operator delete(void *Ptr) { ::operator delete(Ptr); }
 
   static bool classof(const Init *I) {
     return I->getKind() == IK_ListInit;
@@ -786,13 +777,13 @@ class ListInit final : public TypedInit,
     return ArrayRef(getTrailingObjects<const Init *>(), NumValues);
   }
 
-  const Init *getElement(unsigned Index) const { return getValues()[Index]; }
+  const Init *getElement(unsigned Idx) const { return getValues()[Idx]; }
 
   const RecTy *getElementType() const {
     return cast<ListRecTy>(getType())->getElementType();
   }
 
-  const Record *getElementAsRecord(unsigned i) const;
+  const Record *getElementAsRecord(unsigned Idx) const;
 
   const Init *convertInitializerTo(const RecTy *Ty) const override;
 
@@ -1060,6 +1051,8 @@ class CondOpInit final : public TypedInit,
     return ArrayRef(getTrailingObjects<const Init *>() + NumConds, NumConds);
   }
 
+  auto getCondAndVals() const { return zip_equal(getConds(), getVals()); }
+
   const Init *Fold(const Record *CurRec) const;
 
   const Init *resolveReferences(Resolver &R) const override;
@@ -1349,7 +1342,7 @@ class VarDefInit final
   VarDefInit &operator=(const VarDefInit &) = delete;
 
   // Do not use sized deallocation due to trailing objects.
-  void operator delete(void *p) { ::operator delete(p); }
+  void operator delete(void *Ptr) { ::operator delete(Ptr); }
 
   static bool classof(const Init *I) {
     return I->getKind() == IK_VarDefInit;
@@ -1495,6 +1488,8 @@ class DagInit final
     return ArrayRef(getTrailingObjects<const StringInit *>(), NumArgs);
   }
 
+  auto getArgAndNames() const { return zip_equal(getArgs(), getArgNames()); }
+
   const Init *resolveReferences(Resolver &R) const override;
 
   bool isConcrete() const override;
@@ -1798,9 +1793,9 @@ class Record {
   }
 
   void removeValue(const Init *Name) {
-    for (unsigned i = 0, e = Values.size(); i != e; ++i)
-      if (Values[i].getNameInit() == Name) {
-        Values.erase(Values.begin()+i);
+    for (auto [Idx, Value] : enumerate(Values))
+      if (Value.getNameInit() == Name) {
+        Values.erase(Values.begin() + Idx);
         return;
       }
     llvm_unreachable("Cannot remove an entry that does not exist!");
@@ -2123,10 +2118,7 @@ struct LessRecordRegister {
 
     size_t size() { return Parts.size(); }
 
-    std::pair<bool, StringRef> getPart(size_t i) {
-      assert (i < Parts.size() && "Invalid idx!");
-      return Parts[i];
-    }
+    std::pair<bool, StringRef> getPart(size_t Idx) { return Parts[Idx]; }
   };
 
   bool operator()(const Record *Rec1, const Record *Rec2) const {
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index f3d54e6083e48..3df6a32b6b9fd 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -294,11 +294,9 @@ std::string RecordRecTy::getAsString() const {
     return getClasses()[0]->getNameInitAsString();
 
   std::string Str = "{";
-  bool First = true;
+  ListSeparator LS;
   for (const Record *R : getClasses()) {
-    if (!First)
-      Str += ", ";
-    First = false;
+    Str += LS;
     Str += R->getNameInitAsString();
   }
   Str += "}";
@@ -520,9 +518,9 @@ const Init *BitsInit::convertInitializerTo(const RecTy *Ty) const {
 
 std::optional<int64_t> BitsInit::convertInitializerToInt() const {
   int64_t Result = 0;
-  for (unsigned i = 0, e = getNumBits(); i != e; ++i)
-    if (auto *Bit = dyn_cast<BitInit>(getBit(i)))
-      Result |= static_cast<int64_t>(Bit->getValue()) << i;
+  for (auto [Idx, InitV] : enumerate(getBits()))
+    if (auto *Bit = dyn_cast<BitInit>(InitV))
+      Result |= static_cast<int64_t>(Bit->getValue()) << Idx;
     else
       return std::nullopt;
   return Result;
@@ -532,27 +530,30 @@ const Init *
 BitsInit::convertInitializerBitRange(ArrayRef<unsigned> Bits) const {
   SmallVector<const Init *, 16> NewBits(Bits.size());
 
-  for (unsigned i = 0, e = Bits.size(); i != e; ++i) {
-    if (Bits[i] >= getNumBits())
+  for (auto [Bit, NewBit] : zip_equal(Bits, NewBits)) {
+    if (Bit >= getNumBits())
       return nullptr;
-    NewBits[i] = getBit(Bits[i]);
+    NewBit = getBit(Bit);
   }
   return BitsInit::get(getRecordKeeper(), NewBits);
 }
 
+bool BitsInit::isComplete() const {
+  return all_of(getBits(), [](const Init *Bit) { return Bit->isComplete(); });
+}
+bool BitsInit::allInComplete() const {
+  return all_of(getBits(), [](const Init *Bit) { return !Bit->isComplete(); });
+}
 bool BitsInit::isConcrete() const {
-  for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
-    if (!getBit(i)->isConcrete())
-      return false;
-  }
-  return true;
+  return all_of(getBits(), [](const Init *Bit) { return Bit->isConcrete(); });
 }
 
 std::string BitsInit::getAsString() const {
   std::string Result = "{ ";
-  for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
-    if (i) Result += ", ";
-    if (const Init *Bit = getBit(e - i - 1))
+  ListSeparator LS;
+  for (const Init *Bit : reverse(getBits())) {
+    Result += LS;
+    if (Bit)
       Result += Bit->getAsString();
     else
       Result += "*";
@@ -569,9 +570,8 @@ const Init *BitsInit::resolveReferences(Resolver &R) const {
   const Init *CachedBitVarRef = nullptr;
   const Init *CachedBitVarResolved = nullptr;
 
-  for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
-    const Init *CurBit = getBit(i);
-    const Init *NewBit = CurBit;
+  for (auto [CurBit, NewBit] : zip_equal(getBits(), NewBits)) {
+    NewBit = CurBit;
 
     if (const auto *CurBitVar = dyn_cast<VarBitInit>(CurBit)) {
       if (CurBitVar->getBitVar() != CachedBitVarRef) {
@@ -587,7 +587,6 @@ const Init *BitsInit::resolveReferences(Resolver &R) const {
 
     if (isa<UnsetInit>(NewBit) && R.keepUnsetBits())
       NewBit = CurBit;
-    NewBits[i] = NewBit;
     Changed |= CurBit != NewBit;
   }
 
@@ -644,12 +643,11 @@ const Init *IntInit::convertInitializerTo(const RecTy *Ty) const {
 const Init *IntInit::convertInitializerBitRange(ArrayRef<unsigned> Bits) const {
   SmallVector<const Init *, 16> NewBits(Bits.size());
 
-  for (unsigned i = 0, e = Bits.size(); i != e; ++i) {
-    if (Bits[i] >= 64)
+  for (auto [Bit, NewBit] : zip_equal(Bits, NewBits)) {
+    if (Bit >= 64)
       return nullptr;
 
-    NewBits[i] =
-        BitInit::get(getRecordKeeper(), Value & (INT64_C(1) << Bits[i]));
+    NewBit = BitInit::get(getRecordKeeper(), Value & (INT64_C(1) << Bit));
   }
   return BitsInit::get(getRecordKeeper(), NewBits);
 }
@@ -751,8 +749,9 @@ const Init *ListInit::convertInitializerTo(const RecTy *Ty) const {
         Elements.push_back(CI);
         if (CI != I)
           Changed = true;
-      } else
+      } else {
         return nullptr;
+      }
 
     if (!Changed)
       return this;
@@ -762,9 +761,8 @@ const Init *ListInit::convertInitializerTo(const RecTy *Ty) const {
   return nullptr;
 }
 
-const Record *ListInit::getElementAsRecord(unsigned i) const {
-  assert(i < NumValues && "List element index out of range!");
-  const auto *DI = dyn_cast<DefInit>(getElement(i));
+const Record *ListInit::getElementAsRecord(unsigned Idx) const {
+  const auto *DI = dyn_cast<DefInit>(getElement(Idx));
   if (!DI)
     PrintFatalError("Expected record in list!");
   return DI->getDef();
@@ -787,27 +785,20 @@ const Init *ListInit::resolveReferences(Resolver &R) const {
 }
 
 bool ListInit::isComplete() const {
-  for (const Init *Element : *this) {
-    if (!Element->isComplete())
-      return false;
-  }
-  return true;
+  return all_of(*this,
+                [](const Init *Element) { return Element->isComplete(); });
 }
 
 bool ListInit::isConcrete() const {
-  for (const Init *Element : *this) {
-    if (!Element->isConcrete())
-      return false;
-  }
-  return true;
+  return all_of(*this,
+                [](const Init *Element) { return Element->isConcrete(); });
 }
 
 std::string ListInit::getAsString() const {
   std::string Result = "[";
-  const char *sep = "";
+  ListSeparator LS;
   for (const Init *Element : *this) {
-    Result += sep;
-    sep = ", ";
+    Result += LS;
     Result += Element->getAsString();
   }
   return Result + "]";
@@ -1125,9 +1116,9 @@ static const StringInit *interleaveStringList(const ListInit *List,
   SmallString<80> Result(Element->getValue());
   StringInit::StringFormat Fmt = StringInit::SF_String;
 
-  for (unsigned I = 1, E = List->size(); I < E; ++I) {
+  for (const Init *Elem : List->getValues().drop_front()) {
     Result.append(Delim->getValue());
-    const auto *Element = dyn_cast<StringInit>(List->getElement(I));
+    const auto *Element = dyn_cast<StringInit>(Elem);
     if (!Element)
       return nullptr;
     Result.append(Element->getValue());
@@ -1147,10 +1138,10 @@ static const StringInit *interleaveIntList(const ListInit *List,
     return nullptr;
   SmallString<80> Result(Element->getAsString());
 
-  for (unsigned I = 1, E = List->size(); I < E; ++I) {
+  for (const Init *Elem : List->getValues().drop_front()) {
     Result.append(Delim->getValue());
     const auto *Element = dyn_cast_or_null<IntInit>(
-        List->getElement(I)->convertInitializerTo(IntRecTy::get(RK)));
+        Elem->convertInitializerTo(IntRecTy::get(RK)));
     if (!Element)
       return nullptr;
     Result.append(Element->getAsString());
@@ -1317,13 +1308,11 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
 
       SmallVector<const Init *, 8> Args;
       SmallVector<const StringInit *, 8> ArgNames;
-      for (unsigned i = 0, e = LHSs->getNumArgs(); i != e; ++i) {
-        Args.push_back(LHSs->getArg(i));
-        ArgNames.push_back(LHSs->getArgName(i));
-      }
-      for (unsigned i = 0, e = RHSs->getNumArgs(); i != e; ++i) {
-        Args.push_back(RHSs->getArg(i));
-        ArgNames.push_back(RHSs->getArgName(i));
+      for (const DagInit *Dag : {LHSs, RHSs}) {
+        for (auto [Arg, ArgName] : Dag->getArgAndNames()) {
+          Args.push_back(Arg);
+          ArgNames.push_back(ArgName);
+        }
       }
       return DagInit::get(Op, nullptr, Args, ArgNames);
     }
@@ -1422,8 +1411,8 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
     if (!LHSi || !RHSi)
       break;
 
-    auto Start = LHSi->getValue();
-    auto End = RHSi->getValue();
+    int64_t Start = LHSi->getValue();
+    int64_t End = RHSi->getValue();
     SmallVector<const Init *, 8> Args;
     if (getOpcode() == RANGEC) {
       // Closed interval
@@ -1519,15 +1508,8 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
   case SETDAGOP: {
     const auto *Dag = dyn_cast<DagInit>(LHS);
     const auto *Op = dyn_cast<DefInit>(RHS);
-    if (Dag && Op) {
-      SmallVector<const Init *, 8> Args;
-      SmallVector<const StringInit *, 8> ArgNames;
-      for (unsigned i = 0, e = Dag->getNumArgs(); i != e; ++i) {
-        Args.push_back(Dag->getArg(i));
-        ArgNames.push_back(Dag->getArgName(i));
-      }
-      return DagInit::get(Op, nullptr, Args, ArgNames);
-    }
+    if (Dag && Op)
+      return DagInit::get(Op, nullptr, Dag->getArgs(), Dag->getArgNames());
     break;
   }
   case ADD:
@@ -1696,10 +1678,8 @@ static const Init *ForeachDagApply(const Init *LHS, const DagInit *MHSd,
     Change = true;
 
   SmallVector<std::pair<const Init *, const StringInit *>, 8> NewArgs;
-  for (unsigned int i = 0; i < MHSd->getNumArgs(); ++i) {
-    const Init *Arg = MHSd->getArg(i);
+  for (auto [Arg, ArgName] : MHSd->getArgAndNames()) {
     const Init *NewArg;
-    const StringInit *ArgName = MHSd->getArgName(i);
 
     if (const auto *Argd = dyn_cast<DagInit>(Arg))
       NewArg = ForeachDagApply(LHS, Argd, RHS, CurRec);
@@ -1937,9 +1917,9 @@ const Init *TernOpInit::Fold(const Record *CurRec) const {
       assert(*ArgNo < Dag->getNumArgs());
 
       SmallVector<const Init *, 8> Args(Dag->getArgs());
-      SmallVector<const StringInit *, 8> Names(Dag->getArgNames());
       Args[*ArgNo] = RHS;
-      return DagInit::get(Dag->getOperator(), Dag->getName(), Args, Names);
+      return DagInit::get(Dag->getOperator(), Dag->getName(), Args,
+                          Dag->getArgNames());
     }
     break;
   }
@@ -1954,10 +1934,10 @@ const Init *TernOpInit::Fold(const Record *CurRec) const {
 
       assert(*ArgNo < Dag->getNumArgs());
 
-      SmallVector<const Init *, 8> Args(Dag->getArgs());
       SmallVector<const StringInit *, 8> Names(Dag->getArgNames());
       Names[*ArgNo] = dyn_cast<StringInit>(RHS);
-      return DagInit::get(Dag->getOperator(), Dag->getName(), Args, Names);
+      return DagInit::get(Dag->getOperator(), Dag->getName(), Dag->getArgs(),
+                          Names);
     }
     break;
   }
@@ -2546,10 +2526,9 @@ const Init *VarDefInit::Fold() const {
 
 std::string VarDefInit::getAsString() const {
   std::string Result = Class->getNameInitAsString() + "<";
-  const char *sep = "";
+  ListSeparator LS;
   for (const Init *Arg : args()) {
-    Result += sep;
-    sep = ", ";
+    Result += LS;
     Result += Arg->getAsString();
   }
   return Result + ">";
@@ -2648,15 +2627,14 @@ const CondOpInit *CondOpInit::get(ArrayRef<const Init *> Conds,
 
 const Init *CondOpInit::resolveReferences(Resolver &R) const {
   SmallVector<const Init *, 4> NewConds;
+  SmallVector<const Init *, 4> NewVals;
+
   bool Changed = false;
-  for (const Init *Case : getConds()) {
-    const Init *NewCase = Case->resolveReferences(R);
-    NewConds.push_back(NewCase);
-    Changed |= NewCase != Case;
-  }
+  for (auto [Cond, Val] : getCondAndVals()) {
+    const Init *NewCond = Cond->resolveReferences(R);
+    NewConds.push_back(NewCond);
+    Changed |= NewCond != Cond;
 
-  SmallVector<const Init *, 4> NewVals;
-  for (const Init *Val : getVals()) {
     const Init *NewVal = Val->resolveReferences(R);
     NewVals.push_back(NewVal);
     Changed |= NewVal != Val;
@@ -2671,10 +2649,7 @@ const Init *CondOpInit::resolveReferences(Resolver &R) const {
 
 const Init *CondOpInit::Fold(const Record *CurRec) const {
   RecordKeeper &RK = getRecordKeeper();
-  for (unsigned i = 0; i < NumConds; ++i) {
-    const Init *Cond = getCond(i);
-    const Init *Val = getVal(i);
-
+  for (auto [Cond, Val] : getCondAndVals()) {
     if (const auto *CondI = dyn_cast_or_null<IntInit>(
             Cond->convertInitializerTo(IntRecTy::get(RK)))) {
       if (CondI->getValue())
@@ -2692,36 +2667,24 @@ const Init *CondOpInit::Fold(const Record *CurRec) const {
 }
 
 bool CondOpInit::isConcrete() const {
-  for (const Init *Case : getConds())
-    if (!Case->isConcrete())
-      return false;
-
-  for (const Init *Val : getVals())
-    if (!Val->isConcrete())
-      return false;
-
-  return true;
+  return all_of(getCondAndVals(), [](const auto &Pair) {
+    return std::get<0>(Pair)->isConcrete() && std::get<1>(Pair)->isConcrete();
+  });
 }
 
 bool CondOpInit::isComplete() const {
-  for (const Init *Case : getConds())
-    if (!Case->isComplete())
-      return false;
-
-  for (const Init *Val : getVals())
-    if (!Val->isConcrete())
-      return false;
-
-  return true;
+  return all_of(getCondAndVals(), [](const auto &Pair) {
+    return std::get<0>(Pair)->isComplete() && std::get<1>(Pair)->isComplete();
+  });
 }
 
 std::string CondOpInit::getAsString() const {
   std::string Result = "!cond(";
-  for (unsigned i = 0; i < getNumConds(); i++) {
-    Result += getCond(i)->getAsString() + ": ";
-    Result += getVal(i)->getAsString();
-    if (i != getNumConds()-1)
-      Result += ", ";
+  ListSeparator LS;
+  for (auto [Cond, Val] : getCondAndVals()) {
+    Result += LS;
+    Result += Cond->getAsString() + ": ";
+    Result += Val->getAsString();
   }
   return Result + ")";
 }
@@ -2731,20 +2694,15 @@ const Init *CondOpInit::getBit(unsigned Bit) const {
 }
 
 static void ProfileDagInit(FoldingSetNodeID &ID, const Init *V,
-                           const StringInit *VN,
-                           ArrayRef<const Init *> ArgRange,
-                           ArrayRef<const StringInit *> NameRange) {
+                           const StringInit *VN, ArrayRef<const Init *> Args,
+                           ArrayRef<const StringInit *> ArgNames) {
   ID.AddPointer(V);
   ID.AddPointer(VN);
 
-  ArrayRef<const Init *>::iterator Arg = ArgRange.begin();
-  ArrayRef<const StringInit *>::iterator Name = NameRange.begin();
-  while (Arg != ArgRange.end()) {
-    assert(Name != NameRange.end() && "Arg name underflow!");
-    ID.AddPointer(*Arg++);
-    ID.AddPointer(*Name++);
+  for (auto [Arg, Name] : zip_equal(Args, ArgNames)) {
+    ID.AddPointer(Arg);
+    ID.AddPointer(Name);
   }
-  assert(Name == NameRange.end() && "Arg name overflow!");
 }
 
 DagInit::DagInit(const Init *V, const StringInit *VN,
@@ -2805,10 +2763,9 @@ const Record *DagInit::getOperatorAsDef(ArrayRef<SMLoc> Loc) const {
 }
 
 std::optional<unsigned> DagInit::getArgNo(StringRef Name) const {
-  for (unsigned i = 0, e = getNumArgs(); i < e; ++i) {
-    const StringInit *ArgName = getArgName(i);
+  for (auto [Idx, ArgName] : enumerate(getArgNames())) {
     if (ArgName && ArgName->getValue() == Name)
-      return i;
+      return Idx;
   }
   return std::nullopt;
 }
@@ -2833,11 +2790,7 @@ const Init *DagInit::resolveReferences(Resolver &R) const {
 bool DagInit::isConcrete() const {
   if (!Val->isConcrete())
     return false;
-  for (const Init *Elt : getArgs()) {
-    if (!Elt->isConcrete())
-      return false;
-  }
-  return true;
+  return all_of(getArgs(), [](const Init *Elt) { return Elt->isConcrete(); });
 }
 
 std::string DagInit::getAsString() const {
@@ -2845,11 +2798,13 @@ std::string DagInit::getAsString() const {
   if (ValName)
     Result += ":" + ValName->getAsUnquotedString();
   if (!arg_empty()) {
-    Result += " " + getArg(0)->getAsString();
-    if (getArgName(0)) Result += ":$" + getArgName(0)->getAsUnquotedString();
-    for (unsigned i = 1, e = getNumArgs(); i != e; ++i) {
-      Result += ", " + getArg(i)->getAsString();
-      if (getArgName(i)) Result += ":$" + getArgName(i)->getAsUnquotedString();
+    Result += " ";
+    ListSeparator LS;
+    for (auto [Arg, Name] : getArgAndNames()) {
+      Result += LS;
+      Result += Arg->getAsString();
+      if (Name)
+        Result += ":$" + Name->getAsUnquotedString();
     }
   }
   return Result + ")";
@@ -2893,24 +2848,27 @@ std::string RecordVal::getPrintType() const {
 }
 
 bool RecordVa...
[truncated]

[wangpc-pp]

LGTM.

[jurahul]

Thanks. @topperc and @s-barannikov will wait for a few days to give you guys a chance to review as well.