Implementation of "Restrict cross-module struct initializers to be delegating" (#2)

docs/SIL.rst

@@ -2308,6 +2308,7 @@ mark_uninitialized
   sil-instruction ::= 'mark_uninitialized' '[' mu_kind ']' sil-operand
   mu_kind ::= 'var'
   mu_kind ::= 'rootself'
+  mu_kind ::= 'crossmodulerootself'
   mu_kind ::= 'derivedself'
   mu_kind ::= 'derivedselfonly'
   mu_kind ::= 'delegatingself'
@@ -2325,6 +2326,9 @@ the mark_uninitialized instruction refers to:
 
 - ``var``: designates the start of a normal variable live range
 - ``rootself``: designates ``self`` in a struct, enum, or root class
+- ``crossmodulerootself``: same as ``rootself``, but in a case where it's not
+    really safe to treat ``self`` as a root because the original module might add
+    more stored properties. This is only used for Swift 4 compatibility.
 - ``derivedself``: designates ``self`` in a derived (non-root) class
 - ``derivedselfonly``: designates ``self`` in a derived (non-root) class whose stored properties have already been initialized
 - ``delegatingself``: designates ``self`` on a struct, enum, or class in a delegating constructor (one that calls self.init)

include/swift/AST/DiagnosticsSIL.def

@@ -215,6 +215,14 @@ ERROR(mutating_protocol_witness_method_on_let_constant,none,
       "cannot perform mutating operation: '%0' is a 'let' constant",
       (StringRef))
 
+WARNING(designated_init_in_cross_module_extension,none,
+        "initializer for struct %0 must use \"self.init(...)\" or \"self = ...\""
+        "%select{| on all paths}1 because "
+        "%select{it is not in module %2|the struct was imported from C}3",
+        (Type, bool, Identifier, bool))
+NOTE(designated_init_c_struct_fix,none,
+     "use \"self.init()\" to initialize the struct with zero values", ())
+
 
 // Control flow diagnostics.
 ERROR(missing_return,none,

include/swift/AST/DiagnosticsSema.def

@@ -2842,6 +2842,10 @@ NOTE(change_to_mutating,none,
      "mark %select{method|accessor}0 'mutating' to make 'self' mutable",
      (bool))
 
+ERROR(assignment_let_property_delegating_init,none,
+      "'let' property %0 may not be initialized directly; use "
+      "\"self.init(...)\" or \"self = ...\" instead", (DeclName))
+
 // ForEach Stmt
 ERROR(sequence_protocol_broken,none,
       "SequenceType protocol definition is broken", ())

include/swift/SIL/SILInstruction.h

@@ -3164,6 +3164,14 @@ class MarkUninitializedInst
     /// RootSelf designates "self" in a struct, enum, or root class.
     RootSelf,
 
+    /// CrossModuleRootSelf is the same as "RootSelf", but in a case where
+    /// it's not really safe to treat 'self' as root because the original
+    /// module might add more stored properties.
+    ///
+    /// This is only used for Swift 4 compatibility. In Swift 5, cross-module
+    /// initializers are always DelegatingSelf.
+    CrossModuleRootSelf,
+
     /// DerivedSelf designates "self" in a derived (non-root) class.
     DerivedSelf,
 
@@ -3190,6 +3198,9 @@ class MarkUninitializedInst
   bool isRootSelf() const {
     return ThisKind == RootSelf;
   }
+  bool isCrossModuleRootSelf() const {
+    return ThisKind == CrossModuleRootSelf;
+  }
   bool isDerivedClassSelf() const {
     return ThisKind == DerivedSelf;
   }

lib/AST/Decl.cpp

@@ -5346,15 +5346,21 @@ ConstructorDecl::getDelegatingOrChainedInitKind(DiagnosticEngine *diags,
   // Struct initializers that cannot see the layout of the struct type are
   // always delegating. This occurs if the struct type is not fixed layout,
   // and the constructor is either inlinable or defined in another module.
-  //
-  // FIXME: Figure out the right condition to use here that does not depend
-  // on the -enable-resilience flag, and make it conditional on
-  // -swift-version 5 instead, once the "disallow memberwise cross-module
-  // initializer" proposal lands.
-  if (Kind == BodyInitKind::None) {
-    if (isa<StructDecl>(NTD) &&
-        !NTD->hasFixedLayout(getParentModule(), getResilienceExpansion())) {
+  if (Kind == BodyInitKind::None && isa<StructDecl>(NTD)) {
+    if (getResilienceExpansion() == ResilienceExpansion::Minimal &&
+        !NTD->hasFixedLayout()) {
       Kind = BodyInitKind::Delegating;
+
+    } else if (isa<ExtensionDecl>(getDeclContext())) {
+      const ModuleDecl *containingModule = getParentModule();
+      // Prior to Swift 5, cross-module initializers were permitted to be
+      // non-delegating. However, if the struct isn't fixed-layout, we have to
+      // be delegating because, well, we don't know the layout.
+      if (!NTD->hasFixedLayout() ||
+          containingModule->getASTContext().isSwiftVersionAtLeast(5)) {
+        if (containingModule != NTD->getParentModule())
+          Kind = BodyInitKind::Delegating;
+      }
     }
   }
 

lib/ParseSIL/ParseSIL.cpp

@@ -3048,6 +3048,8 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
       Kind = MarkUninitializedInst::Var;
     else if (KindId.str() == "rootself")
       Kind = MarkUninitializedInst::RootSelf;
+    else if (KindId.str() == "crossmodulerootself")
+      Kind = MarkUninitializedInst::CrossModuleRootSelf;
     else if (KindId.str() == "derivedself")
       Kind = MarkUninitializedInst::DerivedSelf;
     else if (KindId.str() == "derivedselfonly")
@@ -3056,8 +3058,8 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
       Kind = MarkUninitializedInst::DelegatingSelf;
     else {
       P.diagnose(KindLoc, diag::expected_tok_in_sil_instr,
-                 "var, rootself, derivedself, derivedselfonly, "
-                 "or delegatingself");
+                 "var, rootself, crossmodulerootself, derivedself, "
+                 "derivedselfonly, or delegatingself");
       return true;
     }
 

lib/SIL/SILPrinter.cpp

@@ -1283,6 +1283,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
     switch (MU->getKind()) {
     case MarkUninitializedInst::Var: *this << "[var] "; break;
     case MarkUninitializedInst::RootSelf:  *this << "[rootself] "; break;
+    case MarkUninitializedInst::CrossModuleRootSelf:
+      *this << "[crossmodulerootself] ";
+      break;
     case MarkUninitializedInst::DerivedSelf:  *this << "[derivedself] "; break;
     case MarkUninitializedInst::DerivedSelfOnly:
       *this << "[derivedselfonly] ";

lib/SILGen/SILGenConstructor.cpp

@@ -200,9 +200,25 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
   assert(!selfTy.getClassOrBoundGenericClass()
          && "can't emit a class ctor here");
 
+  // Decide if we need to do extra work to warn on unsafe behavior in pre-Swift-5
+  // modes.
+  MarkUninitializedInst::Kind MUIKind;
+  if (isDelegating) {
+    MUIKind = MarkUninitializedInst::DelegatingSelf;
+  } else if (getASTContext().isSwiftVersionAtLeast(5)) {
+    MUIKind = MarkUninitializedInst::RootSelf;
+  } else {
+    auto *dc = ctor->getParent();
+    if (isa<ExtensionDecl>(dc) &&
+        dc->getAsStructOrStructExtensionContext()->getParentModule() !=
+          dc->getParentModule()) {
+      MUIKind = MarkUninitializedInst::CrossModuleRootSelf;
+    } else {
+      MUIKind = MarkUninitializedInst::RootSelf;
+    }
+  }
+
   // Allocate the local variable for 'self'.
-  auto MUIKind = isDelegating ? MarkUninitializedInst::DelegatingSelf
-                              : MarkUninitializedInst::RootSelf;
   emitLocalVariableWithCleanup(selfDecl, MUIKind)->finishInitialization(*this);
   SILValue selfLV = VarLocs[selfDecl].value;
 

lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp

@@ -117,6 +117,12 @@ DIMemoryObjectInfo::DIMemoryObjectInfo(SingleValueInstruction *MI)
   // If this is a derived class init method, track an extra element to determine
   // whether super.init has been called at each program point.
   NumElements += unsigned(isDerivedClassSelf());
+
+  // Make sure we track /something/ in a cross-module struct initializer.
+  if (NumElements == 0 && isCrossModuleStructInitSelf()) {
+    NumElements = 1;
+    HasDummyElement = true;
+  }
 }
 
 SILInstruction *DIMemoryObjectInfo::getFunctionEntryPoint() const {

lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.h

@@ -60,15 +60,19 @@ class DIMemoryObjectInfo {
   /// This is the base type of the memory allocation.
   SILType MemorySILType;
 
-  /// True if the memory object being analyzed represents a 'let', which is
-  /// initialize-only (reassignments are not allowed).
-  bool IsLet = false;
-
   /// This is the count of elements being analyzed.  For memory objects that are
   /// tuples, this is the flattened element count.  For 'self' members in init
   /// methods, this is the local field count (+1 for derive classes).
   unsigned NumElements;
 
+  /// True if the memory object being analyzed represents a 'let', which is
+  /// initialize-only (reassignments are not allowed).
+  bool IsLet = false;
+
+  /// True if NumElements has a dummy value in it to force a struct to be
+  /// non-empty.
+  bool HasDummyElement = false;
+
 public:
   DIMemoryObjectInfo(SingleValueInstruction *MemoryInst);
 
@@ -108,13 +112,25 @@ class DIMemoryObjectInfo {
   /// True if the memory object is the 'self' argument of a struct initializer.
   bool isStructInitSelf() const {
     if (auto *MUI = dyn_cast<MarkUninitializedInst>(MemoryInst))
-      if (MUI->isRootSelf())
+      if (MUI->isRootSelf() || MUI->isCrossModuleRootSelf())
         if (auto decl = getType()->getAnyNominal())
           if (isa<StructDecl>(decl))
             return true;
     return false;
   }
 
+  /// True if the memory object is the 'self' argument of a non-delegating
+  /// cross-module struct initializer.
+  bool isCrossModuleStructInitSelf() const {
+    if (auto *MUI = dyn_cast<MarkUninitializedInst>(MemoryInst)) {
+      if (MUI->isCrossModuleRootSelf()) {
+        assert(isStructInitSelf());
+        return true;
+      }
+    }
+    return false;
+  }
+
   /// True if the memory object is the 'self' argument of a class initializer.
   bool isClassInitSelf() const {
     if (auto *MUI = dyn_cast<MarkUninitializedInst>(MemoryInst))
@@ -125,17 +141,15 @@ class DIMemoryObjectInfo {
     return false;
   }
 
-  /// isDerivedClassSelf - Return true if this memory object is the 'self' of
-  /// a derived class init method.
+  /// True if this memory object is the 'self' of a derived class initializer.
   bool isDerivedClassSelf() const {
     if (auto MUI = dyn_cast<MarkUninitializedInst>(MemoryInst))
       return MUI->isDerivedClassSelf();
     return false;
   }
 
-  /// isDerivedClassSelfOnly - Return true if this memory object is the 'self'
-  /// of a derived class init method for which we can assume that all ivars
-  /// have been initialized.
+  /// True if this memory object is the 'self' of a derived class initializer for
+  /// which we can assume that all ivars have been initialized.
   bool isDerivedClassSelfOnly() const {
     if (auto MUI = dyn_cast<MarkUninitializedInst>(MemoryInst))
       return MUI->isDerivedClassSelfOnly();
@@ -171,9 +185,17 @@ class DIMemoryObjectInfo {
   /// stored properties.
   bool isNonDelegatingInit() const {
     if (auto *MUI = dyn_cast<MarkUninitializedInst>(MemoryInst)) {
-      if (MUI->isDerivedClassSelf() || MUI->isDerivedClassSelfOnly() ||
-          MUI->isRootSelf())
+      switch (MUI->getKind()) {
+      case MarkUninitializedInst::Var:
+        return false;
+      case MarkUninitializedInst::RootSelf:
+      case MarkUninitializedInst::CrossModuleRootSelf:
+      case MarkUninitializedInst::DerivedSelf:
+      case MarkUninitializedInst::DerivedSelfOnly:
         return true;
+      case MarkUninitializedInst::DelegatingSelf:
+        return false;
+      }
     }
     return false;
   }