Skip to content

Commit da968db

Browse files
committed
[MoveChecker] Ban exported partial consumption.
To avoid dialecticization based on compilation mode, ban for non-resilient modules partial consumption of aggregates which would be illegal were those modules instead resilient.
1 parent 407fba8 commit da968db

File tree

6 files changed

+369
-8
lines changed

6 files changed

+369
-8
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,12 +831,24 @@ ERROR(sil_movechecking_notconsumable_but_assignable_was_consumed, none,
831831
ERROR(sil_movechecking_cannot_destructure_has_deinit, none,
832832
"cannot partially consume '%0' when it has a deinitializer",
833833
(StringRef))
834+
ERROR(sil_movechecking_cannot_destructure_imported_nonfrozen, none,
835+
"cannot partially consume '%0' of non-frozen type %1 imported from %2",
836+
(StringRef, Type, const ModuleDecl*))
837+
ERROR(sil_movechecking_cannot_destructure_exported_usableFromInline_alwaysEmitIntoClient, none,
838+
"cannot partially consume '%0' of non-frozen usableFromInline type %1 within a function annotated @_alwaysEmitIntoClient",
839+
(StringRef, Type))
834840
ERROR(sil_movechecking_cannot_destructure, none,
835841
"cannot partially consume '%0'",
836842
(StringRef))
837843
ERROR(sil_movechecking_cannot_partially_reinit_has_deinit, none,
838844
"cannot partially reinitialize '%0' when it has a deinitializer; only full reinitialization is allowed",
839845
(StringRef))
846+
ERROR(sil_movechecking_cannot_partially_reinit_nonfrozen, none,
847+
"cannot partially reinitialize '%0' of non-frozen type %1 imported from %2; only full reinitialization is allowed",
848+
(StringRef, Type, const ModuleDecl*))
849+
ERROR(sil_movechecking_cannot_partially_reinit_exported_usableFromInline_alwaysEmitIntoClient, none,
850+
"cannot partially reinitialize '%0' of non-frozen usableFromInline type %1 within a function annotated @_alwaysEmitIntoClient",
851+
(StringRef, Type))
840852
ERROR(sil_movechecking_cannot_partially_reinit, none,
841853
"cannot partially reinitialize '%0' after it has been consumed; only full reinitialization is allowed",
842854
(StringRef))

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,41 @@ struct CopiedLoadBorrowEliminationVisitor
16821682
// MARK: Partial Consume/Reinit Checking
16831683
//===----------------------------------------------------------------------===//
16841684

1685+
static bool hasExplicitFixedLayoutAnnotation(NominalTypeDecl *nominal) {
1686+
return nominal->getAttrs().hasAttribute<FixedLayoutAttr>() ||
1687+
nominal->getAttrs().hasAttribute<FrozenAttr>();
1688+
}
1689+
1690+
static std::optional<PartialMutationError>
1691+
shouldEmitPartialMutationErrorForType(SILType ty, NominalTypeDecl *nominal,
1692+
SILFunction *fn) {
1693+
// A non-frozen type can't be partially mutated within code built in its
1694+
// defining module if that code will be emitted into a client.
1695+
if (fn->getLinkage() == SILLinkage::PublicNonABI &&
1696+
nominal->isUsableFromInline() &&
1697+
!hasExplicitFixedLayoutAnnotation(nominal)) {
1698+
return {PartialMutationError::nonfrozenUsableFromInlineType(ty, *nominal)};
1699+
}
1700+
1701+
// Otherwise, a type can be mutated partially in its defining module.
1702+
if (nominal->getModuleContext() == fn->getModule().getSwiftModule())
1703+
return std::nullopt;
1704+
1705+
// It's defined in another module and used here; it has to be visible.
1706+
assert(nominal
1707+
->getFormalAccessScope(
1708+
/*useDC=*/fn->getDeclContext(),
1709+
/*treatUsableFromInlineAsPublic=*/true)
1710+
.isPublicOrPackage());
1711+
1712+
// Partial mutation is supported only for frozen/fixed-layout types from
1713+
// other modules.
1714+
if (hasExplicitFixedLayoutAnnotation(nominal))
1715+
return std::nullopt;
1716+
1717+
return {PartialMutationError::nonfrozenImportedType(ty, *nominal)};
1718+
}
1719+
16851720
/// Whether an error should be emitted in response to a partial consumption.
16861721
static llvm::Optional<PartialMutationError>
16871722
shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
@@ -1719,13 +1754,17 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
17191754

17201755
LLVM_DEBUG(llvm::dbgs() << " MoveOnlyPartialConsumption enabled!\n");
17211756

1722-
// Otherwise, walk the type looking for the deinit.
1757+
// Otherwise, walk the type looking for the deinit and visibility.
17231758
while (iterType != targetType) {
17241759
// If we have a nominal type as our parent type, see if it has a
17251760
// deinit. We know that it must be non-copyable since copyable types
17261761
// cannot contain non-copyable types and that our parent root type must be
17271762
// an enum, tuple, or struct.
17281763
if (auto *nom = iterType.getNominalOrBoundGenericNominal()) {
1764+
if (auto error = shouldEmitPartialMutationErrorForType(
1765+
iterType, nom, user->getFunction())) {
1766+
return error;
1767+
}
17291768
if (nom->getValueTypeDestructor()) {
17301769
// If we find one, emit an error since we are going to have to extract
17311770
// through the deinit. Emit a nice error saying what it is. Since we

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -834,12 +834,53 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
834834
}();
835835
diagnose(astContext, user, diagnostic, varName);
836836
registerDiagnosticEmitted(address);
837-
auto deinitLoc = error.getNominal().getValueTypeDestructor()->getLoc(
838-
/*SerializedOK=*/false);
837+
auto deinitLoc =
838+
error.getDeinitingNominal().getValueTypeDestructor()->getLoc(
839+
/*SerializedOK=*/false);
839840
if (!deinitLoc)
840841
return;
841842
astContext.Diags.diagnose(deinitLoc, diag::sil_movechecking_deinit_here);
842843
return;
843844
}
845+
case PartialMutationError::Kind::NonfrozenImportedType: {
846+
assert(
847+
astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
848+
astContext.LangOpts.hasFeature(
849+
Feature::MoveOnlyPartialReinitialization));
850+
auto &nominal = error.getNonfrozenImportedNominal();
851+
auto diagnostic = [&]() {
852+
switch (kind) {
853+
case PartialMutation::Kind::Consume:
854+
return diag::sil_movechecking_cannot_destructure_imported_nonfrozen;
855+
case PartialMutation::Kind::Reinit:
856+
return diag::sil_movechecking_cannot_partially_reinit_nonfrozen;
857+
}
858+
}();
859+
diagnose(astContext, user, diagnostic, varName,
860+
nominal.getDeclaredInterfaceType(), nominal.getModuleContext());
861+
registerDiagnosticEmitted(address);
862+
return;
863+
}
864+
case PartialMutationError::Kind::NonfrozenUsableFromInlineType: {
865+
assert(
866+
astContext.LangOpts.hasFeature(Feature::MoveOnlyPartialConsumption) ||
867+
astContext.LangOpts.hasFeature(
868+
Feature::MoveOnlyPartialReinitialization));
869+
auto &nominal = error.getNonfrozenUsableFromInlineNominal();
870+
auto diagnostic = [&]() {
871+
switch (kind) {
872+
case PartialMutation::Kind::Consume:
873+
return diag::
874+
sil_movechecking_cannot_destructure_exported_usableFromInline_alwaysEmitIntoClient;
875+
case PartialMutation::Kind::Reinit:
876+
return diag::
877+
sil_movechecking_cannot_partially_reinit_exported_usableFromInline_alwaysEmitIntoClient;
878+
}
879+
}();
880+
diagnose(astContext, user, diagnostic, varName,
881+
nominal.getDeclaredInterfaceType());
882+
registerDiagnosticEmitted(address);
883+
return;
884+
}
844885
}
845886
}

lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ inline Feature partialMutationFeature(PartialMutation::Kind kind) {
146146
/// enum Kind {
147147
/// case featureDisabled
148148
/// case hasDeinit(NominalTypeDecl)
149+
/// case nonfrozenImportedType(NominalTypeDecl)
150+
/// case nonfrozenUsableFromInlineType(NominalTypeDecl)
149151
/// }
150152
/// }
151153
class PartialMutationError {
@@ -155,7 +157,15 @@ class PartialMutationError {
155157
struct HasDeinit {
156158
NominalTypeDecl &nominal;
157159
};
158-
TaggedUnion<FeatureDisabled, HasDeinit> kind;
160+
struct NonfrozenImportedType {
161+
NominalTypeDecl &nominal;
162+
};
163+
struct NonfrozenUsableFromInlineType {
164+
NominalTypeDecl &nominal;
165+
};
166+
using Payload = TaggedUnion<FeatureDisabled, HasDeinit, NonfrozenImportedType,
167+
NonfrozenUsableFromInlineType>;
168+
Payload payload;
159169

160170
PartialMutationError(SILType type, Payload payload)
161171
: payload(payload), type(type) {}
@@ -182,12 +192,36 @@ class PartialMutationError {
182192
/// range and the full value. It is the \p nominal field of
183193
/// PartialMutationError.
184194
HasDeinit,
195+
/// A partially consumed/reinitialized aggregate is defined in a different
196+
/// module and isn't frozen.
197+
///
198+
/// In the case that the other module was built with library evolution, it
199+
/// isn't legal (or valid SIL) to directly modify the aggregate's fields.
200+
/// To have consistent semantics regardless of whether that other module was
201+
/// built with library evolution, prevent the aggregate from being partially
202+
/// mutated.
203+
NonfrozenImportedType,
204+
/// A partially consumed/reinitialized aggregate is defined in the same
205+
/// module, but the function where it is used is @_alwaysEmitIntoClient.
206+
///
207+
/// In this case, if the module were built with library evolution, it
208+
/// wouldn't be legal (or valid SIL) to directly modify the aggregate's
209+
/// fields within the function when it was included into some client
210+
/// library. To have consistent semantics regardless of having been built
211+
/// with library evolution, prevent the aggregate from being partially
212+
/// mutated.
213+
NonfrozenUsableFromInlineType,
185214
};
186215

187216
operator Kind() {
188-
if (kind.isa<FeatureDisabled>())
217+
if (payload.isa<FeatureDisabled>())
189218
return Kind::FeatureDisabled;
190-
return Kind::HasDeinit;
219+
else if (payload.isa<HasDeinit>())
220+
return Kind::HasDeinit;
221+
else if (payload.isa<NonfrozenImportedType>())
222+
return Kind::NonfrozenImportedType;
223+
assert(payload.isa<NonfrozenUsableFromInlineType>());
224+
return Kind::NonfrozenUsableFromInlineType;
191225
}
192226

193227
static PartialMutationError featureDisabled(SILType type,
@@ -200,8 +234,28 @@ class PartialMutationError {
200234
return PartialMutationError(type, HasDeinit{nominal});
201235
}
202236

203-
NominalTypeDecl &getNominal() { return kind.get<HasDeinit>().nominal; };
204-
PartialMutation::Kind getKind() { return kind.get<FeatureDisabled>().kind; };
237+
static PartialMutationError nonfrozenImportedType(SILType type,
238+
NominalTypeDecl &nominal) {
239+
return PartialMutationError(type, NonfrozenImportedType{nominal});
240+
}
241+
242+
static PartialMutationError
243+
nonfrozenUsableFromInlineType(SILType type, NominalTypeDecl &nominal) {
244+
return PartialMutationError(type, NonfrozenUsableFromInlineType{nominal});
245+
}
246+
247+
PartialMutation::Kind getKind() {
248+
return payload.get<FeatureDisabled>().kind;
249+
}
250+
NominalTypeDecl &getDeinitingNominal() {
251+
return payload.get<HasDeinit>().nominal;
252+
}
253+
NominalTypeDecl &getNonfrozenImportedNominal() {
254+
return payload.get<NonfrozenImportedType>().nominal;
255+
}
256+
NominalTypeDecl &getNonfrozenUsableFromInlineNominal() {
257+
return payload.get<NonfrozenUsableFromInlineType>().nominal;
258+
}
205259
};
206260
} // namespace siloptimizer
207261
} // namespace swift
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend \
3+
// RUN: %s \
4+
// RUN: -emit-sil -verify -verify-additional-prefix fragile- \
5+
// RUN: -sil-verify-all \
6+
// RUN: -enable-experimental-feature MoveOnlyPartialConsumption \
7+
// RUN: -module-name Library
8+
// RUN: %target-swift-frontend \
9+
// RUN: %s \
10+
// RUN: -emit-sil -verify -verify-additional-prefix resilient- \
11+
// RUN: -sil-verify-all \
12+
// RUN: -enable-library-evolution \
13+
// RUN: -enable-experimental-feature MoveOnlyPartialConsumption \
14+
// RUN: -module-name Library
15+
16+
public func take(_ u: consuming Ur) {}
17+
18+
public struct Ur : ~Copyable {
19+
@usableFromInline init() {}
20+
deinit {}
21+
}
22+
23+
@usableFromInline
24+
internal struct Agg_Internal_UFI : ~Copyable {
25+
@usableFromInline
26+
init(u1: consuming Ur, u2: consuming Ur) {
27+
self.u1 = u1
28+
self.u2 = u2
29+
}
30+
31+
@usableFromInline
32+
internal var u1: Ur
33+
@usableFromInline
34+
internal var u2: Ur
35+
}
36+
37+
@_alwaysEmitIntoClient
38+
func piecewise_Agg_Internal_UFI(_ a: consuming Agg_Internal_UFI) {
39+
take(a.u1) // expected-fragile-error{{cannot partially consume 'a' of non-frozen usableFromInline type 'Agg_Internal_UFI' within a function annotated @_alwaysEmitIntoClient}}
40+
// expected-resilient-error@-1{{field 'a.u1' was consumed but not reinitialized; the field must be reinitialized during the access}}
41+
// expected-resilient-note@-2{{consumed here}}
42+
take(a.u2) // expected-fragile-error{{cannot partially consume 'a' of non-frozen usableFromInline type 'Agg_Internal_UFI' within a function annotated @_alwaysEmitIntoClient}}
43+
// expected-resilient-error@-1{{field 'a.u2' was consumed but not reinitialized; the field must be reinitialized during the access}}
44+
// expected-resilient-note@-2{{consumed here}}
45+
}
46+
47+
@_alwaysEmitIntoClient
48+
func get_Agg_Internal_UFI() -> Agg_Internal_UFI {
49+
return Agg_Internal_UFI(u1: Ur(), u2: Ur())
50+
}
51+
52+
@_alwaysEmitIntoClient
53+
public func call_piecewise_Agg_Internal_UFI() {
54+
let a = get_Agg_Internal_UFI()
55+
piecewise_Agg_Internal_UFI(a)
56+
}
57+

0 commit comments

Comments
 (0)