TypeDecoder: Change createTupleType() to take labels as ArrayRef<StringRef> instead of std::string

Author: slavapestov

include/swift/AST/ASTDemangler.h

@@ -110,7 +110,7 @@ class ASTBuilder {
   Type createBoundGenericType(GenericTypeDecl *decl, ArrayRef<Type> args,
                               Type parent);
 
-  Type createTupleType(ArrayRef<Type> eltTypes, StringRef labels);
+  Type createTupleType(ArrayRef<Type> eltTypes, ArrayRef<StringRef> labels);
 
   Type createPackType(ArrayRef<Type> eltTypes);
 

include/swift/Demangling/TypeDecoder.h

@@ -1022,7 +1022,8 @@ class TypeDecoder {
 
     case NodeKind::Tuple: {
       llvm::SmallVector<BuiltType, 8> elements;
-      std::string labels;
+      llvm::SmallVector<StringRef, 8> labels;
+
       for (auto &element : *Node) {
         if (element->getKind() != NodeKind::TupleElement)
           return MAKE_NODE_TYPE_ERROR0(Node, "unexpected kind");
@@ -1034,18 +1035,12 @@ class TypeDecoder {
                                        "no children");
         }
         if (element->getChild(typeChildIndex)->getKind() == NodeKind::TupleElementName) {
-          // Add spaces to terminate all the previous labels if this
-          // is the first we've seen.
-          if (labels.empty()) labels.append(elements.size(), ' ');
-
-          // Add the label and its terminator.
-          labels += element->getChild(typeChildIndex)->getText();
-          labels += ' ';
+          labels.push_back(element->getChild(typeChildIndex)->getText());
           typeChildIndex++;
 
-        // Otherwise, add a space if a previous element had a label.
-        } else if (!labels.empty()) {
-          labels += ' ';
+        // Otherwise, add an empty label.
+        } else {
+          labels.push_back(StringRef());
         }
 
         // Decode the element type.
@@ -1057,7 +1052,7 @@ class TypeDecoder {
 
         elements.push_back(elementType.getType());
       }
-      return Builder.createTupleType(elements, std::move(labels));
+      return Builder.createTupleType(elements, labels);
     }
     case NodeKind::TupleElement:
       if (Node->getNumChildren() < 1)

include/swift/Remote/MetadataReader.h

@@ -871,13 +871,26 @@ class MetadataReader {
       }
 
       // Read the labels string.
-      std::string labels;
+      std::string labelStr;
       if (tupleMeta->Labels &&
-          !Reader->readString(RemoteAddress(tupleMeta->Labels), labels))
+          !Reader->readString(RemoteAddress(tupleMeta->Labels), labelStr))
         return BuiltType();
 
+      std::vector<llvm::StringRef> labels;
+      std::string::size_type end, start = 0;
+      while (true) {
+        end = labelStr.find(' ', start);
+        if (end == std::string::npos)
+          break;
+        labels.push_back(llvm::StringRef(labelStr.data() + start, end - start));
+        start = end + 1;
+      }
+      // Pad the vector with empty labels.
+      for (unsigned i = labels.size(); i < elementTypes.size(); ++i)
+        labels.push_back(StringRef());
+
       auto BuiltTuple =
-          Builder.createTupleType(elementTypes, std::move(labels));
+          Builder.createTupleType(elementTypes, labels);
       TypeCache[MetadataAddress] = BuiltTuple;
       return BuiltTuple;
     }

include/swift/RemoteInspection/TypeRef.h

@@ -83,7 +83,7 @@ class TypeRefID {
     Bits.push_back(Integer >> 32);
   }
 
