[Mangling] Include private discriminators in constructor manglings. (#9880)

Previously, two constructors with the same full name and argument
types would get identical manglings even if they were declared
'private' or 'fileprivate' in different files. This would lead to
symbol collisions in whole-module builds. Add a new mangling node for
private discriminators on base-name-less decls to make this unique.

This still doesn't fix the existing issue with private members, named
or not, conflicting when they're in the /same/ file, but since Swift 4
makes those members visible to one another (SE-0169) that's only an
issue in Swift 3 mode anyway, and as such probably won't get fixed at
all.

rdar://problem/27758199

Author: jrose-apple

docs/ABI.rst

@@ -877,8 +877,8 @@ Entities
   curry-thunk ::= 'Tc'
 
   // The leading type is the function type
-  entity-spec ::= type 'fC'                  // allocating constructor
-  entity-spec ::= type 'fc'                  // non-allocating constructor
+  entity-spec ::= type file-discriminator? 'fC'      // allocating constructor
+  entity-spec ::= type file-discriminator? 'fc'      // non-allocating constructor
   entity-spec ::= type 'fU' INDEX            // explicit anonymous closure expression
   entity-spec ::= type 'fu' INDEX            // implicit anonymous closure
   entity-spec ::= 'fA' INDEX                 // default argument N+1 generator
@@ -911,14 +911,16 @@ Entities
 
   decl-name ::= identifier
   decl-name ::= identifier 'L' INDEX         // locally-discriminated declaration
-  decl-name ::= identifier identifier 'LL'    // file-discriminated declaration
-
-The first identifier in a file-discriminated ``<decl-name>>`` is a string that
-represents the file the original declaration came from.
-It should be considered unique within the enclosing module.
-The second identifier is the name of the entity.
-Not all declarations marked ``private`` declarations will use this mangling;
-if the entity's context is enough to uniquely identify the entity, the simple
+  decl-name ::= identifier identifier 'LL'   // file-discriminated declaration
+
+  file-discriminator ::= identifier 'Ll'     // anonymous file-discriminated declaration
+
+The identifier in a ``<file-discriminator>`` and the first identifier in a
+file-discriminated ``<decl-name>`` is a string that represents the file the
+original declaration came from. It should be considered unique within the
+enclosing module. The second identifier is the name of the entity. Not all
+declarations marked ``private`` declarations will use this mangling; if the
+entity's context is enough to uniquely identify the entity, the simple
 ``identifier`` form is preferred.
 
 Declaration Contexts

lib/AST/ASTMangler.cpp

@@ -501,6 +501,29 @@ static unsigned getUnnamedParamIndex(const ParamDecl *D) {
   llvm_unreachable("param not found");
 }
 
+static StringRef getPrivateDiscriminatorIfNecessary(const ValueDecl *decl) {
+  if (!decl->hasAccessibility() ||
+      decl->getFormalAccess() > Accessibility::FilePrivate ||
+      isInPrivateOrLocalContext(decl)) {
+    return StringRef();
+  }
+
+  // Mangle non-local private declarations with a textual discriminator
+  // based on their enclosing file.
+  auto topLevelContext = decl->getDeclContext()->getModuleScopeContext();
+  auto fileUnit = cast<FileUnit>(topLevelContext);
+
+  Identifier discriminator =
+      fileUnit->getDiscriminatorForPrivateValue(decl);
+  assert(!discriminator.empty());
+  assert(!isNonAscii(discriminator.str()) &&
+         "discriminator contains non-ASCII characters");
+  (void)&isNonAscii;
+  assert(!clang::isDigit(discriminator.str().front()) &&
+         "not a valid identifier");
+  return discriminator.str();
+}
+
 void ASTMangler::appendDeclName(const ValueDecl *decl) {
   if (decl->isOperator()) {
     auto name = decl->getBaseName().getIdentifier().str();
@@ -536,27 +559,10 @@ void ASTMangler::appendDeclName(const ValueDecl *decl) {
     // Mangle local declarations with a numeric discriminator.
     return appendOperator("L", Index(decl->getLocalDiscriminator()));
   }
-  if (decl->hasAccessibility() &&
-      decl->getFormalAccess() <= Accessibility::FilePrivate &&
-      !isInPrivateOrLocalContext(decl)) {
-    // Mangle non-local private declarations with a textual discriminator
-    // based on their enclosing file.
-
-    // The first <identifier> is a discriminator string unique to the decl's
-    // original source file.
-    auto topLevelContext = decl->getDeclContext()->getModuleScopeContext();
-    auto fileUnit = cast<FileUnit>(topLevelContext);
-
-    Identifier discriminator =
-      fileUnit->getDiscriminatorForPrivateValue(decl);
-    assert(!discriminator.empty());
-    assert(!isNonAscii(discriminator.str()) &&
-           "discriminator contains non-ASCII characters");
-    (void)&isNonAscii;
-    assert(!clang::isDigit(discriminator.str().front()) &&
-           "not a valid identifier");
-
-    appendIdentifier(discriminator.str());
+
+  StringRef privateDiscriminator = getPrivateDiscriminatorIfNecessary(decl);
+  if (!privateDiscriminator.empty()) {
+    appendIdentifier(privateDiscriminator.str());
     return appendOperator("LL");
   }
 }
@@ -1802,6 +1808,11 @@ void ASTMangler::appendConstructorEntity(const ConstructorDecl *ctor,
                                          bool isAllocating) {
   appendContextOf(ctor);
   appendDeclType(ctor);
+  StringRef privateDiscriminator = getPrivateDiscriminatorIfNecessary(ctor);
+  if (!privateDiscriminator.empty()) {
+    appendIdentifier(privateDiscriminator);
+    appendOperator("Ll");
+  }
   appendOperator(isAllocating ? "fC" : "fc");
 }
 

lib/Demangling/Demangler.cpp

@@ -641,6 +641,10 @@ NodePointer Demangler::demangleLocalIdentifier() {
     NodePointer name = popNode(isDeclName);
     return createWithChildren(Node::Kind::PrivateDeclName, discriminator, name);
   }
+  if (nextIf('l')) {
+    NodePointer discriminator = popNode(Node::Kind::Identifier);
+    return createWithChild(Node::Kind::PrivateDeclName, discriminator);
+  }
   NodePointer discriminator = demangleIndexAsNode();
   NodePointer name = popNode(isDeclName);
   return createWithChildren(Node::Kind::LocalDeclName, discriminator, name);
@@ -1672,7 +1676,14 @@ NodePointer Demangler::demangleMetatypeRepresentation() {
 }
 
 NodePointer Demangler::demangleFunctionEntity() {
-  enum { None, Type, TypeAndName, TypeAndIndex, Index } Args;
+  enum {
+    None,
+    Type,
+    TypeAndName,
+    TypeAndMaybePrivateName,
+    TypeAndIndex,
+    Index
+  } Args;
 
   Node::Kind Kind = Node::Kind::EmptyList;
   switch (nextChar()) {
@@ -1681,8 +1692,10 @@ NodePointer Demangler::demangleFunctionEntity() {
     case 'E': Args = None; Kind = Node::Kind::IVarDestroyer; break;
     case 'e': Args = None; Kind = Node::Kind::IVarInitializer; break;
     case 'i': Args = None; Kind = Node::Kind::Initializer; break;
-    case 'C': Args = Type; Kind = Node::Kind::Allocator; break;
-    case 'c': Args = Type; Kind = Node::Kind::Constructor; break;
+    case 'C':
+      Args = TypeAndMaybePrivateName; Kind = Node::Kind::Allocator; break;
+    case 'c':
+      Args = TypeAndMaybePrivateName; Kind = Node::Kind::Constructor; break;
     case 'g': Args = TypeAndName; Kind = Node::Kind::Getter; break;
     case 'G': Args = TypeAndName; Kind = Node::Kind::GlobalGetter; break;
     case 's': Args = TypeAndName; Kind = Node::Kind::Setter; break;
@@ -1727,6 +1740,10 @@ NodePointer Demangler::demangleFunctionEntity() {
       Child2 = popNode(Node::Kind::Type);
       Child1 = popNode(isDeclName);
       break;
+    case TypeAndMaybePrivateName:
+      Child1 = popNode(Node::Kind::PrivateDeclName);
+      Child2 = popNode(Node::Kind::Type);
+      break;
     case TypeAndIndex:
       Child1 = demangleIndexAsNode();
       Child2 = popNode(Node::Kind::Type);
@@ -1743,6 +1760,11 @@ NodePointer Demangler::demangleFunctionEntity() {
     case Index:
       Entity = addChild(Entity, Child1);
       break;
+    case TypeAndMaybePrivateName:
+      if (Child1)
+        Entity = addChild(Entity, Child1);
+      Entity = addChild(Entity, Child2);
+      break;
     case TypeAndName:
     case TypeAndIndex:
       Entity = addChild(Entity, Child1);

lib/Demangling/NodePrinter.cpp

@@ -867,13 +867,19 @@ NodePointer NodePrinter::print(NodePointer Node, bool asPrefixContext) {
     Printer << " #" << (Node->getChild(0)->getIndex() + 1);
     return nullptr;
   case Node::Kind::PrivateDeclName:
-    if (Options.ShowPrivateDiscriminators)
-      Printer << '(';
+    if (Node->getNumChildren() > 1) {
+      if (Options.ShowPrivateDiscriminators)
+        Printer << '(';
 
-    print(Node->getChild(1));
+      print(Node->getChild(1));
 
-    if (Options.ShowPrivateDiscriminators)
-      Printer << " in " << Node->getChild(0)->getText() << ')';
+      if (Options.ShowPrivateDiscriminators)
+        Printer << " in " << Node->getChild(0)->getText() << ')';
+    } else {
+      if (Options.ShowPrivateDiscriminators) {
+        Printer << "(in " << Node->getChild(0)->getText() << ')';
+      }
+    }
     return nullptr;
   case Node::Kind::Module:
     if (Options.DisplayModuleNames)
@@ -1472,7 +1478,7 @@ NodePointer NodePrinter::print(NodePointer Node, bool asPrefixContext) {
                                          "__allocating_init" : "init");
   case Node::Kind::Constructor:
     return printEntity(Node, asPrefixContext, TypePrinting::FunctionStyle,
-                       /*hasName*/false, "init");
+                       /*hasName*/Node->getNumChildren() > 2, "init");
   case Node::Kind::Destructor:
     return printEntity(Node, asPrefixContext, TypePrinting::NoType,
                        /*hasName*/false, "deinit");
@@ -1771,8 +1777,9 @@ printEntity(NodePointer Entity, bool asPrefixContext, TypePrinting TypePr,
       Printer << " of ";
       ExtraName = "";
     }
+    size_t CurrentPos = Printer.getStringRef().size();
     print(Entity->getChild(1));
-    if (!ExtraName.empty())
+    if (Printer.getStringRef().size() != CurrentPos && !ExtraName.empty())
       Printer << '.';
   }
   if (!ExtraName.empty()) {

lib/Demangling/Remangler.cpp

@@ -273,6 +273,7 @@ class Remangler {
   void mangleAnyNominalType(Node *node);
   void mangleAnyGenericType(Node *node, char TypeOp);
   void mangleGenericArgs(Node *node, char &Separator);
+  void mangleAnyConstructor(Node *node, char kindOp);
 
 #define NODE(ID)                                                        \
   void mangle##ID(Node *node);
@@ -478,8 +479,7 @@ void Remangler::mangleGenericArgs(Node *node, char &Separator) {
 }
 
 void Remangler::mangleAllocator(Node *node) {
-  mangleChildNodes(node);
-  Buffer << "fC";
+  mangleAnyConstructor(node, 'C');
 }
 
 void Remangler::mangleArgumentTuple(Node *node) {
@@ -600,9 +600,18 @@ void Remangler::mangleClass(Node *node) {
   mangleAnyNominalType(node);
 }
 
+void Remangler::mangleAnyConstructor(Node *node, char kindOp) {
+  mangleChildNode(node, 0);
+  if (node->getNumChildren() > 2) {
+    assert(node->getNumChildren() == 3);
+    mangleChildNode(node, 2);
+  }
+  mangleChildNode(node, 1);
+  Buffer << "f" << kindOp;
+}
+
 void Remangler::mangleConstructor(Node *node) {
-  mangleChildNodes(node);
-  Buffer << "fc";
+  mangleAnyConstructor(node, 'c');
 }
 
 void Remangler::mangleDeallocator(Node *node) {
@@ -1385,7 +1394,7 @@ void Remangler::manglePrefixOperator(Node *node) {
 
 void Remangler::manglePrivateDeclName(Node *node) {
   mangleChildNodesReversed(node);
-  Buffer << "LL";
+  Buffer << (node->getNumChildren() == 1 ? "Ll" : "LL");
 }
 
 void Remangler::mangleProtocol(Node *node) {

test/Demangle/Inputs/manglings.txt

@@ -259,4 +259,4 @@ _T0Tk ---> _T0Tk
 _T0A8 ---> _T0A8
 _T0s30ReversedRandomAccessCollectionVyxGTfq3nnpf_nTfq1cn_nTfq4x_n ---> _T0s30ReversedRandomAccessCollectionVyxGTfq3nnpf_nTfq1cn_nTfq4x_n
 _T03abc6testitySiFTm ---> merged abc.testit(Swift.Int) -> ()
-
+_T04main4TestCACSi1x_tc6_PRIV_Llfc ---> main.Test.(in _PRIV_).init(x: Swift.Int) -> main.Test

test/Demangle/Inputs/simplified-manglings.txt

@@ -204,4 +204,4 @@ _TTRXFo_oCSo13SKPhysicsBodydVSC7CGPointdVSC8CGVectordGSpV10ObjectiveC8ObjCBool__
 _T0So13SKPhysicsBodyCSC7CGPointVSC8CGVectorVSpy10ObjectiveC8ObjCBoolVGIxxyyy_AbdFSpyAIGIyByyyy_TR ---> thunk for @callee_owned (@owned SKPhysicsBody, @unowned CGPoint, @unowned CGVector, @unowned UnsafeMutablePointer<ObjCBool>) -> ()
 _T04main1_yyF ---> _()
 _T03abc6testitySiFTm ---> testit(_:)
-
+_T04main4TestCACSi1x_tc6_PRIV_Llfc ---> Test.init(x:)

test/SILGen/mangling_private.swift

@@ -21,20 +21,32 @@ private func privateFunc() -> Int {
 public struct PublicStruct {
   // CHECK-LABEL: sil private @_T016mangling_private12PublicStructV0B6Method33_A3CCBB841DB59E79A4AD4EE458655068LLyyFZ
   private static func privateMethod() {}
+
+  // CHECK-LABEL: sil private @_T016mangling_private12PublicStructVACSi1x_tc33_A3CCBB841DB59E79A4AD4EE458655068LlfC
+  private init(x: Int) {}
 }
 
 public struct InternalStruct {
   // CHECK-LABEL: sil private @_T016mangling_private14InternalStructV0B6Method33_A3CCBB841DB59E79A4AD4EE458655068LLyyFZ
   private static func privateMethod() {}
+
+  // CHECK-LABEL: sil private @_T016mangling_private14InternalStructVACSi1x_tc33_A3CCBB841DB59E79A4AD4EE458655068LlfC
+  private init(x: Int) {}
 }
 
 private struct PrivateStruct {
   // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLV0B6MethodyyFZ
   private static func privateMethod() {}
 
+  // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLVADSi1x_tcfC
+  private init(x: Int) {}
+
   struct Inner {
     // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLV5InnerV0B6MethodyyFZ
     private static func privateMethod() {}
+
+    // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLV5InnerVAFSi1x_tcfC
+    private init(x: Int) {}
   }
 }
 
@@ -47,10 +59,16 @@ func localTypes() {
 extension PublicStruct {
   // CHECK-LABEL: sil private @_T016mangling_private12PublicStructV16extPrivateMethod33_A3CCBB841DB59E79A4AD4EE458655068LLyyF
   private func extPrivateMethod() {}
+
+  // CHECK-LABEL: sil private @_T016mangling_private12PublicStructVACSi3ext_tc33_A3CCBB841DB59E79A4AD4EE458655068LlfC
+  private init(ext: Int) {}
 }
 extension PrivateStruct {
   // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLV03extC6MethodyyF
   private func extPrivateMethod() {}
+
+  // CHECK-LABEL: sil private @_T016mangling_private13PrivateStruct33_A3CCBB841DB59E79A4AD4EE458655068LLVADSi3ext_tcfC
+  private init(ext: Int) {}
 }
 
 // CHECK-LABEL: sil private @_T016mangling_private10localTypesyyF11LocalStructL_V0B6MethodyyFZ