Merge pull request #68930 from jckarter/disable-let-stored-property-destructuring

Disable destructuring initializations in struct `let` stored properties.

Author: jckarter

include/swift/AST/DiagnosticsSema.def

@@ -2175,6 +2175,9 @@ ERROR(pattern_binds_no_variables,none,
 ERROR(variable_bound_by_no_pattern,none,
       "variable %0 is not bound by any pattern",
       (const VarDecl *))
+ERROR(destructuring_let_struct_stored_property_unsupported,none,
+      "binding multiple 'let' stored properties from a single initializer expression in a struct is unsupported",
+      ())
 
 WARNING(optional_ambiguous_case_ref,none,
         "assuming you mean '%0.%2'; did you mean '%1.%2' instead?",

include/swift/Basic/Features.def

@@ -242,6 +242,9 @@ EXPERIMENTAL_FEATURE(NoncopyableGenerics, false)
 /// Enables typed throws.
 EXPERIMENTAL_FEATURE(TypedThrows, true)
 
+/// Allow destructuring stored `let` bindings in structs.
+EXPERIMENTAL_FEATURE(StructLetDestructuring, true)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

lib/AST/ASTPrinter.cpp

@@ -3488,6 +3488,26 @@ static bool usesFeatureRawLayout(Decl *decl) {
   return decl->getAttrs().hasAttribute<RawLayoutAttr>();
 }
 
+static bool usesFeatureStructLetDestructuring(Decl *decl) {
+  auto sd = dyn_cast<StructDecl>(decl);
+  if (!sd)
+    return false;
+    
+  for (auto member : sd->getStoredProperties()) {
+    if (!member->isLet())
+      continue;
+    
+    auto init = member->getParentPattern();
+    if (!init)
+      continue;
+      
+    if (!init->getSingleVar())
+      return true;
+  }
+  
+  return false;
+}
+
 static bool hasParameterPacks(Decl *decl) {
   if (auto genericContext = decl->getAsGenericContext()) {
     auto sig = genericContext->getGenericSignature();

lib/SILGen/SILGenConstructor.cpp

@@ -470,6 +470,13 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
           .forwardInto(SGF, Loc, init.get());
         ++elti;
       } else {
+        // TODO: This doesn't correctly take into account destructuring
+        // pattern bindings on `let`s, for example `let (a, b) = foo()`. In
+        // cases like that, we ought to evaluate the initializer expression once
+        // and then do a pattern assignment to the variables in the pattern.
+        // That case is currently forbidden with an "unsupported" error message
+        // in Sema.
+        
         assert(field->getTypeInContext()->getReferenceStorageReferent()->isEqual(
                    field->getParentExecutableInitializer()->getType()) &&
                "Initialization of field with mismatched type!");
@@ -534,6 +541,13 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
       ++elti;
     } else {
       // Otherwise, use its initializer.
+      // TODO: This doesn't correctly take into account destructuring
+      // pattern bindings on `let`s, for example `let (a, b) = foo()`. In
+      // cases like that, we ought to evaluate the initializer expression once
+      // and then do a pattern assignment to the variables in the pattern.
+      // That case is currently forbidden with an "unsupported" error message
+      // in Sema.
+
       assert(field->isParentExecutabledInitialized());
       Expr *init = field->getParentExecutableInitializer();
 

lib/Sema/TypeCheckConstraints.cpp

@@ -882,6 +882,7 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
   if (hadError)
     PBD->setInvalid();
   PBD->setInitializerChecked(patternNumber);
+  
   return hadError;
 }
 

lib/Sema/TypeCheckStorage.cpp

@@ -567,6 +567,21 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate(
                                  : GlobalVariable);
     }
   }
+  
+  // If the pattern binding appears as a compound stored `let` property with an
+  // initializer inside of a struct type, diagnose it as unsupported.
+  // This hasn't ever been implemented properly.
+  if (!Context.LangOpts.hasFeature(Feature::StructLetDestructuring)
+      && !binding->isStatic()
+      && binding->isInitialized(entryNumber)
+      && isa<StructDecl>(binding->getDeclContext())
+      && !pattern->getSingleVar()
+      && !vars.empty()
+      && vars[0]->isLet()) {
+    Context.Diags.diagnose(binding->getPattern(entryNumber)->getLoc(),
+                           diag::destructuring_let_struct_stored_property_unsupported);
+  }
+  
   return &pbe;
 }
 

test/ModuleInterface/stored-properties-client.swift

@@ -4,12 +4,12 @@
 
 // 1. Build ../stored-properties.swift to a dylib and emit its interface in %t
 
-// RUN: %target-build-swift-dylib(%t/%target-library-name(StoredProperties)) -emit-module-interface-path %t/StoredProperties.swiftinterface %S/stored-properties.swift -module-name StoredProperties -swift-version 5
+// RUN: %target-build-swift-dylib(%t/%target-library-name(StoredProperties)) -enable-experimental-feature StructLetDestructuring -emit-module-interface-path %t/StoredProperties.swiftinterface %S/stored-properties.swift -module-name StoredProperties -swift-version 5
 // RUN: %target-swift-typecheck-module-from-interface(%t/StoredProperties.swiftinterface) -module-name StoredProperties
 
 // 2. Build this file and link with StoredProperties
 
