Skip to content

Commit 7c755ea

Browse files
committed
Disable destructuring initializations in struct let stored properties.
This looks like it was never properly implemented, since when we generate the memberwise initializer for the struct in SILGen, it incorrectly tries to apply the entire initializer expression to each variable binding in the pattern, rather than destructuring the result and pattern-matching it to the variables. Since it never worked it doesn't look like anyone is using this, so let's put up an error saying it's unsupported until we can implement it properly. Add `StructLetDestructuring` as an experimental feature flag so that tests around the feature for things like module interface printing can still work.
1 parent 2548312 commit 7c755ea

File tree

10 files changed

+103
-11
lines changed

10 files changed

+103
-11
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2136,6 +2136,9 @@ ERROR(pattern_binds_no_variables,none,
21362136
ERROR(variable_bound_by_no_pattern,none,
21372137
"variable %0 is not bound by any pattern",
21382138
(const VarDecl *))
2139+
ERROR(destructuring_let_struct_stored_property_unsupported,none,
2140+
"binding multiple 'let' stored properties from a single initializer expression in a struct is unsupported",
2141+
())
21392142

21402143
WARNING(optional_ambiguous_case_ref,none,
21412144
"assuming you mean '%0.%2'; did you mean '%1.%2' instead?",

include/swift/Basic/Features.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ EXPERIMENTAL_FEATURE(PlaygroundExtendedCallbacks, true)
229229
/// Enable the `@_rawLayout` attribute.
230230
EXPERIMENTAL_FEATURE(RawLayout, true)
231231

232+
/// Allow destructuring stored `let` bindings in structs.
233+
EXPERIMENTAL_FEATURE(StructLetDestructuring, true)
234+
232235
#undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
233236
#undef EXPERIMENTAL_FEATURE
234237
#undef UPCOMING_FEATURE

lib/AST/ASTPrinter.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3457,6 +3457,26 @@ static bool usesFeatureRawLayout(Decl *decl) {
34573457
return decl->getAttrs().hasAttribute<RawLayoutAttr>();
34583458
}
34593459

3460+
static bool usesFeatureStructLetDestructuring(Decl *decl) {
3461+
auto sd = dyn_cast<StructDecl>(decl);
3462+
if (!sd)
3463+
return false;
3464+
3465+
for (auto member : sd->getStoredProperties()) {
3466+
if (!member->isLet())
3467+
continue;
3468+
3469+
auto init = member->getParentPattern();
3470+
if (!init)
3471+
continue;
3472+
3473+
if (!init->getSingleVar())
3474+
return true;
3475+
}
3476+
3477+
return false;
3478+
}
3479+
34603480
static bool hasParameterPacks(Decl *decl) {
34613481
if (auto genericContext = decl->getAsGenericContext()) {
34623482
auto sig = genericContext->getGenericSignature();

lib/SILGen/SILGenConstructor.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,13 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
470470
.forwardInto(SGF, Loc, init.get());
471471
++elti;
472472
} else {
473+
// TODO: This doesn't correctly take into account destructuring
474+
// pattern bindings on `let`s, for example `let (a, b) = foo()`. In
475+
// cases like that, we ought to evaluate the initializer expression once
476+
// and then do a pattern assignment to the variables in the pattern.
477+
// That case is currently forbidden with an "unsupported" error message
478+
// in Sema.
479+
473480
assert(field->getTypeInContext()->getReferenceStorageReferent()->isEqual(
474481
field->getParentExecutableInitializer()->getType()) &&
475482
"Initialization of field with mismatched type!");
@@ -534,6 +541,13 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
534541
++elti;
535542
} else {
536543
// Otherwise, use its initializer.
544+
// TODO: This doesn't correctly take into account destructuring
545+
// pattern bindings on `let`s, for example `let (a, b) = foo()`. In
546+
// cases like that, we ought to evaluate the initializer expression once
547+
// and then do a pattern assignment to the variables in the pattern.
548+
// That case is currently forbidden with an "unsupported" error message
549+
// in Sema.
550+
537551
assert(field->isParentExecutabledInitialized());
538552
Expr *init = field->getParentExecutableInitializer();
539553

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,7 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
869869
if (hadError)
870870
PBD->setInvalid();
871871
PBD->setInitializerChecked(patternNumber);
872+
872873
return hadError;
873874
}
874875

lib/Sema/TypeCheckStorage.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,21 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate(
567567
: GlobalVariable);
568568
}
569569
}
570+
571+
// If the pattern binding appears as a compound stored `let` property with an
572+
// initializer inside of a struct type, diagnose it as unsupported.
573+
// This hasn't ever been implemented properly.
574+
if (!Context.LangOpts.hasFeature(Feature::StructLetDestructuring)
575+
&& !binding->isStatic()
576+
&& binding->isInitialized(entryNumber)
577+
&& isa<StructDecl>(binding->getDeclContext())
578+
&& !pattern->getSingleVar()
579+
&& !vars.empty()
580+
&& vars[0]->isLet()) {
581+
Context.Diags.diagnose(binding->getPattern(entryNumber)->getLoc(),
582+
diag::destructuring_let_struct_stored_property_unsupported);
583+
}
584+
570585
return &pbe;
571586
}
572587

