Skip to content

Commit

Permalink
Fix a number of bugs with the new materializeForSet SILGen,
Browse files Browse the repository at this point in the history 
including one which triggered only for non-subscripts and
a number which triggered with different cases of classes.

Swift SVN r31198
  • Loading branch information
@rjmccall
rjmccall committed Aug 13, 2015
1 parent e56e489 commit 7e81f7e
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 8 deletions.
4 changes: 4 additions & 0 deletions lib/SILGen/LValue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,10 @@ class LValue {
AbstractionPattern origFormalType,
CanType substFormalType);

/// Form a class-reference l-value. Only suitable as the base of
/// very specific member components.
static LValue forClassReference(ManagedValue reference);

bool isValid() const { return !Path.empty(); }

/// Is this lvalue purely physical?
Expand Down
10 changes: 7 additions & 3 deletions lib/SILGen/SILGenApply.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3689,11 +3689,15 @@ ArgumentSource SILGenFunction::prepareAccessorBaseArg(SILLocation loc,
//
// However, when the base is a reference type and the target is
// a non-class protocol, this is innocuous.
#ifndef NDEBUG
auto isNonClassProtocolMember = [](Decl *d) {
auto p = d->getDeclContext()->isProtocolOrProtocolExtensionContext();
return (p && !p->requiresClass());
};
#endif
assert((!selfParam.isIndirectInOut() ||
(baseFormalType->isAnyClassReferenceType() &&
isa<ProtocolDecl>(accessor.getDecl()->getDeclContext()) &&
!cast<ProtocolDecl>(accessor.getDecl()->getDeclContext())
->requiresClass())) &&
isNonClassProtocolMember(accessor.getDecl()))) &&
"passing unmaterialized r-value as inout argument");

base = emitMaterializeIntoTemporary(*this, loc, base);
Expand Down
63 changes: 58 additions & 5 deletions lib/SILGen/SILGenLValue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,21 @@ LValue LValue::forAddress(ManagedValue address,
return lv;
}

LValue LValue::forClassReference(ManagedValue ref) {
assert(ref.isPlusZeroRValueOrTrivial());
assert(ref.getType().isObject());
assert(ref.getType().getSwiftRValueType()->mayHaveSuperclass());

CanType classType = ref.getType().getSwiftRValueType();
LValueTypeData typeData = {
AbstractionPattern(classType), classType, ref.getType()
};

LValue lv;
lv.add<ValueComponent>(ref, typeData);
return lv;
}

