DI: Warn on non-delegating cross-module struct initializers

...as detected by initializing an individual field without having
initialized the whole object (via `self = value`).

This only applies in pre-Swift-5 mode because the next commit will
treat all cross-module struct initializers as delegating in Swift 5.

Author: jrose-apple

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,11 @@ 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 it is not in module %2",
+        (Type, bool, Identifier))
+
 
 // Control flow diagnostics.
 ERROR(missing_return,none,

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/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.h

@@ -108,13 +108,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 +137,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 +181,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;
   }

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -506,6 +506,12 @@ namespace {
     /// This is true when there is a destroy on a path where the self value may
     /// have been consumed, in which case there is nothing to do.
     bool HasConditionalSelfInitialized = false;
+    
+    /// This is true when the object being checked is a 'self' parameter for a
+    /// struct in a non-delegating cross-module initializer. In this case, the
+    /// initializer is not allowed to be fieldwise in Swift 5, so we produce a
+    /// warning in Swift 4 and earlier.
+    bool WantsCrossModuleStructInitializerDiagnostic = false;
 
     // Keep track of whether we've emitted an error.  We only emit one error per
     // location as a policy decision.
@@ -631,6 +637,11 @@ LifetimeChecker::LifetimeChecker(const DIMemoryObjectInfo &TheMemory,
   // locally inferred by the loop above.  Mark any unset elements as not
   // available.
   MemBBInfo.setUnknownToNotAvailable();
+
+  // Finally, check if we need to emit compatibility diagnostics for cross-module
+  // non-delegating struct initializers.
+  if (TheMemory.isCrossModuleStructInitSelf())
+    WantsCrossModuleStructInitializerDiagnostic = true;
 }
 
 /// Determine whether the specified block is reachable from the entry of the
@@ -995,6 +1006,50 @@ void LifetimeChecker::handleStoreUse(unsigned UseID) {
     }
   }
 
+  // Check if we're in a struct initializer that uses CrossModuleRootSelf rather
+  // than DelegatingSelf for Swift 4 compatibility. We look for a problem case by
+  // seeing if there are any assignments to individual fields that might be
+  // initializations; that is, that they're not dominated by `self = other`.
+
+  auto isFullValueAssignment = [this](const SILInstruction *inst) -> bool {
+    SILValue addr;
+    if (auto *copyAddr = dyn_cast<CopyAddrInst>(inst))
+      addr = copyAddr->getDest();
+    else if (auto *assign = dyn_cast<AssignInst>(inst))
+      addr = assign->getDest();
+    else
+      return false;
+
+    if (auto *access = dyn_cast<BeginAccessInst>(addr))
+      addr = access->getSource();
+    if (auto *projection = dyn_cast<ProjectBoxInst>(addr))
+      addr = projection->getOperand();
+
+    return addr == TheMemory.getAddress();
+  };
+
+  if (!isFullyInitialized && WantsCrossModuleStructInitializerDiagnostic &&
+      !isFullValueAssignment(Use.Inst)) {
+    // Deliberately don't check shouldEmitError here; we're using DI to approximate
+    // whether this would be a valid delegating initializer, but the error when it
+    // /is/ a delegating initializer won't be path-sensitive.
+
+    Type selfTy;
+    SILLocation fnLoc = TheMemory.getFunction().getLocation();
+    if (auto *ctor = fnLoc.getAsASTNode<ConstructorDecl>())
+      selfTy = ctor->getImplicitSelfDecl()->getType()->getInOutObjectType();
+    else
+      selfTy = TheMemory.getType();
+
+    diagnose(Module, Use.Inst->getLoc(),
+             diag::designated_init_in_cross_module_extension,
+             selfTy, !isFullyUninitialized,
+             selfTy->getAnyNominal()->getParentModule()->getName());
+
+    // Don't emit more than one of these diagnostics per initializer.
+    WantsCrossModuleStructInitializerDiagnostic = false;
+  }
+
   // If this is an initialization or a normal assignment, upgrade the store to
   // an initialization or assign in the uses list so that clients know about it.
   if (isFullyUninitialized) {