test/ModuleInterface/stored-properties-client.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44

55
// 1. Build ../stored-properties.swift to a dylib and emit its interface in %t
66

7-
// 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
7+
// 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
88
// RUN: %target-swift-typecheck-module-from-interface(%t/StoredProperties.swiftinterface) -module-name StoredProperties
99

1010
// 2. Build this file and link with StoredProperties
1111

12-
// RUN: %target-build-swift %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
12+
// RUN: %target-build-swift -enable-experimental-feature StructLetDestructuring %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
1313

1414
// 3. Codesign and run this, and ensure it exits successfully.
1515

@@ -20,9 +20,9 @@
2020

2121
// RUN: %empty-directory(%t)
2222

23-
// 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
23+
// 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
2424

25-
// RUN: %target-build-swift %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
25+
// RUN: %target-build-swift -enable-experimental-feature StructLetDestructuring %s -I %t -L %t -lStoredProperties -o %t/stored-properties-client %target-rpath(%t)
2626
// RUN: %target-codesign %t/stored-properties-client %t/%target-library-name(StoredProperties)
2727
// RUN: %target-run %t/stored-properties-client %t/%target-library-name(StoredProperties)
2828

test/ModuleInterface/stored-properties.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
// RUN: %empty-directory(%t)
22

3-
// RUN: %target-swift-frontend -typecheck -emit-module-interface-path %t.swiftinterface -module-name StoredProperties %s
3+
// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -typecheck -emit-module-interface-path %t.swiftinterface -module-name StoredProperties %s
44
// RUN: %target-swift-typecheck-module-from-interface(%t.swiftinterface) -module-name StoredProperties
55
// RUN: %FileCheck %s < %t.swiftinterface --check-prefix CHECK --check-prefix COMMON
66

7-
// RUN: %target-swift-frontend -typecheck -emit-module-interface-path %t-resilient.swiftinterface -module-name StoredProperties -enable-library-evolution %s
7+
// RUN: %target-swift-frontend -enable-experimental-feature StructLetDestructuring -typecheck -emit-module-interface-path %t-resilient.swiftinterface -module-name StoredProperties -enable-library-evolution %s
88
// RUN: %target-swift-typecheck-module-from-interface(%t-resilient.swiftinterface) -module-name StoredProperties
99
// RUN: %FileCheck %s < %t-resilient.swiftinterface --check-prefix RESILIENT --check-prefix COMMON
1010

11-
// RUN: %target-swift-frontend -emit-module -o %t/Test.swiftmodule -module-name StoredProperties %t.swiftinterface -disable-objc-attr-requires-foundation-module
12-
// 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
11+
// 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
12+
// 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
1313

14-
// 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
15-
// 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
14+
// 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
15+
// 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
1616

1717
// COMMON: public struct HasStoredProperties {
1818
public struct HasStoredProperties {
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %target-swift-frontend -typecheck -verify %s
2+
3+
// https://github.com/apple/swift/issues/68915
4+
// Destructuring initializations for `let` properties in structs isn't
5+
// implemented correctly in SILGen, so diagnose it as unsupported for now.
6+
7+
struct Foo {
8+
var value: Int = 42
9+
10+
let (aaa, bbb) = ("aaa", "bbb") // expected-error{{unsupported}}
11+
12+
let (z1, z2, z3) = ("one", 1, Double.pi) // expected-error{{unsupported}}
13+
14+
15+
func tellMe() {
16+
print(foo.aaa)
17+
print(foo.bbb) // output: aaa
18+
19+
20+
assert(aaa == "aaa")
21+
assert(bbb == "bbb", "bbb should be bbb but it's \(bbb)")
22+
}
23+
24+
}
25+
26+
27+
let foo = Foo(/*value: 1*/)
28+
29+
30+
foo.tellMe()
31+
32+
33+
34+
35+
print("Hello")

test/decl/var/properties.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,8 @@ _ = r19874152S5() // ok
12151215

12161216

12171217
struct r19874152S6 {
1218-
let (a,b) = (1,2) // Cannot handle implicit synth of this yet.
1218+
// Cannot handle implicit synth of this yet.
1219+
let (a,b) = (1,2) // expected-error {{unsupported}}
12191220
}
12201221
_ = r19874152S5() // ok
12211222

0 commit comments

Comments
 (0)