void LValue::addMemberComponent(SILGenFunction &gen, SILLocation loc,
AbstractStorageDecl *storage,
ArrayRef<Substitution> subs,
Expand Down Expand Up @@ -2739,8 +2754,9 @@ struct MaterializeForSetEmitter {
.getObjectType();

// In a protocol witness thunk, we always want to use ordinary
// access semantics. But when we're changing this to use For standard implementations, we'll need to
// modify this to use direct semantics.
// access semantics. But when we're changing for standard
// implementations, we'll need to modify this to use direct
// semantics.
TheAccessSemantics = AccessSemantics::Ordinary;
IsSuper = false;
}
Expand Down Expand Up @@ -2792,12 +2808,49 @@ struct MaterializeForSetEmitter {
RValue collectIndicesFromParameters(SILGenFunction &gen, SILLocation loc,
ArrayRef<ManagedValue> sourceIndices);

LValue buildSelfLValue(SILGenFunction &gen, SILLocation loc,
ManagedValue self) {
// All of the complexity here is tied up with class types. If the
// substituted type isn't a reference type, then we can't have a
// class-bounded protocol or inheritance, and the simple case just
// works.
AbstractionPattern selfPattern(SubstSelfType);
if (!SubstSelfType->mayHaveSuperclass()) {
return LValue::forAddress(self, selfPattern, SubstSelfType);
}

CanType witnessSelfType =
Witness->computeInterfaceSelfType(false)->getCanonicalType();
witnessSelfType = getSubstWitnessInterfaceType(witnessSelfType);
if (auto selfTuple = dyn_cast<TupleType>(witnessSelfType)) {
assert(selfTuple->getNumElements() == 1);
witnessSelfType = selfTuple.getElementType(0);
}
witnessSelfType = witnessSelfType.getLValueOrInOutObjectType();

// Eagerly loading here could cause an unnecessary
// load+materialize in some cases, but it's not really important.
SILValue selfValue = self.getValue();
if (selfValue.getType().isAddress()) {
selfValue = gen.B.createLoad(loc, selfValue);
}

// Do a derived-to-base conversion if necessary.
if (witnessSelfType != SubstSelfType) {
auto selfSILType = SILType::getPrimitiveObjectType(witnessSelfType);
selfValue = gen.B.createUpcast(loc, selfValue, selfSILType);
}

// Recreate as a borrowed value.
self = ManagedValue::forUnmanaged(selfValue);
return LValue::forClassReference(self);
}

LValue buildLValue(SILGenFunction &gen, SILLocation loc,
ManagedValue self, RValue &&indices,
AccessKind accessKind) {
// Begin with the 'self' value.
AbstractionPattern selfPattern(SubstSelfType);
LValue lv = LValue::forAddress(self, selfPattern, SubstSelfType);
LValue lv = buildSelfLValue(gen, loc, self);

auto strategy =
WitnessStorage->getAccessStrategy(TheAccessSemantics, accessKind);
Expand Down Expand Up @@ -3212,7 +3265,7 @@ MaterializeForSetEmitter::emitUsingGetterSetter(SILGenFunction &gen,
SILFunction *&callback) {
// Copy the indices into the callback storage.
const TypeLowering *indicesTL = nullptr;
CleanupHandle indicesCleanup;
CleanupHandle indicesCleanup = CleanupHandle::invalid();
CanType indicesFormalType;
if (isa<SubscriptDecl>(WitnessStorage)) {
indicesFormalType = indices.getType();
Expand Down
115 changes: 115 additions & 0 deletions test/SILGen/materializeForSet.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,121 @@ class Base {
set(value) {}
}

var storedFunction: () -> Int = { 0 }
final var finalStoredFunction: () -> Int = { 0 }
var computedFunction: () -> Int {
get { return {0} }
set {}
}
}

class Derived : Base {}

protocol Abstractable {
typealias Result
var storedFunction: () -> Result { get set }
var finalStoredFunction: () -> Result { get set }
var computedFunction: () -> Result { get set }
}

// Validate that we thunk materializeForSet correctly when there's
// an abstraction pattern present.

extension Derived : Abstractable {}
// SILGEN: sil hidden [transparent] [thunk] @_TTWC17materializeForSet7DerivedS_12AbstractableS_FS1_m14storedFunctionFT_qq_S1_6Result :
// SILGEN: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $*Derived):
// SILGEN-NEXT: [[RESULT_ADDR:%.*]] = pointer_to_address %0 : $Builtin.RawPointer to $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: [[T0:%.*]] = load %2 : $*Derived
// SILGEN-NEXT: [[SELF:%.*]] = upcast [[T0]] : $Derived to $Base
// SILGEN-NEXT: [[TEMP:%.*]] = alloc_stack $@callee_owned () -> Int
// SILGEN-NEXT: [[FN:%.*]] = class_method [[SELF]] : $Base, #Base.storedFunction!getter.1
// SILGEN-NEXT: [[RESULT:%.*]] = apply [[FN]]([[SELF]])
// SILGEN-NEXT: store [[RESULT]] to [[TEMP]]
// SILGEN-NEXT: [[RESULT:%.*]] = load [[TEMP]]
// SILGEN-NEXT: strong_retain [[RESULT]]
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[REABSTRACTOR:%.*]] = function_ref @_TTRXFo__dSi_XFo__iSi_ : $@convention(thin) (@out Int, @owned @callee_owned () -> Int) -> ()
// SILGEN-NEXT: [[T1:%.*]] = partial_apply [[REABSTRACTOR]]([[RESULT]])
// SILGEN-NEXT: store [[T1]] to [[RESULT_ADDR]]
// SILGEN-NEXT: [[RESULT_PTR:%.*]] = address_to_pointer [[RESULT_ADDR]] : $*@callee_owned (@out Int) -> () to $Builtin.RawPointer
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[T0:%.*]] = function_ref @_TTWC17materializeForSet7DerivedS_12AbstractableS_FFCS_4Basem14storedFunctionFT_SiU_XfTBpRBBRS2_XMTS2__T_
// SILGEN-NEXT: [[CALLBACK:%.*]] = enum $Optional<@convention(thin) (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, inout Derived, @thick Derived.Type) -> ()>, #Optional.Some!enumelt.1, [[T0]]
// SILGEN-NEXT: [[T0:%.*]] = tuple ([[RESULT_PTR]] : $Builtin.RawPointer, [[CALLBACK]] : $Optional<@convention(thin) (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, inout Derived, @thick Derived.Type) -> ()>)
// SILGEN-NEXT: destroy_addr [[TEMP]]
// SILGEN-NEXT: dealloc_stack [[TEMP]]
// SILGEN-NEXT: return [[T0]]

// SILGEN: sil hidden [transparent] @_TTWC17materializeForSet7DerivedS_12AbstractableS_FFCS_4Basem14storedFunctionFT_SiU_XfTBpRBBRS2_XMTS2__T_ : $@convention(thin) (Builtin.RawPointer, @inout Builtin.UnsafeValueBuffer, @inout Derived, @thick Derived.Type) -> ()
// SILGEN: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $*Derived, %3 : $@thick Derived.Type):
// SILGEN-NEXT: [[T0:%.*]] = load %2 : $*Derived
// SILGEN-NEXT: [[SELF:%.*]] = upcast [[T0]] : $Derived to $Base
// SILGEN-NEXT: [[RESULT_ADDR:%.*]] = pointer_to_address %0 : $Builtin.RawPointer to $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: [[VALUE:%.*]] = load [[RESULT_ADDR]] : $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[REABSTRACTOR:%.*]] = function_ref @_TTRXFo__iSi_XFo__dSi_ : $@convention(thin) (@owned @callee_owned (@out Int) -> ()) -> Int
// SILGEN-NEXT: [[NEWVALUE:%.*]] = partial_apply [[REABSTRACTOR]]([[VALUE]])
// SILGEN-NEXT: [[FN:%.*]] = class_method [[SELF]] : $Base, #Base.storedFunction!setter.1 : Base -> (() -> Int) -> ()
// SILGEN-NEXT: apply [[FN]]([[NEWVALUE]], [[SELF]])
// SILGEN-NEXT: tuple ()
// SILGEN-NEXT: return

// SILGEN: sil hidden [transparent] [thunk] @_TTWC17materializeForSet7DerivedS_12AbstractableS_FS1_m19finalStoredFunctionFT_qq_S1_6Result :
// SILGEN: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $*Derived):
// SILGEN-NEXT: [[RESULT_ADDR:%.*]] = pointer_to_address %0 : $Builtin.RawPointer to $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: [[T0:%.*]] = load %2 : $*Derived
// SILGEN-NEXT: [[SELF:%.*]] = upcast [[T0]] : $Derived to $Base
// SILGEN-NEXT: [[ADDR:%.*]] = ref_element_addr [[SELF]] : $Base, #Base.finalStoredFunction
// SILGEN-NEXT: [[RESULT:%.*]] = load [[ADDR]]
// SILGEN-NEXT: strong_retain [[RESULT]]
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[REABSTRACTOR:%.*]] = function_ref @_TTRXFo__dSi_XFo__iSi_ : $@convention(thin) (@out Int, @owned @callee_owned () -> Int) -> ()
// SILGEN-NEXT: [[T1:%.*]] = partial_apply [[REABSTRACTOR]]([[RESULT]])
// SILGEN-NEXT: store [[T1]] to [[RESULT_ADDR]]
// SILGEN-NEXT: [[RESULT_PTR:%.*]] = address_to_pointer [[RESULT_ADDR]] : $*@callee_owned (@out Int) -> () to $Builtin.RawPointer
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[T0:%.*]] = function_ref @_TTWC17materializeForSet7DerivedS_12AbstractableS_FFCS_4Basem19finalStoredFunctionFT_SiU_XfTBpRBBRS2_XMTS2__T_
// SILGEN-NEXT: [[CALLBACK:%.*]] = enum $Optional<@convention(thin) (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, inout Derived, @thick Derived.Type) -> ()>, #Optional.Some!enumelt.1, [[T0]]
// SILGEN-NEXT: [[T0:%.*]] = tuple ([[RESULT_PTR]] : $Builtin.RawPointer, [[CALLBACK]] : $Optional<@convention(thin) (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, inout Derived, @thick Derived.Type) -> ()>)
// SILGEN-NEXT: return [[T0]]