-  void addString(const std::string &String) {
+  void addString(llvm::StringRef String) {
     if (String.empty()) {
       Bits.push_back(0);
     } else {
@@ -364,47 +364,34 @@ class BoundGenericTypeRef final : public TypeRef, public NominalTypeTrait {
 class TupleTypeRef final : public TypeRef {
 protected:
   std::vector<const TypeRef *> Elements;
-  std::string Labels;
+  std::vector<StringRef> Labels;
 
   static TypeRefID Profile(const std::vector<const TypeRef *> &Elements,
-                           const std::string &Labels) {
+                           const std::vector<StringRef> &Labels) {
     TypeRefID ID;
     for (auto Element : Elements)
       ID.addPointer(Element);
-    ID.addString(Labels);
+    for (auto Label : Labels)
+      ID.addString(Label);
     return ID;
   }
 
 public:
-  TupleTypeRef(std::vector<const TypeRef *> Elements, std::string &&Labels)
+  TupleTypeRef(std::vector<const TypeRef *> Elements,
+               std::vector<StringRef> Labels)
       : TypeRef(TypeRefKind::Tuple), Elements(std::move(Elements)),
-        Labels(Labels) {}
+        Labels(std::move(Labels)) {}
 
   template <typename Allocator>
   static const TupleTypeRef *create(Allocator &A,
                                     std::vector<const TypeRef *> Elements,
-                                    std::string &&Labels) {
+                                    const std::vector<StringRef> Labels) {
     FIND_OR_CREATE_TYPEREF(A, TupleTypeRef, Elements, Labels);
   }
 
   const std::vector<const TypeRef *> &getElements() const { return Elements; };
-  const std::string &getLabelString() const { return Labels; };
-  std::vector<llvm::StringRef> getLabels() const {
-    std::vector<llvm::StringRef> Vec;
-    std::string::size_type End, Start = 0;
-    while (true) {
-      End = Labels.find(' ', Start);
-      if (End == std::string::npos)
-        break;
-      Vec.push_back(llvm::StringRef(Labels.data() + Start, End - Start));
-      Start = End + 1;
-    }
-    // A canonicalized TypeRef has an empty label string.
-    // Pad the vector with empty labels.
-    for (unsigned N = Vec.size(); N < Elements.size(); ++N)
-      Vec.push_back({});
-    return Vec;
-  };
+
+  const std::vector<llvm::StringRef> &getLabels() const { return Labels; }
 
   static bool classof(const TypeRef *TR) {
     return TR->getKind() == TypeRefKind::Tuple;

include/swift/RemoteInspection/TypeRefBuilder.h

@@ -743,8 +743,8 @@ class TypeRefBuilder {
   }
 
   const TupleTypeRef *createTupleType(llvm::ArrayRef<const TypeRef *> elements,
-                                      std::string &&labels) {
-    return TupleTypeRef::create(*this, elements, std::move(labels));
+                                      llvm::ArrayRef<StringRef> labels) {
+    return TupleTypeRef::create(*this, elements, labels);
   }
 
   const TypeRef *createPackType(llvm::ArrayRef<const TypeRef *> elements) {
@@ -825,7 +825,7 @@ class TypeRefBuilder {
       break;
     }
 
-    auto result = createTupleType({}, "");
+    auto result = createTupleType({}, llvm::ArrayRef<llvm::StringRef>());
     return FunctionTypeRef::create(
         *this, {}, result, funcFlags, diffKind, nullptr);
   }

lib/AST/ASTDemangler.cpp

@@ -327,18 +327,14 @@ Type ASTBuilder::createBoundGenericType(GenericTypeDecl *decl,
   return aliasDecl->getDeclaredInterfaceType().subst(subMap);
 }
 
-Type ASTBuilder::createTupleType(ArrayRef<Type> eltTypes, StringRef labels) {
+Type ASTBuilder::createTupleType(ArrayRef<Type> eltTypes, ArrayRef<StringRef> labels) {
   SmallVector<TupleTypeElt, 4> elements;
   elements.reserve(eltTypes.size());
-  for (auto eltType : eltTypes) {
+  for (unsigned i : indices(eltTypes)) {
     Identifier label;
-    if (!labels.empty()) {
-      auto split = labels.split(' ');
-      if (!split.first.empty())
-        label = Ctx.getIdentifier(split.first);
-      labels = split.second;
-    }
-    elements.emplace_back(eltType, label);
+    if (!labels[i].empty())
+      label = Ctx.getIdentifier(labels[i]);
+    elements.emplace_back(eltTypes[i], label);
   }
 
   return TupleType::get(elements, Ctx);

stdlib/public/RemoteInspection/TypeRef.cpp

@@ -111,7 +111,7 @@ class PrintTypeRef : public TypeRefVisitor<PrintTypeRef, void> {
 
   void visitTupleTypeRef(const TupleTypeRef *T) {
     printHeader("tuple");
-    T->getLabels();
+
     auto Labels = T->getLabels();
     for (auto NameElement : llvm::zip_first(Labels, T->getElements())) {
       auto Label = std::get<0>(NameElement);
@@ -1133,8 +1133,8 @@ class ThickenMetatype
     std::vector<const TypeRef *> Elements;
     for (auto Element : T->getElements())
       Elements.push_back(visit(Element));
-    std::string Labels = T->getLabelString();
-    return TupleTypeRef::create(Builder, Elements, std::move(Labels));
+    auto Labels = T->getLabels();
+    return TupleTypeRef::create(Builder, Elements, Labels);
   }
 
   const TypeRef *visitFunctionTypeRef(const FunctionTypeRef *F) {
@@ -1268,8 +1268,8 @@ class TypeRefSubstitution
     std::vector<const TypeRef *> Elements;
     for (auto Element : T->getElements())
       Elements.push_back(visit(Element));
-    std::string Labels = T->getLabelString();
-    return TupleTypeRef::create(Builder, Elements, std::move(Labels));
+    auto Labels = T->getLabels();
+    return TupleTypeRef::create(Builder, Elements, Labels);
   }
 
   const TypeRef *visitFunctionTypeRef(const FunctionTypeRef *F) {

stdlib/public/runtime/MetadataLookup.cpp

@@ -20,6 +20,7 @@
 #include "Tracing.h"
 #include "swift/ABI/TypeIdentity.h"
 #include "swift/Basic/Lazy.h"
+#include "swift/Basic/Range.h"
 #include "swift/Demangling/Demangler.h"
 #include "swift/Demangling/TypeDecoder.h"
 #include "swift/RemoteInspection/Records.h"
@@ -1944,22 +1945,40 @@ class DecodedMetadataBuilder {
 
   TypeLookupErrorOr<BuiltType>
   createTupleType(llvm::ArrayRef<BuiltType> elements,
-                  std::string labels) const {
+                  llvm::ArrayRef<StringRef> labels) const {
     for (auto element : elements) {
       if (!element.isMetadata()) {
         return TYPE_LOOKUP_ERROR_FMT("Tried to build a tuple type where "
                                      "an element type is a pack");
       }
     }
 
+    std::string labelStr;
+    for (unsigned i : indices(labels)) {
+      auto label = labels[i];
+      if (label.empty()) {
+        if (!labelStr.empty())
+          labelStr += ' ';
+        continue;
+      }
+
+      // Add spaces to terminate all the previous labels if this
+      // is the first we've seen.
+      if (labelStr.empty()) labelStr.append(i, ' ');
+
+      // Add the label and its terminator.
+      labelStr += label;
+      labelStr += ' ';
+    }
+
     auto flags = TupleTypeFlags().withNumElements(elements.size());
-    if (!labels.empty())
+    if (!labelStr.empty())
       flags = flags.withNonConstantLabels(true);
     return BuiltType(
         swift_getTupleTypeMetadata(
           MetadataState::Abstract, flags,
           reinterpret_cast<const Metadata * const *>(elements.data()),
-          labels.empty() ? nullptr : labels.c_str(),
+          labelStr.empty() ? nullptr : labelStr.c_str(),
           /*proposedWitnesses=*/nullptr));
   }
 

unittests/Reflection/TypeRef.cpp

@@ -99,17 +99,17 @@ TEST(TypeRefTest, UniqueTupleTypeRef) {
   auto N2 = Builder.createNominalType(XYZ_decl, nullptr);
 
   std::vector<const TypeRef *> Void;
-  auto Void1 = Builder.createTupleType(Void, "");
-  auto Void2 = Builder.createTupleType(Void, "");
+  auto Void1 = Builder.createTupleType(Void, ArrayRef<StringRef>());
+  auto Void2 = Builder.createTupleType(Void, ArrayRef<StringRef>());
 
   EXPECT_EQ(Void1, Void2);
 
   std::vector<const TypeRef *> Elements1 { N1, N2 };
   std::vector<const TypeRef *> Elements2 { N1, N2, N2 };
 
-  auto T1 = Builder.createTupleType(Elements1, "");
-  auto T2 = Builder.createTupleType(Elements1, "");
-  auto T3 = Builder.createTupleType(Elements2, "");
+  auto T1 = Builder.createTupleType(Elements1, ArrayRef<StringRef>());
+  auto T2 = Builder.createTupleType(Elements1, ArrayRef<StringRef>());
+  auto T3 = Builder.createTupleType(Elements2, ArrayRef<StringRef>());
 
   EXPECT_EQ(T1, T2);
   EXPECT_NE(T2, T3);
@@ -121,7 +121,7 @@ TEST(TypeRefTest, UniqueFunctionTypeRef) {
   TypeRefBuilder Builder(TypeRefBuilder::ForTesting);
 
   std::vector<const TypeRef *> Void;
-  auto VoidResult = Builder.createTupleType(Void, "");
+  auto VoidResult = Builder.createTupleType(Void, ArrayRef<StringRef>());
   Param Param1 = Builder.createNominalType(ABC_decl, nullptr);
   Param Param2 = Builder.createNominalType(XYZ_decl, nullptr);
 
@@ -130,7 +130,8 @@ TEST(TypeRefTest, UniqueFunctionTypeRef) {
   std::vector<Param> Parameters2{Param1, Param1};
 
   auto Result =
-      Builder.createTupleType({Param1.getType(), Param2.getType()}, "");
+      Builder.createTupleType({Param1.getType(), Param2.getType()},
+                              ArrayRef<StringRef>());
 
   auto F1 = Builder.createFunctionType(
       Parameters1, Result, FunctionTypeFlags(),
@@ -517,7 +518,8 @@ TEST(TypeRefTest, DeriveSubstitutions) {
   auto Nominal = Builder.createBoundGenericType(NominalName, NominalArgs,
                                                /*parent*/ nullptr);
 
-  auto Result = Builder.createTupleType({GTP00, GTP01}, "");
+  auto Result = Builder.createTupleType({GTP00, GTP01},
+                                        ArrayRef<StringRef>());
   auto Func = Builder.createFunctionType(
       {Nominal}, Result, FunctionTypeFlags(),
       FunctionMetadataDifferentiabilityKind::NonDifferentiable, nullptr);