Skip to content

Commit 80a7ae1

Browse files
committed
SIL: Treat __shared and __owned just like inout when lowering function types
Even with an opaque abstraction pattern, we must explode a parameter list containing __shared and __owned elements. Otherwise, we produce invalid lowered SIL types. Note that this is already an issue, because the stdlib has a handful of declarations using __owned.
1 parent 3f4227e commit 80a7ae1

File tree

6 files changed

+44
-30
lines changed

6 files changed

+44
-30
lines changed

include/swift/AST/Types.h

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -361,8 +361,9 @@ class alignas(1 << TypeAlignInBits) TypeBase {
361361
);
362362

363363
SWIFT_INLINE_BITFIELD_FULL(TupleType, TypeBase, 1+32,
364-
/// Whether an element of the tuple is inout.
365-
HasInOutElement : 1,
364+
/// Whether an element of the tuple is inout, __shared or __owned.
365+
/// Values cannot have such tuple types in the language.
366+
HasElementWithOwnership : 1,
366367

367368
: NumPadBits,
368369

@@ -1876,9 +1877,9 @@ class TupleType final : public TypeBase, public llvm::FoldingSetNode,
18761877
/// return the element index, otherwise return -1.
18771878
int getNamedElementId(Identifier I) const;
18781879

1879-
/// Returns true if this tuple has inout elements.
1880-
bool hasInOutElement() const {
1881-
return static_cast<bool>(Bits.TupleType.HasInOutElement);
1880+
/// Returns true if this tuple has inout, __shared or __owned elements.
1881+
bool hasElementWithOwnership() const {
1882+
return static_cast<bool>(Bits.TupleType.HasElementWithOwnership);
18821883
}
18831884

18841885
// Implement isa/cast/dyncast/etc.
@@ -1895,9 +1896,9 @@ class TupleType final : public TypeBase, public llvm::FoldingSetNode,
18951896
private:
18961897
TupleType(ArrayRef<TupleTypeElt> elements, const ASTContext *CanCtx,
18971898
RecursiveTypeProperties properties,
1898-
bool hasInOut)
1899+
bool hasElementWithOwnership)
18991900
: TypeBase(TypeKind::Tuple, CanCtx, properties) {
1900-
Bits.TupleType.HasInOutElement = hasInOut;
1901+
Bits.TupleType.HasElementWithOwnership = hasElementWithOwnership;
19011902
Bits.TupleType.Count = elements.size();
19021903
std::uninitialized_copy(elements.begin(), elements.end(),
19031904
getTrailingObjects<TupleTypeElt>());
@@ -4980,13 +4981,13 @@ inline bool TypeBase::isTypeParameter() {
49804981
inline bool TypeBase::isMaterializable() {
49814982
if (hasLValueType())
49824983
return false;
4983-
4984+
49844985
if (is<InOutType>())
49854986
return false;
4986-
4987+
49874988
if (auto *TTy = getAs<TupleType>())
4988-
return !TTy->hasInOutElement();
4989-
4989+
return !TTy->hasElementWithOwnership();
4990+
49904991
return true;
49914992
}
49924993

lib/AST/ASTContext.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2974,7 +2974,7 @@ Type TupleType::get(ArrayRef<TupleTypeElt> Fields, const ASTContext &C) {
29742974
Fields[0].getParameterFlags());
29752975

29762976
RecursiveTypeProperties properties;
2977-
bool hasInOut = false;
2977+
bool hasElementWithOwnership = false;
29782978
for (const TupleTypeElt &Elt : Fields) {
29792979
auto eltTy = Elt.getType();
29802980
if (!eltTy) continue;
@@ -2984,11 +2984,13 @@ Type TupleType::get(ArrayRef<TupleTypeElt> Fields, const ASTContext &C) {
29842984
// non-paren tuples are malformed and will be diagnosed later.
29852985
if (auto *TTy = Elt.getType()->getAs<TupleType>()) {
29862986
if (TTy->getNumElements() == 1)
2987-
hasInOut |= TTy->hasInOutElement();
2987+
hasElementWithOwnership |= TTy->hasElementWithOwnership();
29882988
} else if (auto *Pty = dyn_cast<ParenType>(Elt.getType().getPointer())) {
2989-
hasInOut |= Pty->getParameterFlags().isInOut();
2989+
hasElementWithOwnership |= (Pty->getParameterFlags().getValueOwnership() !=
2990+
ValueOwnership::Default);
29902991
} else {
2991-
hasInOut |= Elt.getParameterFlags().isInOut();
2992+
hasElementWithOwnership |= (Elt.getParameterFlags().getValueOwnership() !=
2993+
ValueOwnership::Default);
29922994
}
29932995
}
29942996

@@ -3016,7 +3018,7 @@ Type TupleType::get(ArrayRef<TupleTypeElt> Fields, const ASTContext &C) {
30163018
sizeof(TupleTypeElt) * Fields.size(),
30173019
alignof(TupleType), arena);
30183020
auto New = new (mem) TupleType(Fields, IsCanonical ? &C : nullptr, properties,
3019-
hasInOut);
3021+
hasElementWithOwnership);
30203022
C.getImpl().getArena(arena).TupleTypes.InsertNode(New, InsertPos);
30213023
return New;
30223024
}

lib/SIL/SILFunctionType.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -604,9 +604,10 @@ class DestructureInputs {
604604
ParameterTypeFlags paramFlags, CanType ty) {
605605
CanTupleType tty = dyn_cast<TupleType>(ty);
606606
// If the abstraction pattern is opaque, and the tuple type is
607-
// materializable -- if it doesn't contain an l-value type -- then it's
608-
// a valid target for substitution and we should not expand it.
609-
if (!tty || (pattern.isTypeParameter() && !tty->hasInOutElement())) {
607+
// a valid target for substitution, don't expand it.
608+
if (!tty ||
609+
(pattern.isTypeParameter() &&
610+
!tty->hasElementWithOwnership())) {
610611
visit(paramFlags.getValueOwnership(), /*forSelf=*/false, pattern, ty,
611612
silRepresentation);
612613
return;

lib/SILGen/SILGenApply.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2540,7 +2540,7 @@ class ArgEmitter {
25402540

25412541
bool isUnmaterializableTupleType(CanType type) {
25422542
if (auto tuple = dyn_cast<TupleType>(type))
2543-
if (tuple->hasInOutElement())
2543+
if (tuple->hasElementWithOwnership())
25442544
return true;
25452545
return false;
25462546
}

lib/SILGen/SILGenPoly.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -872,8 +872,8 @@ namespace {
872872
}
873873
}
874874

875-
// Special-case: tuples containing inouts.
876-
if (inputTupleType && inputTupleType->hasInOutElement()) {
875+
// Special-case: tuples containing inouts, __shared or __owned.
876+
if (inputTupleType && inputTupleType->hasElementWithOwnership()) {
877877
// Non-materializable tuple types cannot be bound as generic
878878
// arguments, so none of the remaining transformations apply.
879879
// Instead, the outermost tuple layer is exploded, even when
@@ -1056,8 +1056,8 @@ namespace {
10561056
CanTupleType inputTupleType,
10571057
AbstractionPattern outputOrigType,
10581058
CanTupleType outputTupleType) {
1059-
assert(!inputTupleType->hasInOutElement() &&
1060-
!outputTupleType->hasInOutElement());
1059+
assert(!inputTupleType->hasElementWithOwnership() &&
1060+
!outputTupleType->hasElementWithOwnership());
10611061
assert(inputTupleType->getNumElements() ==
10621062
outputTupleType->getNumElements());
10631063

@@ -1149,8 +1149,8 @@ namespace {
11491149
// when witness method thunks re-abstract a non-mutating
11501150
// witness for a mutating requirement. The inout self is just
11511151
// loaded to produce a value in this case.
1152-
assert(inputSubstType->hasInOutElement() ||
1153-
!outputSubstType->hasInOutElement());
1152+
assert(inputSubstType->hasElementWithOwnership() ||
1153+
!outputSubstType->hasElementWithOwnership());
11541154
assert(inputSubstType->getNumElements() ==
11551155
outputSubstType->getNumElements());
11561156

@@ -1171,8 +1171,8 @@ namespace {
11711171
ManagedValue inputTupleAddr) {
11721172
assert(inputOrigType.isTypeParameter());
11731173
assert(outputOrigType.matchesTuple(outputSubstType));
1174-
assert(!inputSubstType->hasInOutElement() &&
1175-
!outputSubstType->hasInOutElement());
1174+
assert(!inputSubstType->hasElementWithOwnership() &&
1175+
!outputSubstType->hasElementWithOwnership());
11761176
assert(inputSubstType->getNumElements() ==
11771177
outputSubstType->getNumElements());
11781178

@@ -1218,8 +1218,8 @@ namespace {
12181218
TemporaryInitialization &tupleInit) {
12191219
assert(inputOrigType.matchesTuple(inputSubstType));
12201220
assert(outputOrigType.matchesTuple(outputSubstType));
1221-
assert(!inputSubstType->hasInOutElement() &&
1222-
!outputSubstType->hasInOutElement());
1221+
assert(!inputSubstType->hasElementWithOwnership() &&
1222+
!outputSubstType->hasElementWithOwnership());
12231223
assert(inputSubstType->getNumElements() ==
12241224
outputSubstType->getNumElements());
12251225

test/SILGen/reabstract.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,13 @@ closureTakingOptional({ (_: Any) -> () in })
103103
// CHECK: [[OPTADDR:%.*]] = init_existential_addr [[ANYADDR]] : $*Any, $Optional<Int>
104104
// CHECK: store %0 to [trivial] [[OPTADDR]] : $*Optional<Int>
105105
// CHECK: apply %1([[ANYADDR]]) : $@noescape @callee_guaranteed (@in_guaranteed Any) -> ()
106+
107+
// Same behavior as above with other ownership qualifiers.
108+
func evenLessFun(_ s: __shared C, _ o: __owned C) {}
109+
110+
// CHECK-LABEL: sil shared [transparent] [serializable] [reabstraction_thunk] @$S10reabstract1CCACIeggx_A2CytIegnir_TR : $@convention(thin) (@in_guaranteed C, @in C, @guaranteed @callee_guaranteed (@guaranteed C, @owned C) -> ()) -> @out ()
111+
// CHECK-LABEL: sil shared [transparent] [serializable] [reabstraction_thunk] @$S10reabstract1CCACytIegnir_A2CIeggx_TR : $@convention(thin) (@guaranteed C, @owned C, @guaranteed @callee_guaranteed (@in_guaranteed C, @in C) -> @out ()) -> ()
112+
func testSharedOwnedOpaque(_ s: C, o: C) {
113+
let box = Box(t: evenLessFun)
114+
box.t(s, o)
115+
}

0 commit comments

Comments
 (0)