Clean up the semantics of 'let' properties in a number of ways:

- We switch to a model where let properties may be "initialized", but never
  reassigned.  Specifically, immutable properties in structs/classes may have
  an init value specified in their declaration (but can then never be reset
  in any init implementation) or not (in which case they must be initialized 
  exactly once on all paths through every init.  This makes a lot more sense 
  for immutability, defines several problems away, and provides a path to
  supporting things like (rdar://16181314)

- We now *never* default initialize an immutable property.  Formerly
  we would default initialize optional let properties to nil, but this
  isn't actually useful, and allows an error of omission with let 
  properties.  

This resolves: <rdar://problem/19035287> let properties should only be initializable, not reassignable
and possibly other radars.





Swift SVN r23779

include/swift/AST/DiagnosticsSIL.def

@@ -137,6 +137,14 @@ ERROR(global_variable_function_use_uninit,sil_analysis,none,
 ERROR(struct_not_fully_initialized,sil_analysis,none,
       "struct '%0' must be completely initialized before a member is stored to",
       (StringRef))
+ERROR(immutable_property_already_initialized,sil_analysis,none,
+      "immutable property '%0' may only be initialized once",
+      (StringRef))
+NOTE(initial_value_provided_in_let_decl,sil_analysis,none,
+     "initial value already provided in 'let' declaration", ())
+ERROR(immutable_property_passed_inout,sil_analysis,none,
+      "immutable property '%0' may not be passed to an inout argument",
+      (StringRef))
 
 
 ERROR(object_not_fully_initialized_before_failure,sil_analysis,none,

include/swift/AST/DiagnosticsSema.def

@@ -863,8 +863,6 @@ ERROR(decl_no_default_init_ivar_hole,sema_tcd,none,
       "cannot use initial value when one of the variables is '_'", ())
 NOTE(decl_init_here,sema_tcd,none,
      "initial value is here", ())
-WARNING(let_default_init,sema_tcd,none,
-        "immutable value is default initialized and can never be changed", ())
 
 
 // Inheritance

lib/SILPasses/DIMemoryUseCollector.cpp

@@ -254,6 +254,26 @@ getPathStringToElement(unsigned Element, std::string &Result) const {
                                    IsSelfOfNonDelegatingInitializer, Result);
 }
 
+/// If the specified value is a 'let' property in an initializer, return true.
+bool DIMemoryObjectInfo::isElementLetProperty(unsigned Element) const {
+  // If we aren't representing 'self' in a non-delegating initializer, then we
+  // can't have 'let' properties.
+  if (!IsSelfOfNonDelegatingInitializer) return false;
+
+  auto *NTD = cast<NominalTypeDecl>(getType()->getAnyNominal());
+  for (auto *VD : NTD->getStoredProperties()) {
+    auto FieldType = VD->getType()->getCanonicalType();
+    unsigned NumFieldElements = getElementCountRec(FieldType, false);
+    if (Element < NumFieldElements)
+      return VD->isLet();
+    Element -= NumFieldElements;
+  }
+
+  // Otherwise, we miscounted elements?
+  assert(Element == 0 && "Element count problem");
+  return false;
+}
+
 
 //===----------------------------------------------------------------------===//
 //                        DIMemoryUse Implementation

lib/SILPasses/DIMemoryUseCollector.h

@@ -155,6 +155,9 @@ public:
   /// be determined, return it.  Otherwise, return null.
   ValueDecl *getPathStringToElement(unsigned Element,
                                     std::string &Result) const;
+
+  /// If the specified value is a 'let' property in an initializer, return true.
+  bool isElementLetProperty(unsigned Element) const;
 };
 
 

lib/SILPasses/DefiniteInitialization.cpp

@@ -370,6 +370,8 @@ namespace {
     bool isInitializedAtUse(const DIMemoryUse &Use, bool *SuperInitDone = 0);
 
     void handleStoreUse(unsigned UseID);
+    void handleInOutUse(const DIMemoryUse &Use);
+
     void handleLoadUseFailure(const DIMemoryUse &InstInfo,
                               bool IsSuperInitComplete);
     void handleSuperInitUse(const DIMemoryUse &InstInfo);
@@ -649,8 +651,7 @@ void LifetimeChecker::doIt() {
     }
 
     case DIUseKind::InOutUse:
-      if (!isInitializedAtUse(Use))
+      handleInOutUse(Use);
-        diagnoseInitError(Use, diag::variable_inout_before_initialized);
       break;
 
     case DIUseKind::Escape:
@@ -726,16 +727,7 @@ void LifetimeChecker::handleStoreUse(unsigned UseID) {
   auto Liveness = getLivenessAtInst(InstInfo.Inst, InstInfo.FirstElement,
                                     InstInfo.NumElements);
 
-  // If this is a partial store into a struct and the whole struct hasn't been
+  // Check to see if the stored location is either fully uninitialized or fully
-  // initialized, diagnose this as an error.
-  if (InstInfo.Kind == DIUseKind::PartialStore &&
-      Liveness.get(InstInfo.FirstElement) != DIKind::Yes) {
-    assert(InstInfo.NumElements == 1 && "partial stores are intra-element");
-    diagnoseInitError(InstInfo, diag::struct_not_fully_initialized);
-    return;
-  }
-
-  // Check to see if the store is either fully uninitialized or fully
   // initialized.
   bool isFullyInitialized = true;
   bool isFullyUninitialized = true;
@@ -748,6 +740,38 @@ void LifetimeChecker::handleStoreUse(unsigned UseID) {
       isFullyUninitialized = false;
   }
 
+  // If this is a partial store into a struct and the whole struct hasn't been
+  // initialized, diagnose this as an error.
+  if (InstInfo.Kind == DIUseKind::PartialStore && !isFullyInitialized) {
+    assert(InstInfo.NumElements == 1 && "partial stores are intra-element");
+    diagnoseInitError(InstInfo, diag::struct_not_fully_initialized);
+    return;
+  }
+
+  // If this is a store to a 'let' property in an initializer, then we only
+  // allow the assignment if the property was completely uninitialized.
+  // Overwrites are not permitted.
+  if (TheMemory.IsSelfOfNonDelegatingInitializer &&
+      (InstInfo.Kind == DIUseKind::PartialStore || !isFullyUninitialized)) {
+
+    for (unsigned i = InstInfo.FirstElement, e = i+InstInfo.NumElements;
+         i != e; ++i) {
+      if (Liveness.get(i) == DIKind::No || !TheMemory.isElementLetProperty(i))
+        continue;
+
+      std::string PropertyName = "self";
+      auto *VD = TheMemory.getPathStringToElement(i, PropertyName);
+      diagnose(Module, InstInfo.Inst->getLoc(),
+               diag::immutable_property_already_initialized, PropertyName);
+      if (auto *Var = dyn_cast<VarDecl>(VD))
+        if (auto *InitPat = Var->getParentPattern())
+          if (InitPat->hasInit())
+            diagnose(Module, SILLocation(VD),
+                     diag::initial_value_provided_in_let_decl);
+      return;
+    }
+  }
+
   // 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) {
@@ -778,6 +802,33 @@ void LifetimeChecker::handleStoreUse(unsigned UseID) {
   updateInstructionForInitState(InstInfo);
 }
 
+void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
+  // inout uses are generally straight-forward: the memory must be initialized
+  // before the "address" is passed as an l-value.
+  if (!isInitializedAtUse(Use)) {
+    diagnoseInitError(Use, diag::variable_inout_before_initialized);
+    return;
+  }
+
+  // One additional check: 'let' properties may never be passed inout, because
+  // they are only allowed to have their initial value set, not a subsequent
+  // overwrite.
+  if (TheMemory.IsSelfOfNonDelegatingInitializer) {
+    for (unsigned i = Use.FirstElement, e = i+Use.NumElements;
+         i != e; ++i) {
+      if (!TheMemory.isElementLetProperty(i))
+        continue;
+
+      std::string PropertyName = "self";
+      (void)TheMemory.getPathStringToElement(i, PropertyName);
+      diagnose(Module, Use.Inst->getLoc(),
+               diag::immutable_property_passed_inout, PropertyName);
+      return;
+    }
+  }
+}
+
+
 /// handleLoadUseFailure - Check and diagnose various failures when a load use
 /// is not fully initialized.
 ///

lib/Sema/TypeCheckDecl.cpp

@@ -3723,36 +3723,26 @@ public:
       if (PBD->getPattern()->hasType() && !PBD->hasInit() &&
           PBD->hasStorage() && !PBD->getPattern()->getType()->is<ErrorType>()) {
 
-        // If we have a type-adjusting attribute, apply it now.
+        // If we have a type-adjusting attribute (like ownership), apply it now.
-        // Also record whether the pattern-binding is for a debugger variable.
-        bool isDebuggerVar = false;
         if (auto var = PBD->getSingleVar()) {
-          isDebuggerVar = var->isDebuggerVar();
-            
           if (auto *OA = var->getAttrs().getAttribute<OwnershipAttr>())
             TC.checkOwnershipAttr(var, OA);
         }
 
-        // Make sure we don't have a @NSManaged property.
+        // Decide whether we should suppress default initialization.
-        bool hasNSManaged = false;
+        bool suppressDefaultInit = false;
         PBD->getPattern()->forEachVariable([&](VarDecl *var) {
-          if (var->getAttrs().hasAttribute<NSManagedAttr>())
+          // @NSManaged properties never get default initialized, nor do
-            hasNSManaged = true;
+          // debugger variables and immutable properties.
+          if (var->getAttrs().hasAttribute<NSManagedAttr>() ||
+              var->isDebuggerVar() ||
+              var->isLet())
+            suppressDefaultInit = true;
         });
 
-        if (!hasNSManaged && !isDebuggerVar) {
+        if (!suppressDefaultInit) {
           auto type = PBD->getPattern()->getType();
           if (auto defaultInit = buildDefaultInitializer(TC, type)) {
-            // If any of the default initialized values are immutable, then
-            // emit a diagnostic.  We don't do this for members of types, since
-            // the init members have write access to the let values.
-            if (!PBD->getDeclContext()->isTypeContext()) {
-              PBD->getPattern()->forEachVariable([&] (VarDecl *VD) {
-                if (VD->isLet())
-                  TC.diagnose(VD->getLoc(), diag::let_default_init);
-              });
-            }
-
             // If we got a default initializer, install it and re-type-check it
             // to make sure it is properly coerced to the pattern type.
             PBD->setInit(defaultInit, /*checked=*/false);

stdlib/unittest/LifetimeTracked.swift

@@ -41,7 +41,7 @@ public final class LifetimeTracked : ForwardIndexType, Printable {
     return trackedCount
   }
   
-  public let value: Int = 0
+  public let value: Int
   public var serialNumber: Int = 0
 }
 

stdlib/unittest/StdlibUnittest.swift.gyb

@@ -32,9 +32,11 @@ public struct SourceLoc {
 }
 
 public struct SourceLocStack {
-  let locs: _UnitTestArray<SourceLoc> = []
+  let locs: _UnitTestArray<SourceLoc>
 
-  public init() {}
+  public init() {
+    locs = []
+  }
 
   public init(_ loc: SourceLoc) {
     locs = [ loc ]

test/SILGen/unsupported_recursive_value_type.swift

@@ -6,6 +6,8 @@ struct SelfRecursiveStruct { // expected-error{{recursive value type}}
 
 struct OptionallyRecursiveStruct { // expected-error{{recursive value type}}
   let a: OptionallyRecursiveStruct?
+
+  init() { a = OptionallyRecursiveStruct() }
 }
 
 struct IndirectlyRecursiveStruct1 { // expected-error{{recursive value type}}

test/SILPasses/definite_init_diagnostics.swift

@@ -695,7 +695,7 @@ class rdar18414728Base {
     if let p1 = prop { // expected-error {{use of 'self' in property access 'prop' before all stored properties are initialized}}
       aaaaa = p1
     }
-    aaaaa = "foo"
+    aaaaa = "foo"  // expected-error {{immutable property 'self.aaaaa' may only be initialized once}}
   }
 
   init(a : ()) {
@@ -726,7 +726,7 @@ class rdar18414728Derived : rdar18414728Base {
     if let p1 = prop2 {  // expected-error {{use of 'self' in property access 'prop2' before super.init initializes self}}
       aaaaa2 = p1
     }
-    aaaaa2 = "foo"
+    aaaaa2 = "foo"    // expected-error {{immutable property 'self.aaaaa2' may only be initialized once}}
     super.init()
   }
 
@@ -744,7 +744,7 @@ class rdar18414728Derived : rdar18414728Base {
 
   override init(c : ()) {
     super.init()        // expected-error {{property 'self.aaaaa2' not initialized at super.init call}}
-    aaaaa2 = "foo"
+    aaaaa2 = "foo"      // expected-error {{immutable property 'self.aaaaa2' may only be initialized once}}
     method2()
   }
 
@@ -817,3 +817,49 @@ struct rdar17207456Struct {
   }
 }
 
+
+// <rdar://problem/19035287> let properties should only be initializable, not reassignable
+struct LetProperties {
+  let (u, v) : (Int, Int)
+  let x = 42    // expected-note{{initial value already provided in 'let' declaration}}
+  let y : Int
+  let z : Int?  // expected-note{{'self.z' not initialized}}
+
+  // Let properties can be initialized naturally exactly once along any given
+  // path through an initializer.
+  init(cond : Bool) {
+    if cond {
+      (u,v) = (4,2)
+      y = 71
+    } else {
+      y = 13
+      v = 2
+      u = v+1
+    }
+    z = nil
+  }
+
+  // Multiple initializations are an error.
+  init(a : Int) {
+    x = a   // expected-error {{immutable property 'self.x' may only be initialized once}}
+    y = a
+    y = a   // expected-error {{immutable property 'self.y' may only be initialized once}}
+
+    u = a
+    v = a
+    u = a   // expected-error {{immutable property 'self.u' may only be initialized once}}
+    v = a   // expected-error {{immutable property 'self.v' may only be initialized once}}
+
+  }  // expected-error {{return from initializer without initializing all stored properties}}
+
+  // inout uses of let properties are an error.
+  init() {
+    u = 1; v = 13; y = 1 ; z = u
+
+    var variable = 42
+    swap(&u, &variable)  // expected-error {{immutable property 'self.u' may not be passed to an inout argument}}
+  }
+}
+
+
+

test/decl/init/default-initialization.swift

@@ -104,6 +104,3 @@ class MultipleInitDerived : MultipleInitBase {
   override init() { } // expected-error{{super.init isn't called before returning from initializer}}
 }
 
-// <rdar://problem/17501765> Swift should warn about immutable default initialized values
-let uselessValue : String?  // expected-warning {{immutable value is default initialized and can never be changed}}
-

test/decl/var/variables.swift

@@ -69,3 +69,8 @@ class SomeClass {}
 weak let V = SomeClass()  // expected-error {{'weak' must be a mutable variable, because it may change at runtime}}
 
 let a = b ; let b = a // expected-error{{could not infer type for 'a'}} expected-error{{could not infer type for 'b'}}
+
+
+// <rdar://problem/17501765> Swift should warn about immutable default initialized values
+let uselessValue : String? // expected-error {{'let' declarations require an initializer expression}}
+