Merge pull request #22200 from slavapestov/fix-remangler

Remangler round-tripping fixes

Author: slavapestov

include/swift/AST/ASTMangler.h

@@ -76,6 +76,15 @@ class ASTMangler : public Mangler {
   ASTMangler(bool DWARFMangling = false)
     : DWARFMangling(DWARFMangling) {}
 
+  void addTypeSubstitution(Type type) {
+    type = dropProtocolsFromAssociatedTypes(type);
+    addSubstitution(type.getPointer());
+  }
+  bool tryMangleTypeSubstitution(Type type) {
+    type = dropProtocolsFromAssociatedTypes(type);
+    return tryMangleSubstitution(type.getPointer());
+  }
+
   std::string mangleClosureEntity(const AbstractClosureExpr *closure,
                                   SymbolKind SKind);
 
@@ -261,6 +270,9 @@ class ASTMangler : public Mangler {
                                    unsigned initialParamDepth,
                                    ArrayRef<Requirement> requirements);
 
+  DependentMemberType *dropProtocolFromAssociatedType(DependentMemberType *dmt);
+  Type dropProtocolsFromAssociatedTypes(Type type);
+
   void appendAssociatedTypeName(DependentMemberType *dmt);
 
   void appendClosureEntity(const SerializedAbstractClosureExpr *closure);

include/swift/Demangling/Demangle.h

@@ -564,7 +564,10 @@ const char *getNodeKindString(swift::Demangle::Node::Kind k);
 /// Useful for debugging.
 std::string getNodeTreeAsString(NodePointer Root);
 
+bool nodeConsumesGenericArgs(Node *node);
+
 bool isSpecialized(Node *node);
+
 NodePointer getUnspecialized(Node *node, NodeFactory &Factory);
 std::string archetypeName(Node::IndexType index, Node::IndexType depth);
 

lib/AST/ASTMangler.cpp

@@ -737,15 +737,15 @@ void ASTMangler::appendType(Type type) {
 
       if (aliasTy->getSubstitutionMap().hasAnySubstitutableParams()) {
         // Try to mangle the entire name as a substitution.
-        if (tryMangleSubstitution(tybase))
+        if (tryMangleTypeSubstitution(tybase))
           return;
 
         appendAnyGenericType(decl);
         bool isFirstArgList = true;
         appendBoundGenericArgs(type, isFirstArgList);
         appendRetroactiveConformances(type);
         appendOperator("G");
-        addSubstitution(type.getPointer());
+        addTypeSubstitution(type);
         return;
       }
 
@@ -840,7 +840,7 @@ void ASTMangler::appendType(Type type) {
         Decl = type->getAnyGeneric();
       if (shouldMangleAsGeneric(type)) {
         // Try to mangle the entire name as a substitution.
-        if (tryMangleSubstitution(tybase))
+        if (tryMangleTypeSubstitution(tybase))
           return;
 
         if (isStdlibType(Decl) && Decl->getName().str() == "Optional") {
@@ -855,7 +855,7 @@ void ASTMangler::appendType(Type type) {
           appendRetroactiveConformances(type);
           appendOperator("G");
         }
-        addSubstitution(type.getPointer());
+        addTypeSubstitution(type);
         return;
       }
       appendAnyGenericType(type->getAnyGeneric());
@@ -901,7 +901,7 @@ void ASTMangler::appendType(Type type) {
 
     case TypeKind::DependentMember: {
       auto *DepTy = cast<DependentMemberType>(tybase);
-      if (tryMangleSubstitution(DepTy))
+      if (tryMangleTypeSubstitution(DepTy))
         return;
 
       bool isAssocTypeAtDepth = false;
@@ -920,7 +920,7 @@ void ASTMangler::appendType(Type type) {
         appendAssociatedTypeName(DepTy);
         appendOperator("qa");
       }
-      addSubstitution(DepTy);
+      addTypeSubstitution(DepTy);
       return;
     }
 
@@ -1633,7 +1633,7 @@ void ASTMangler::appendAnyGenericType(const GenericTypeDecl *decl) {
   }
 
   // Try to mangle the entire name as a substitution.
-  if (tryMangleSubstitution(key.getPointer()))
+  if (tryMangleTypeSubstitution(key))
     return;
 
   // Try to mangle a symbolic reference for a nominal type.
@@ -1642,7 +1642,7 @@ void ASTMangler::appendAnyGenericType(const GenericTypeDecl *decl) {
     if (nominal && (!CanSymbolicReference || CanSymbolicReference(nominal))) {
       appendSymbolicReference(nominal);
       // Substitutions can refer back to the symbolic reference.
-      addSubstitution(key.getPointer());
+      addTypeSubstitution(key);
       return;
     }
   }
@@ -1705,7 +1705,7 @@ void ASTMangler::appendAnyGenericType(const GenericTypeDecl *decl) {
     }
   }
 
-  addSubstitution(key.getPointer());
+  addTypeSubstitution(key);
 }
 
 void ASTMangler::appendFunction(AnyFunctionType *fn, bool isFunctionMangling) {
@@ -1941,7 +1941,7 @@ void ASTMangler::appendRequirement(const Requirement &reqt) {
 
   if (auto *DT = FirstTy->getAs<DependentMemberType>()) {
     bool isAssocTypeAtDepth = false;
-    if (tryMangleSubstitution(DT)) {
+    if (tryMangleTypeSubstitution(DT)) {
       switch (reqt.getKind()) {
         case RequirementKind::Conformance:
           return appendOperator("RQ");
@@ -1957,7 +1957,7 @@ void ASTMangler::appendRequirement(const Requirement &reqt) {
       llvm_unreachable("bad requirement type");
     }
     GenericTypeParamType *gpBase = appendAssocType(DT, isAssocTypeAtDepth);
-    addSubstitution(DT);
+    addTypeSubstitution(DT);
     assert(gpBase);
     switch (reqt.getKind()) {
       case RequirementKind::Conformance:
@@ -2043,24 +2043,54 @@ void ASTMangler::appendGenericSignatureParts(
   appendOperator("r", StringRef(OpStorage.data(), OpStorage.size()));
 }
 
+// If the base type is known to have a single protocol conformance
+// in the current generic context, then we don't need to disambiguate the
+// associated type name by protocol.
+DependentMemberType *
+ASTMangler::dropProtocolFromAssociatedType(DependentMemberType *dmt) {
+  auto baseTy = dmt->getBase();
+  bool unambiguous = (!dmt->getAssocType() ||
+                      CurGenericSignature->getConformsTo(baseTy).size() <= 1);
+
+  if (auto *baseDMT = baseTy->getAs<DependentMemberType>())
+    baseTy = dropProtocolFromAssociatedType(baseDMT);
+
+  if (unambiguous)
+    return DependentMemberType::get(baseTy, dmt->getName());
+
+  return DependentMemberType::get(baseTy, dmt->getAssocType());
+}
+
+Type
+ASTMangler::dropProtocolsFromAssociatedTypes(Type type) {
+  if (!OptimizeProtocolNames || !CurGenericSignature)
+    return type;
+
+  if (!type->hasDependentMember())
+    return type;
+
+  return type.transform([&](Type t) -> Type {
+    if (auto *dmt = dyn_cast<DependentMemberType>(t.getPointer()))
+      return dropProtocolFromAssociatedType(dmt);
+    return t;
+  });
+}
+
 void ASTMangler::appendAssociatedTypeName(DependentMemberType *dmt) {
-  auto assocTy = dmt->getAssocType();
-
-  // If the base type is known to have a single protocol conformance
-  // in the current generic context, then we don't need to disambiguate the
-  // associated type name by protocol.
-  // This can result in getting the same mangled string for different
-  // DependentMemberTypes. This is not a problem but re-mangling might do more
-  // aggressive substitutions, which means that the re-mangled name may differ
-  // from the original mangled name.
-  // FIXME: We ought to be able to get to the generic signature from a
-  // dependent type, but can't yet. Shouldn't need this side channel.
-
-  appendIdentifier(assocTy->getName().str());
-  if (!OptimizeProtocolNames || !CurGenericSignature
-      || CurGenericSignature->getConformsTo(dmt->getBase()).size() > 1) {
-    appendAnyGenericType(assocTy->getProtocol());
+  if (auto assocTy = dmt->getAssocType()) {
+    appendIdentifier(assocTy->getName().str());
+
+    // If the base type is known to have a single protocol conformance
+    // in the current generic context, then we don't need to disambiguate the
+    // associated type name by protocol.
+    if (!OptimizeProtocolNames || !CurGenericSignature
+        || CurGenericSignature->getConformsTo(dmt->getBase()).size() > 1) {
+      appendAnyGenericType(assocTy->getProtocol());
+    }
+    return;
   }
+
+  appendIdentifier(dmt->getName().str());
 }
 
 void ASTMangler::appendClosureEntity(

lib/Basic/Mangler.cpp

@@ -180,19 +180,9 @@ void Mangler::verify(StringRef nameStr) {
   if (Remangled == nameStr)
     return;
 
-  // There are cases (e.g. with dependent associated types) which results in
-  // different remangled names. See ASTMangler::appendAssociatedTypeName.
-  // This is no problem for the compiler, but we have to be more tolerant for
-  // those cases. Instead we try to re-de-mangle the remangled name.
-  NodePointer RootOfRemangled = Dem.demangleSymbol(Remangled);
-  std::string ReDemangled = mangleNode(RootOfRemangled);
-  if (Remangled == ReDemangled)
-    return;
-
   llvm::errs() << "Remangling failed:\n"
                   "original     = " << nameStr << "\n"
-                  "remangled    = " << Remangled << "\n"
-                  "re-demangled = " << ReDemangled << '\n';
+                  "remangled    = " << Remangled << "\n";
   abort();
 #endif
 }

lib/Demangling/Demangler.cpp

@@ -1458,6 +1458,20 @@ NodePointer Demangler::demangleBoundGenericType() {
   return NTy;
 }
 
+bool Demangle::nodeConsumesGenericArgs(Node *node) {
+  switch (node->getKind()) {
+    case Node::Kind::Variable:
+    case Node::Kind::Subscript:
+    case Node::Kind::ImplicitClosure:
+    case Node::Kind::ExplicitClosure:
+    case Node::Kind::DefaultArgumentInitializer:
+    case Node::Kind::Initializer:
+      return false;
+    default:
+      return true;
+  }
+}
+
 NodePointer Demangler::demangleBoundGenericArgs(NodePointer Nominal,
                                     const Vector<NodePointer> &TypeLists,
                                     size_t TypeListIdx) {
@@ -1493,18 +1507,7 @@ NodePointer Demangler::demangleBoundGenericArgs(NodePointer Nominal,
     return nullptr;
   NodePointer Context = Nominal->getFirstChild();
 
-  bool consumesGenericArgs = true;
-  switch (Nominal->getKind()) {
-    case Node::Kind::Variable:
-    case Node::Kind::ExplicitClosure:
-    case Node::Kind::Subscript:
-      // Those nodes can appear in the context but do not "consume" a generic
-      // argument type list.
-      consumesGenericArgs = false;
-      break;
-    default:
-      break;
-  }
+  bool consumesGenericArgs = nodeConsumesGenericArgs(Nominal);
 
   NodePointer args = TypeLists[TypeListIdx];