-// RUN: %target-build-swift %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
+// RUN: %target-build-swift -enable-experimental-feature StructLetDestructuring %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
 
 // 3. Codesign and run this, and ensure it exits successfully.
 
@@ -20,9 +20,9 @@
 
 // RUN: %empty-directory(%t)
 
-// RUN: %target-build-swift-dylib(%t/%target-library-name(StoredProperties)) -emit-module-interface-path %t/StoredProperties.swiftinterface %S/stored-properties.swift -module-name StoredProperties -swift-version 5 -enable-library-evolution
+// RUN: %target-build-swift-dylib(%t/%target-library-name(StoredProperties)) -enable-experimental-feature StructLetDestructuring -emit-module-interface-path %t/StoredProperties.swiftinterface %S/stored-properties.swift -module-name StoredProperties -swift-version 5 -enable-library-evolution
 
-// RUN: %target-build-swift %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
+// RUN: %target-build-swift -enable-experimental-feature StructLetDestructuring %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
 // RUN: %target-codesign %t/stored-properties-client %t/%target-library-name(StoredProperties)
 // RUN: %target-run %t/stored-properties-client %t/%target-library-name(StoredProperties)
 

test/ModuleInterface/stored-properties.swift

@@ -1,18 +1,18 @@
 // RUN: %empty-directory(%t)
 
-// RUN: %target-swift-frontend -typecheck -emit-module-interface-path %t.swiftinterface -module-name StoredProperties %s
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -typecheck -emit-module-interface-path %t.swiftinterface -module-name StoredProperties %s
 // RUN: %target-swift-typecheck-module-from-interface(%t.swiftinterface) -module-name StoredProperties
 // RUN: %FileCheck %s < %t.swiftinterface --check-prefix CHECK --check-prefix COMMON
 
-// RUN: %target-swift-frontend -typecheck -emit-module-interface-path %t-resilient.swiftinterface -module-name StoredProperties -enable-library-evolution %s
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -typecheck -emit-module-interface-path %t-resilient.swiftinterface -module-name StoredProperties -enable-library-evolution %s
 // RUN: %target-swift-typecheck-module-from-interface(%t-resilient.swiftinterface) -module-name StoredProperties
 // RUN: %FileCheck %s < %t-resilient.swiftinterface --check-prefix RESILIENT --check-prefix COMMON
 
-// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -module-name StoredProperties %t.swiftinterface -disable-objc-attr-requires-foundation-module
-// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -module-name StoredProperties -emit-module-interface-path - | %FileCheck %s --check-prefix CHECK --check-prefix COMMON
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -emit-module -o %t/Test.swiftmodule -module-name StoredProperties %t.swiftinterface -disable-objc-attr-requires-foundation-module
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -emit-module -o /dev/null -merge-modules %t/Test.swiftmodule -module-name StoredProperties -emit-module-interface-path - | %FileCheck %s --check-prefix CHECK --check-prefix COMMON
 
-// RUN: %target-swift-frontend -emit-module -o %t/TestResilient.swiftmodule -module-name StoredProperties -enable-library-evolution %t-resilient.swiftinterface -disable-objc-attr-requires-foundation-module
-// RUN: %target-swift-frontend -emit-module -o /dev/null -merge-modules %t/TestResilient.swiftmodule -module-name StoredProperties -enable-library-evolution -emit-module-interface-path - | %FileCheck %s --check-prefix RESILIENT --check-prefix COMMON
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -emit-module -o %t/TestResilient.swiftmodule -module-name StoredProperties -enable-library-evolution %t-resilient.swiftinterface -disable-objc-attr-requires-foundation-module
+// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -emit-module -o /dev/null -merge-modules %t/TestResilient.swiftmodule -module-name StoredProperties -enable-library-evolution -emit-module-interface-path - | %FileCheck %s --check-prefix RESILIENT --check-prefix COMMON
 
 // COMMON: public struct HasStoredProperties {
 public struct HasStoredProperties {

test/Sema/struct_property_let_destructuring.swift

@@ -0,0 +1,35 @@
+// RUN: %target-swift-frontend -typecheck -verify %s
+
+// https://github.com/apple/swift/issues/68915
+// Destructuring initializations for `let` properties in structs isn't
+// implemented correctly in SILGen, so diagnose it as unsupported for now.
+
+struct Foo {
+    var value: Int = 42
+
+    let (aaa, bbb) = ("aaa", "bbb") // expected-error{{unsupported}}
+
+    let (z1, z2, z3) = ("one", 1, Double.pi) // expected-error{{unsupported}}
+
+
+    func tellMe() {
+        print(foo.aaa)
+        print(foo.bbb)          // output:  aaa
+
+
+        assert(aaa == "aaa")
+        assert(bbb == "bbb", "bbb should be bbb but it's \(bbb)")
+    }
+
+}
+
+
+let foo = Foo(/*value: 1*/)
+
+
+foo.tellMe()
+
+
+
+
+print("Hello")

test/decl/var/properties.swift

@@ -1215,7 +1215,8 @@ _ = r19874152S5()  // ok
 
 
 struct r19874152S6 {
-  let (a,b) = (1,2)   // Cannot handle implicit synth of this yet.
+  // Cannot handle implicit synth of this yet.
+  let (a,b) = (1,2)   // expected-error {{unsupported}}
 }
 _ = r19874152S5()  // ok