// SILGEN: sil hidden [transparent] @_TTWC17materializeForSet7DerivedS_12AbstractableS_FFCS_4Basem19finalStoredFunctionFT_SiU_XfTBpRBBRS2_XMTS2__T_ :
// SILGEN: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $*Derived, %3 : $@thick Derived.Type):
// SILGEN-NEXT: [[T0:%.*]] = load %2 : $*Derived
// SILGEN-NEXT: [[SELF:%.*]] = upcast [[T0]] : $Derived to $Base
// SILGEN-NEXT: [[RESULT_ADDR:%.*]] = pointer_to_address %0 : $Builtin.RawPointer to $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: [[VALUE:%.*]] = load [[RESULT_ADDR]] : $*@callee_owned (@out Int) -> ()
// SILGEN-NEXT: function_ref
// SILGEN-NEXT: [[REABSTRACTOR:%.*]] = function_ref @_TTRXFo__iSi_XFo__dSi_ : $@convention(thin) (@owned @callee_owned (@out Int) -> ()) -> Int
// SILGEN-NEXT: [[NEWVALUE:%.*]] = partial_apply [[REABSTRACTOR]]([[VALUE]])
// SILGEN-NEXT: [[ADDR:%.*]] = ref_element_addr [[SELF]] : $Base, #Base.finalStoredFunction
// SILGEN-NEXT: assign [[NEWVALUE]] to [[ADDR]]
// SILGEN-NEXT: tuple ()
// SILGEN-NEXT: return

protocol ClassAbstractable : class {
typealias Result
var storedFunction: () -> Result { get set }
var finalStoredFunction: () -> Result { get set }
var computedFunction: () -> Result { get set }
}
extension Derived : ClassAbstractable {}

protocol Signatures {
typealias Result
var computedFunction: () -> Result { get set }
}
protocol Implementations {}
extension Implementations {
var computedFunction: () -> Int {
get { return {0} }
set {}
}
}

class ImplementingClass : Implementations, Signatures {}
struct ImplementingStruct : Implementations, Signatures {
var ref: ImplementingClass?
}

class HasDidSet : Base {
Expand Down

0 comments on commit 7e81f7e

Please sign in to comment.