From ad01c1e92927da042e1afd1790dd352de2d32641 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 22 Dec 2016 18:50:27 -0500 Subject: [PATCH 1/8] SILGen: Implement missing function conversions from tuples to Any Fixes and . --- lib/SILGen/SILGenPoly.cpp | 145 +++++++++++++----- test/SILGen/function_conversion.swift | 101 +++++++++++- .../0045-sr3267.swift | 2 +- ...-irgensilfunction-visitfullapplysite.swift | 2 +- 4 files changed, 211 insertions(+), 39 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/0045-sr3267.swift (57%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28293-irgensilfunction-visitfullapplysite.swift (89%) diff --git a/lib/SILGen/SILGenPoly.cpp b/lib/SILGen/SILGenPoly.cpp index 1b061cc8c75fb..8b33aaa25b396 100644 --- a/lib/SILGen/SILGenPoly.cpp +++ b/lib/SILGen/SILGenPoly.cpp @@ -876,17 +876,49 @@ namespace { // Tuple types are subtypes of their optionals if (auto outputObjectType = outputSubstType.getAnyOptionalObjectType()) { - // The input is exploded and the output is an optional tuple. - // Translate values and collect them into a single optional - // payload. - auto outputTupleType = cast(outputObjectType); + auto outputOrigObjectType = outputOrigType.getAnyOptionalObjectType(); + + if (auto outputTupleType = dyn_cast(outputObjectType)) { + // The input is exploded and the output is an optional tuple. + // Translate values and collect them into a single optional + // payload. + + auto result = + translateAndImplodeIntoOptional(inputOrigType, + inputTupleType, + outputOrigObjectType, + outputTupleType); + Outputs.push_back(result); + return; + } + + // Tuple types are subtypes of optionals of Any, too. + assert(outputObjectType->isAny()); + + // First, construct the existential. + auto result = + translateAndImplodeIntoAny(inputOrigType, + inputTupleType, + outputOrigObjectType, + outputObjectType); + + // Now, convert it to an optional. + translateSingle(outputOrigObjectType, outputObjectType, + outputOrigType, outputSubstType, + result, claimNextOutputType()); + return; + } - return translateAndImplodeIntoOptional(inputOrigType, - inputTupleType, - outputOrigType.getAnyOptionalObjectType(), - outputTupleType); + if (outputSubstType->isAny()) { + claimNextOutputType(); - // FIXME: optional of Any (ugh...) + auto result = + translateAndImplodeIntoAny(inputOrigType, + inputTupleType, + outputOrigType, + outputSubstType); + Outputs.push_back(result); + return; } if (outputTupleType) { @@ -908,7 +940,6 @@ namespace { return; } - // FIXME: Tuple-to-Any conversions llvm_unreachable("Unhandled conversion from exploded tuple"); } @@ -995,10 +1026,11 @@ namespace { /// Handle a tuple that has been exploded in the input but wrapped in /// an optional in the output. - void translateAndImplodeIntoOptional(AbstractionPattern inputOrigType, - CanTupleType inputTupleType, - AbstractionPattern outputOrigType, - CanTupleType outputTupleType) { + ManagedValue + translateAndImplodeIntoOptional(AbstractionPattern inputOrigType, + CanTupleType inputTupleType, + AbstractionPattern outputOrigType, + CanTupleType outputTupleType) { assert(!inputTupleType->hasInOut() && !outputTupleType->hasInOut()); assert(inputTupleType->getNumElements() == @@ -1018,7 +1050,7 @@ namespace { optionalTy = SGF.F.mapTypeIntoContext(optionalTy); auto optional = SGF.B.createEnum(Loc, payload.getValue(), someDecl, optionalTy); - Outputs.push_back(ManagedValue(optional, payload.getCleanup())); + return ManagedValue(optional, payload.getCleanup()); } else { auto optionalBuf = SGF.emitTemporaryAllocation(Loc, optionalTy); auto tupleBuf = SGF.B.createInitEnumDataAddr(Loc, optionalBuf, someDecl, @@ -1033,10 +1065,38 @@ namespace { SGF.B.createInjectEnumAddr(Loc, optionalBuf, someDecl); auto payload = tupleTemp->getManagedAddress(); - Outputs.push_back(ManagedValue(optionalBuf, payload.getCleanup())); + return ManagedValue(optionalBuf, payload.getCleanup()); } } - + + /// Handle a tuple that has been exploded in the input but wrapped + /// in an existential in the output. + ManagedValue + translateAndImplodeIntoAny(AbstractionPattern inputOrigType, + CanTupleType inputTupleType, + AbstractionPattern outputOrigType, + CanType outputSubstType) { + auto existentialTy = SGF.getLoweredType(outputOrigType, outputSubstType); + auto existentialBuf = SGF.emitTemporaryAllocation(Loc, existentialTy); + + auto opaque = AbstractionPattern::getOpaque(); + auto &concreteTL = SGF.getTypeLowering(opaque, inputTupleType); + + auto tupleBuf = + SGF.B.createInitExistentialAddr(Loc, existentialBuf, + inputTupleType, + concreteTL.getLoweredType(), + /*conformances=*/{}); + + auto tupleTemp = SGF.useBufferAsTemporary(tupleBuf, concreteTL); + translateAndImplodeInto(inputOrigType, inputTupleType, + opaque, inputTupleType, + *tupleTemp); + + auto payload = tupleTemp->getManagedAddress(); + return ManagedValue(existentialBuf, payload.getCleanup()); + } + /// Handle a tuple that has been exploded in both the input and /// the output. void translateParallelExploded(AbstractionPattern inputOrigType, @@ -1643,11 +1703,10 @@ void ResultPlanner::plan(AbstractionPattern innerOrigType, CanType outerSubstType, PlanData &planData) { // The substituted types must match up in tuple-ness and arity. - // (Existential erasure could complicate this if we add that as a subtyping - // relationship.) assert(isa(innerSubstType) == isa(outerSubstType) || (isa(innerSubstType) && - outerSubstType->getAnyOptionalObjectType())); + (outerSubstType->isAny() || + outerSubstType->getAnyOptionalObjectType()))); assert(!isa(outerSubstType) || cast(innerSubstType)->getNumElements() == cast(outerSubstType)->getNumElements()); @@ -1772,26 +1831,40 @@ ResultPlanner::planTupleIntoIndirectResult(AbstractionPattern innerOrigType, // Figure out what kind of optional it is. CanType outerSubstObjectType = outerSubstType.getAnyOptionalObjectType(); - assert(outerSubstObjectType && - "inner type was a tuple but outer type was neither a tuple nor " - "optional"); - auto someDecl = Gen.getASTContext().getOptionalSomeDecl(); + if (outerSubstObjectType) { + auto someDecl = Gen.getASTContext().getOptionalSomeDecl(); + + // Prepare the value slot in the optional value. + SILType outerObjectType = + outerResultAddr->getType().getAnyOptionalObjectType(); + SILValue outerObjectResultAddr + = Gen.B.createInitEnumDataAddr(Loc, outerResultAddr, someDecl, + outerObjectType); + + // Emit into that address. + planTupleIntoIndirectResult(innerOrigType, innerSubstType, + outerOrigType.getAnyOptionalObjectType(), + outerSubstObjectType, + planData, outerObjectResultAddr); + + // Add an operation to finish the enum initialization. + addInjectOptionalIndirect(someDecl, outerResultAddr); + return; + } - // Prepare the value slot in the optional value. - SILType outerObjectType = - outerResultAddr->getType().getAnyOptionalObjectType(); - SILValue outerObjectResultAddr - = Gen.B.createInitEnumDataAddr(Loc, outerResultAddr, someDecl, - outerObjectType); + assert(outerSubstType->isAny()); + + // Prepare the value slot in the existential. + auto opaque = AbstractionPattern::getOpaque(); + SILValue outerConcreteResultAddr + = Gen.B.createInitExistentialAddr(Loc, outerResultAddr, innerSubstType, + Gen.getLoweredType(opaque, innerSubstType), + /*conformances=*/{}); // Emit into that address. planTupleIntoIndirectResult(innerOrigType, innerSubstType, - outerOrigType.getAnyOptionalObjectType(), - outerSubstObjectType, - planData, outerObjectResultAddr); - - // Add an operation to finish the enum initialization. - addInjectOptionalIndirect(someDecl, outerResultAddr); + innerOrigType, innerSubstType, + planData, outerConcreteResultAddr); return; } diff --git a/test/SILGen/function_conversion.swift b/test/SILGen/function_conversion.swift index 800c3427e4d76..32520170d4822 100644 --- a/test/SILGen/function_conversion.swift +++ b/test/SILGen/function_conversion.swift @@ -439,9 +439,108 @@ func convTupleScalarOpaque(_ f: @escaping (T...) -> ()) -> ((_ args: T...) -> // CHECK: apply {{.*}} // CHECK: return - func convAnyHashable(t: T) { let fn: (T, T) -> Bool = { (x: AnyHashable, y: AnyHashable) in x == y } } + +// ==== Convert exploded tuples to Any or Optional + +// CHECK-LABEL: sil hidden @_TF19function_conversion12convTupleAnyFTFT_T_FT_TSiSi_FP_T_FGSqP__T__T_ +// CHECK: function_ref @_TTRXFo___XFo__iP__ +// CHECK: partial_apply +// CHECK: function_ref @_TTRXFo___XFo__iGSqP___ +// CHECK: partial_apply +// CHECK: function_ref @_TTRXFo__dSidSi_XFo__iP__ +// CHECK: partial_apply +// CHECK: function_ref @_TTRXFo__dSidSi_XFo__iGSqP___ +// CHECK: partial_apply +// CHECK: function_ref @_TTRXFo_iP___XFo_dSidSi__ +// CHECK: partial_apply +// CHECK: function_ref @_TTRXFo_iGSqP____XFo_dSidSi__ +// CHECK: partial_apply + +func convTupleAny(_ f1: @escaping () -> (), + _ f2: @escaping () -> (Int, Int), + _ f3: @escaping (Any) -> (), + _ f4: @escaping (Any?) -> ()) { + let _: () -> Any = f1 + let _: () -> Any? = f1 + + let _: () -> Any = f2 + let _: () -> Any? = f2 + + let _: ((Int, Int)) -> () = f3 + + let _: ((Int, Int)) -> () = f4 +} + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo___XFo__iP__ : $@convention(thin) (@owned @callee_owned () -> ()) -> @out Any +// CHECK: init_existential_addr %0 : $*Any, $() +// CHECK-NEXT: apply %1() +// CHECK-NEXT: tuple () +// CHECK-NEXT: return + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo___XFo__iGSqP___ : $@convention(thin) (@owned @callee_owned () -> ()) -> @out Optional +// CHECK: [[ENUM_PAYLOAD:%.*]] = init_enum_data_addr %0 : $*Optional, #Optional.some!enumelt.1 +// CHECK-NEXT: init_existential_addr [[ENUM_PAYLOAD]] : $*Any, $() +// CHECK-NEXT: apply %1() +// CHECK-NEXT: inject_enum_addr %0 : $*Optional, #Optional.some!enumelt.1 +// CHECK-NEXT: tuple () +// CHECK-NEXT: return + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo__dSidSi_XFo__iP__ : $@convention(thin) (@owned @callee_owned () -> (Int, Int)) -> @out Any +// CHECK: [[ANY_PAYLOAD:%.*]] = init_existential_addr %0 +// CHECK-NEXT: [[LEFT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: [[RIGHT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: [[RESULT:%.*]] = apply %1() +// CHECK-NEXT: [[LEFT:%.*]] = tuple_extract [[RESULT]] +// CHECK-NEXT: [[RIGHT:%.*]] = tuple_extract [[RESULT]] +// CHECK-NEXT: store [[LEFT:%.*]] to [trivial] [[LEFT_ADDR]] +// CHECK-NEXT: store [[RIGHT:%.*]] to [trivial] [[RIGHT_ADDR]] +// CHECK-NEXT: tuple () +// CHECK-NEXT: return + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo__dSidSi_XFo__iGSqP___ : $@convention(thin) (@owned @callee_owned () -> (Int, Int)) -> @out Optional { +// CHECK: [[OPTIONAL_PAYLOAD:%.*]] = init_enum_data_addr %0 +// CHECK-NEXT: [[ANY_PAYLOAD:%.*]] = init_existential_addr [[OPTIONAL_PAYLOAD]] +// CHECK-NEXT: [[LEFT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: [[RIGHT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: [[RESULT:%.*]] = apply %1() +// CHECK-NEXT: [[LEFT:%.*]] = tuple_extract [[RESULT]] +// CHECK-NEXT: [[RIGHT:%.*]] = tuple_extract [[RESULT]] +// CHECK-NEXT: store [[LEFT:%.*]] to [trivial] [[LEFT_ADDR]] +// CHECK-NEXT: store [[RIGHT:%.*]] to [trivial] [[RIGHT_ADDR]] +// CHECK-NEXT: inject_enum_addr %0 +// CHECK-NEXT: tuple () +// CHECK-NEXT: return + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo_iP___XFo_dSidSi__ : $@convention(thin) (Int, Int, @owned @callee_owned (@in Any) -> ()) -> () +// CHECK: [[ANY_VALUE:%.*]] = alloc_stack $Any +// CHECK-NEXT: [[ANY_PAYLOAD:%.*]] = init_existential_addr [[ANY_VALUE]] +// CHECK-NEXT: [[LEFT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: store %0 to [trivial] [[LEFT_ADDR]] +// CHECK-NEXT: [[RIGHT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: store %1 to [trivial] [[RIGHT_ADDR]] +// CHECK-NEXT: apply %2([[ANY_VALUE]]) +// CHECK-NEXT: tuple () +// CHECK-NEXT: dealloc_stack [[ANY_VALUE]] +// CHECK-NEXT: return + +// CHECK-LABEL: sil shared [transparent] [reabstraction_thunk] @_TTRXFo_iGSqP____XFo_dSidSi__ : $@convention(thin) (Int, Int, @owned @callee_owned (@in Optional) -> ()) -> () +// CHECK: [[ANY_VALUE:%.*]] = alloc_stack $Any +// CHECK-NEXT: [[ANY_PAYLOAD:%.*]] = init_existential_addr [[ANY_VALUE]] +// CHECK-NEXT: [[LEFT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: store %0 to [trivial] [[LEFT_ADDR]] +// CHECK-NEXT: [[RIGHT_ADDR:%.*]] = tuple_element_addr [[ANY_PAYLOAD]] +// CHECK-NEXT: store %1 to [trivial] [[RIGHT_ADDR]] +// CHECK-NEXT: [[OPTIONAL_VALUE:%.*]] = alloc_stack $Optional +// CHECK-NEXT: [[OPTIONAL_PAYLOAD:%.*]] = init_enum_data_addr [[OPTIONAL_VALUE]] +// CHECK-NEXT: copy_addr [take] [[ANY_VALUE]] to [initialization] [[OPTIONAL_PAYLOAD]] +// CHECK-NEXT: inject_enum_addr [[OPTIONAL_VALUE]] +// CHECK-NEXT: apply %2([[OPTIONAL_VALUE]]) +// CHECK-NEXT: tuple () +// CHECK-NEXT: dealloc_stack [[OPTIONAL_VALUE]] +// CHECK-NEXT: dealloc_stack [[ANY_VALUE]] +// CHECK-NEXT: return diff --git a/validation-test/compiler_crashers_2/0045-sr3267.swift b/validation-test/compiler_crashers_2_fixed/0045-sr3267.swift similarity index 57% rename from validation-test/compiler_crashers_2/0045-sr3267.swift rename to validation-test/compiler_crashers_2_fixed/0045-sr3267.swift index ce1533fef0db1..1d7f40ad06c98 100644 --- a/validation-test/compiler_crashers_2/0045-sr3267.swift +++ b/validation-test/compiler_crashers_2_fixed/0045-sr3267.swift @@ -1,4 +1,4 @@ -// RUN: not --crash %target-swift-frontend %s -parse -emit-silgen +// RUN: %target-swift-frontend %s -parse -emit-silgen // let function: () -> Any = { () -> Void in } diff --git a/validation-test/compiler_crashers/28293-irgensilfunction-visitfullapplysite.swift b/validation-test/compiler_crashers_fixed/28293-irgensilfunction-visitfullapplysite.swift similarity index 89% rename from validation-test/compiler_crashers/28293-irgensilfunction-visitfullapplysite.swift rename to validation-test/compiler_crashers_fixed/28293-irgensilfunction-visitfullapplysite.swift index 6ba84678f451e..4e114d1ecdc4f 100644 --- a/validation-test/compiler_crashers/28293-irgensilfunction-visitfullapplysite.swift +++ b/validation-test/compiler_crashers_fixed/28293-irgensilfunction-visitfullapplysite.swift @@ -5,7 +5,7 @@ // See https://swift.org/LICENSE.txt for license information // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// RUN: not --crash %target-swift-frontend %s -emit-sil +// RUN: %target-swift-frontend %s -emit-sil // Submitted by https://github.com/deville (Andrii Chernenko) From 2517701cd4e91a3449c897c3a1ce89f3e85fec9d Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 23 Dec 2016 09:18:55 -0500 Subject: [PATCH 2/8] SILGen: Emit local types with a separate pass instead of while walking statements Otherwise we might miss emitting a local type that's inside unreachable code. Normally such a type cannot be found via name lookup either, but IRGen will walk the list of local types and try to emit a class with no SIL vtable, which will crash. Fixes . --- lib/SILGen/SILGen.cpp | 3 +++ lib/SILGen/SILGenFunction.h | 5 ++++- lib/SILGen/SILGenType.cpp | 4 ---- test/SILGen/local_types.swift | 10 ++++++++++ test/SILGen/mangling_private.swift | 2 +- test/SILGen/types.swift | 9 +++++---- 6 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 test/SILGen/local_types.swift diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 1b2de18c8393f..646d3aa065dcc 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1408,6 +1408,9 @@ void SILGenModule::emitSourceFile(SourceFile *sf, unsigned startElem) { for (Decl *D : llvm::makeArrayRef(sf->Decls).slice(startElem)) visit(D); + for (Decl *D : sf->LocalTypeDecls) + visit(D); + // Mark any conformances as "used". for (auto conformance : sf->getUsedConformances()) useConformance(ProtocolConformanceRef(conformance)); diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 281d3c86bbb3d..822e221e77ed7 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -1545,7 +1545,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction llvm_unreachable("Not yet implemented"); } - void visitNominalTypeDecl(NominalTypeDecl *D); void visitFuncDecl(FuncDecl *D); void visitPatternBindingDecl(PatternBindingDecl *D); @@ -1554,6 +1553,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction std::unique_ptr emitPatternBindingInitialization(Pattern *P, JumpDest failureDest); + void visitNominalTypeDecl(NominalTypeDecl *D) { + // No lowering support needed. + } + void visitTypeAliasDecl(TypeAliasDecl *D) { // No lowering support needed. } diff --git a/lib/SILGen/SILGenType.cpp b/lib/SILGen/SILGenType.cpp index 4a3b684b47f34..b23f9ee4191fc 100644 --- a/lib/SILGen/SILGenType.cpp +++ b/lib/SILGen/SILGenType.cpp @@ -417,10 +417,6 @@ void SILGenModule::visitNominalTypeDecl(NominalTypeDecl *ntd) { SILGenType(*this, ntd).emitType(); } -void SILGenFunction::visitNominalTypeDecl(NominalTypeDecl *ntd) { - SILGenType(SGM, ntd).emitType(); -} - /// SILGenExtension - an ASTVisitor for generating SIL from method declarations /// and protocol conformances inside type extensions. class SILGenExtension : public TypeMemberVisitor { diff --git a/test/SILGen/local_types.swift b/test/SILGen/local_types.swift new file mode 100644 index 0000000000000..2431cc6b8450f --- /dev/null +++ b/test/SILGen/local_types.swift @@ -0,0 +1,10 @@ +// RUN: %target-swift-frontend -parse-as-library -emit-silgen %s -verify | %FileCheck %s +// RUN: %target-swift-frontend -parse-as-library -emit-ir %s + +func function() { + return + + class UnreachableClass {} // expected-warning {{code after 'return' will never be executed}} +} + +// CHECK-LABEL: sil_vtable UnreachableClass diff --git a/test/SILGen/mangling_private.swift b/test/SILGen/mangling_private.swift index 60db2d397bf0a..fc6f4a9db405a 100644 --- a/test/SILGen/mangling_private.swift +++ b/test/SILGen/mangling_private.swift @@ -40,7 +40,6 @@ private struct PrivateStruct { func localTypes() { struct LocalStruct { - // CHECK-LABEL: sil shared @_TZFVF16mangling_private10localTypesFT_T_L_11LocalStruct13privateMethodfT_T_ private static func privateMethod() {} } } @@ -54,6 +53,7 @@ extension PrivateStruct { private func extPrivateMethod() {} } +// CHECK-LABEL: sil shared @_TZFVF16mangling_private10localTypesFT_T_L_11LocalStruct13privateMethodfT_T_ // CHECK-LABEL: sil_vtable Sub { class Sub : Base { diff --git a/test/SILGen/types.swift b/test/SILGen/types.swift index 9aeeb3239d62b..0859a1854bb9b 100644 --- a/test/SILGen/types.swift +++ b/test/SILGen/types.swift @@ -33,14 +33,13 @@ struct S { } class SC { - // CHECK-LABEL: sil hidden @_TFCV5types1S2SC3bar{{.*}} + // CHECK-LABEL: sil hidden @_TFCV5types1S2SC3bar{{.*}} func bar() {} } } func f() { class FC { - // CHECK-LABEL: sil shared @_TFCF5types1fFT_T_L_2FC3zim{{.*}} func zim() {} } } @@ -48,12 +47,10 @@ func f() { func g(b b : Bool) { if (b) { class FC { - // CHECK-LABEL: sil shared @_TFCF5types1gFT1bSb_T_L_2FC3zim{{.*}} func zim() {} } } else { class FC { - // CHECK-LABEL: sil shared @_TFCF5types1gFT1bSb_T_L0_2FC3zim{{.*}} func zim() {} } } @@ -94,3 +91,7 @@ func referencedFromFunctionEnumFields(_ x: ReferencedFromFunctionEnum) return (nil, g) } } + +// CHECK-LABEL: sil shared @_TFCF5types1fFT_T_L_2FC3zim{{.*}} +// CHECK-LABEL: sil shared @_TFCF5types1gFT1bSb_T_L_2FC3zim{{.*}} +// CHECK-LABEL: sil shared @_TFCF5types1gFT1bSb_T_L0_2FC3zim{{.*}} From 9f8ccd41f3cc1b7ab203199cd2ca08ab40834221 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 30 Dec 2016 22:10:46 -0800 Subject: [PATCH 3/8] SILGen: Fix for tuple conversions in function argument position Swift admits implicit conversions between tuple types to introduce and eliminate argument labels, and re-order argument labels. These are expressed as TupleShuffleExpr in the AST. SILGen has two different code paths for lowering TupleShuffleExpr, which is also used for varargs and default arguments in call argument emission. These two code paths support different subsets of TupleShuffleExpr; neither one supports the full generality of the other, but there is some overlap. Work around the damage by routing the two different "kinds" of TupleShuffleExprs to the correct place in argument emission. The next incremental step here would be to refactor ArgEmitter to make it usable when lowering SubscriptExpr; then the RValueEmitter's support for varargs in TupleShuffleExpr can go away. Once that's done we can split off an ArgumentExpr from TupleShuffleExpr, and in the fullness of time, fold ArgumentExpr into ApplyExpr. At that point this dark corner of the AST will start to be sane... Fixes . --- lib/SILGen/ArgumentSource.cpp | 30 +++++++++++++++++- test/SILGen/argument_shuffle.swift | 50 ++++++++++++++++++++++++++++++ test/SILGen/arguments.swift | 14 ++++----- 3 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 test/SILGen/argument_shuffle.swift diff --git a/lib/SILGen/ArgumentSource.cpp b/lib/SILGen/ArgumentSource.cpp index 954ab625dbeae..3e662adb13351 100644 --- a/lib/SILGen/ArgumentSource.cpp +++ b/lib/SILGen/ArgumentSource.cpp @@ -55,7 +55,35 @@ bool ArgumentSource::requiresCalleeToEvaluate() { case Kind::LValue: return false; case Kind::Expr: - return isa(asKnownExpr()); + // FIXME: TupleShuffleExprs come in two flavors: + // + // 1) as apply arguments, where they're used to insert default + // argument value and collect varargs + // + // 2) as tuple conversions, where they can introduce, eliminate + // and re-order fields + // + // Case 1) must be emitted by ArgEmitter, and Case 2) must be + // emitted by RValueEmitter. + // + // It would be good to split up TupleShuffleExpr into these two + // cases, and simplify ArgEmitter since it no longer has to deal + // with re-ordering. However for now, SubscriptExpr emits the + // index argument via the RValueEmitter, so the RValueEmitter has + // to know about varargs, duplicating some of the logic in + // ArgEmitter. + // + // Once this is fixed, we can also consider allowing subscripts + // to have default arguments. + if (auto *shuffleExpr = dyn_cast(asKnownExpr())) { + for (auto index : shuffleExpr->getElementMapping()) { + if (index == TupleShuffleExpr::DefaultInitialize || + index == TupleShuffleExpr::CallerDefaultInitialize || + index == TupleShuffleExpr::Variadic) + return true; + } + } + return false; } llvm_unreachable("Unhandled Kind in switch."); diff --git a/test/SILGen/argument_shuffle.swift b/test/SILGen/argument_shuffle.swift new file mode 100644 index 0000000000000..aaa419f85500a --- /dev/null +++ b/test/SILGen/argument_shuffle.swift @@ -0,0 +1,50 @@ +// RUN: %target-swift-frontend -emit-silgen %s + +struct Horse { + func walk(_: (String, Int), reverse: Bool) {} + func trot(_: (x: String, y: Int), halfhalt: Bool) {} + func canter(_: T, counter: Bool) {} +} + +var kevin = Horse<(x: String, y: Int)>() +var loki = Horse<(String, Int)>() + +// + +// No conversion +let noLabelsTuple = ("x", 1) +kevin.walk(("x", 1), reverse: false) +kevin.walk(noLabelsTuple, reverse: false) + +loki.canter(("x", 1), counter: false) +loki.canter(noLabelsTuple, counter: false) + +// Introducing labels +kevin.trot(("x", 1), halfhalt: false) +kevin.trot(noLabelsTuple, halfhalt: false) + +kevin.canter(("x", 1), counter: false) +kevin.canter(noLabelsTuple, counter: false) + +// Eliminating labels +let labelsTuple = (x: "x", y: 1) +kevin.walk((x: "x", y: 1), reverse: false) +kevin.walk(labelsTuple, reverse: false) + +loki.canter((x: "x", y: 1), counter: false) +loki.canter(labelsTuple, counter: false) + +// No conversion +kevin.trot((x: "x", y: 1), halfhalt: false) +kevin.trot(labelsTuple, halfhalt: false) + +kevin.canter((x: "x", y: 1), counter: false) +kevin.canter(labelsTuple, counter: false) + +// Shuffling labels +let shuffledLabelsTuple = (y: 1, x: "x") +kevin.trot((y: 1, x: "x"), halfhalt: false) +kevin.trot(shuffledLabelsTuple, halfhalt: false) + +kevin.canter((y: 1, x: "x"), counter: false) +kevin.canter(shuffledLabelsTuple, counter: false) diff --git a/test/SILGen/arguments.swift b/test/SILGen/arguments.swift index c3efdbb554d43..0251c1c7cbafd 100644 --- a/test/SILGen/arguments.swift +++ b/test/SILGen/arguments.swift @@ -18,25 +18,25 @@ func _deallocateUninitializedArray(_: Array) {} var i:Int, f:Float, c:UnicodeScalar -func arg_tuple(x x: Int, y: Float) {} +func arg_tuple(x: Int, y: Float) {} // CHECK-LABEL: sil hidden @_TFs9arg_tupleFT1xSi1ySf_T_ // CHECK: bb0([[X:%[0-9]+]] : $Int, [[Y:%[0-9]+]] : $Float): arg_tuple(x: i, y: f) -func arg_deep_tuples(x x: Int, y: (Float, UnicodeScalar)) {} +func arg_deep_tuples(x: Int, y: (Float, UnicodeScalar)) {} // CHECK-LABEL: sil hidden @_TFs15arg_deep_tuplesFT1xSi1yTSfSc__T_ // CHECK: bb0([[X:%[0-9]+]] : $Int, [[Y_0:%[0-9]+]] : $Float, [[Y_1:%[0-9]+]] : $UnicodeScalar): arg_deep_tuples(x:i, y:(f, c)) var unnamed_subtuple = (f, c) -arg_deep_tuples(x: i, y: unnamed_subtuple) -// FIXME rdar://problem/12985801 -- tuple conversion confuses named fields +arg_deep_tuples(x:i, y: unnamed_subtuple) +// rdar://problem/12985801 -- tuple conversion confuses named fields // of a subtuple with those of outer tuple -//var named_subtuple = (x:f, y:c) -//arg_deep_tuples(i, named_subtuple) +var named_subtuple = (x:f, y:c) +arg_deep_tuples(x:i, y: named_subtuple) -func arg_deep_tuples_2(x x: Int, _: (y: Float, z: UnicodeScalar)) {} +func arg_deep_tuples_2(x: Int, _: (y: Float, z: UnicodeScalar)) {} // CHECK-LABEL: sil hidden @_TFs17arg_deep_tuples_2FT1xSiT1ySf1zSc__T_ // CHECK: bb0([[X:%[0-9]+]] : $Int, [[Y:%[0-9]+]] : $Float, [[Z:%[0-9]+]] : $UnicodeScalar): From ab81426cbba3895084806ce7cee451a4d344285f Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 30 Dec 2016 22:44:38 -0800 Subject: [PATCH 4/8] SIL: Fix SILType substitution bug with DynamicSelfType vs MetatypeType Previously, SIL type lowering would assume a MetatypeType was completely lowered if it had a representation, but this is not quite right; after substitution, we can have a MetatypeType whose instance type is a DynamicSelfType. Strip these away more eagerly, since they show up in SILType::subst(), where we first substitute AST-level generic parameters, and then lower the result to get the final SIL type. Fixes . --- lib/SIL/SILFunctionType.cpp | 22 ++++++++++++++++++++++ lib/SIL/TypeLowering.cpp | 28 ++-------------------------- test/SILGen/dynamic_self.swift | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/lib/SIL/SILFunctionType.cpp b/lib/SIL/SILFunctionType.cpp index 4c08fb2fdcbcf..2f70e077b5e2e 100644 --- a/lib/SIL/SILFunctionType.cpp +++ b/lib/SIL/SILFunctionType.cpp @@ -2209,6 +2209,28 @@ namespace { substObjectType)); } + /// Metatypes get DynamicSelfType stripped off the instance type. + CanType visitMetatypeType(CanMetatypeType origType) { + CanType origInstanceType = origType.getInstanceType(); + CanType substInstanceType = origInstanceType.subst( + Subst, Conformances, None)->getCanonicalType(); + + // If the substitution didn't change anything, we know that the + // original type was a lowered type, so we're good. + if (origInstanceType == substInstanceType) { + return origType; + } + + // If this is a DynamicSelf metatype, turn it into a metatype of the + // underlying self type. + if (auto dynamicSelf = dyn_cast(substInstanceType)) { + substInstanceType = dynamicSelf.getSelfType(); + } + + return CanMetatypeType::get(substInstanceType, + origType->getRepresentation()); + } + /// Any other type is would be a valid type in the AST. Just /// apply the substitution on the AST level and then lower that. CanType visitType(CanType origType) { diff --git a/lib/SIL/TypeLowering.cpp b/lib/SIL/TypeLowering.cpp index 699fe2a4b3bc9..960a2735026b9 100644 --- a/lib/SIL/TypeLowering.cpp +++ b/lib/SIL/TypeLowering.cpp @@ -1235,30 +1235,6 @@ void TypeConverter::insert(TypeKey k, const TypeLowering *tl) { Types[k.getCachingKey()] = tl; } -#ifndef NDEBUG -/// Is this type a lowered type? -static bool isLoweredType(CanType type) { - if (isa(type) || isa(type)) - return false; - if (isa(type)) - return false; - if (auto tuple = dyn_cast(type)) { - for (auto elt : tuple.getElementTypes()) - if (!isLoweredType(elt)) - return false; - return true; - } - OptionalTypeKind optKind; - if (auto objectType = type.getAnyOptionalObjectType(optKind)) { - return (optKind == OTK_Optional && isLoweredType(objectType)); - } - if (auto meta = dyn_cast(type)) { - return meta->hasRepresentation(); - } - return true; -} -#endif - /// Lower each of the elements of the substituted type according to /// the abstraction pattern of the given original type. static CanTupleType getLoweredTupleType(TypeConverter &tc, @@ -1525,7 +1501,7 @@ const TypeLowering &TypeConverter::getTypeLowering(SILType type) { const TypeLowering & TypeConverter::getTypeLoweringForLoweredType(TypeKey key) { auto type = key.SubstType; - assert(isLoweredType(type) && "type is not lowered!"); + assert(type->isLegalSILType() && "type is not lowered!"); (void)type; // Re-using uncurry level 0 is reasonable because our uncurrying @@ -1542,7 +1518,7 @@ TypeConverter::getTypeLoweringForLoweredType(TypeKey key) { const TypeLowering & TypeConverter::getTypeLoweringForUncachedLoweredType(TypeKey key) { assert(!find(key) && "re-entrant or already cached"); - assert(isLoweredType(key.SubstType) && "type is not already lowered"); + assert(key.SubstType->isLegalSILType() && "type is not already lowered"); #ifndef NDEBUG // Catch reentrancy bugs. diff --git a/test/SILGen/dynamic_self.swift b/test/SILGen/dynamic_self.swift index f1dd030e281bf..c063e12b0a3db 100644 --- a/test/SILGen/dynamic_self.swift +++ b/test/SILGen/dynamic_self.swift @@ -207,6 +207,20 @@ class Z { } +// Make sure we erase dynamic Self in a metatype instance type when +// performing SIL type substitution. + +func makeInstance(_ cls: T.Type) -> T { + return cls.init() +} + +class FactoryBase { + required init() { } + func before() -> Self { + return makeInstance(type(of: self)) + } +} + // CHECK-LABEL: sil_witness_table hidden X: P module dynamic_self { // CHECK: method #P.f!1: @_TTWC12dynamic_self1XS_1PS_FS1_1f From 54754c549ba6cfc72602bd24debe6a5798a15bd7 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sat, 31 Dec 2016 17:20:03 -0800 Subject: [PATCH 5/8] SILGen: Fix dynamic calls of 'throwing' methods Fixes . --- lib/SILGen/SILGenApply.cpp | 8 ++++---- test/SILGen/dynamic_lookup_throws.swift | 27 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 test/SILGen/dynamic_lookup_throws.swift diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 7eeab05477d51..fdb4acfdd9d03 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -558,14 +558,14 @@ class Callee { assert(level <= Constant.uncurryLevel && "uncurrying past natural uncurry level of method"); - auto constant = Constant.atUncurryLevel(level); + constant = Constant.atUncurryLevel(level); // Lower the substituted type from the AST, which should have any generic // parameters in the original signature erased to their upper bounds. auto objcFormalType = SubstFormalType.withExtInfo( SubstFormalType->getExtInfo() .withSILRepresentation(SILFunctionTypeRepresentation::ObjCMethod)); auto fnType = gen.SGM.M.Types - .getUncachedSILFunctionTypeForConstant(constant, objcFormalType); + .getUncachedSILFunctionTypeForConstant(*constant, objcFormalType); auto closureType = replaceSelfTypeForDynamicLookup(gen.getASTContext(), fnType, @@ -574,9 +574,9 @@ class Callee { SILValue fn = gen.B.createDynamicMethod(Loc, SelfValue, - constant, + *constant, SILType::getPrimitiveObjectType(closureType), - /*volatile*/ constant.isForeign); + /*volatile*/ Constant.isForeign); mv = ManagedValue::forUnmanaged(fn); break; } diff --git a/test/SILGen/dynamic_lookup_throws.swift b/test/SILGen/dynamic_lookup_throws.swift new file mode 100644 index 0000000000000..eaedd780697f6 --- /dev/null +++ b/test/SILGen/dynamic_lookup_throws.swift @@ -0,0 +1,27 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: %build-clang-importer-objc-overlays + +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk-nosource -I %t) -emit-silgen -parse-as-library %s | %FileCheck %s + +// REQUIRES: objc_interop + +import Foundation + +class Blub : NSObject { + func blub() throws {} +} + +// CHECK-LABEL: sil hidden @_TF21dynamic_lookup_throws8testBlubFzT1aPs9AnyObject__T_ : $@convention(thin) (@owned AnyObject) -> @error Error +func testBlub(a: AnyObject) throws { + // CHECK: open_existential_ref %0 : $AnyObject to $@opened("[[OPENED:.*]]") AnyObject + // CHECK: dynamic_method [volatile] %4 : $@opened("[[OPENED]]") AnyObject, #Blub.blub!1.foreign : (Blub) -> () throws -> (), $@convention(objc_method) (Optional>>, @opened("[[OPENED]]") AnyObject) -> ObjCBool + // CHECK: cond_br {{%.*}}, bb1, bb2 + + // CHECK: bb1 + // CHECK: return + + // CHECK: bb2 + // CHECK: function_ref @swift_convertNSErrorToError + // CHECK: throw {{%.*}} : $Error + try a.blub() +} From 4daf56b648be076833da21e60449e435a56417f0 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sat, 31 Dec 2016 18:20:25 -0800 Subject: [PATCH 6/8] SILGen: Fix calls to literal constructors defined in protocol extensions Fixes . --- lib/SILGen/SILGenApply.cpp | 7 +++-- .../0061-sr3173.swift | 31 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 validation-test/compiler_crashers_2_fixed/0061-sr3173.swift diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index fdb4acfdd9d03..c1614e4861295 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -4694,15 +4694,16 @@ static RValue emitApplyAllocatingInitializer(SILGenFunction &SGF, { // Determine the self metatype type. CanSILFunctionType substFnType = - SGF.getLoweredType(substFormalType, /*uncurryLevel=*/1) - .castTo(); + initConstant.SILFnType->substGenericArgs(SGF.SGM.M, subs); SILType selfParamMetaTy = substFnType->getSelfParameter().getSILType(); if (overriddenSelfType) { // If the 'self' type has been overridden, form a metatype to the // overriding 'Self' type. Type overriddenSelfMetaType = - MetatypeType::get(overriddenSelfType, SGF.getASTContext()); + MetatypeType::get(overriddenSelfType, + selfParamMetaTy.castTo() + ->getRepresentation()); selfMetaTy = SGF.getLoweredType(overriddenSelfMetaType->getCanonicalType()); } else { diff --git a/validation-test/compiler_crashers_2_fixed/0061-sr3173.swift b/validation-test/compiler_crashers_2_fixed/0061-sr3173.swift new file mode 100644 index 0000000000000..4dd17d6e7da94 --- /dev/null +++ b/validation-test/compiler_crashers_2_fixed/0061-sr3173.swift @@ -0,0 +1,31 @@ +// RUN: %target-swift-frontend %s -emit-silgen + +protocol P: Equatable, ExpressibleByStringLiteral { + var uid: String { get } + init(uid: String) +} + +extension P { + // Equatable + public static func ==(lhs: Self, rhs: Self) -> Bool { + return lhs.uid == rhs.uid + } + + // ExpressibleByStringLiteral + public init(stringLiteral value: String) { + self.init(uid: value) + } + public init(unicodeScalarLiteral value: String) { + self.init(uid: value) + } + public init(extendedGraphemeClusterLiteral value: String) { + self.init(uid: value) + } +} + +struct Test: P { + var uid: String + static let s1: Test = "s1" +} + +Test.s1 == "test" From 5f9fe6fa2c464ec65073839eb683045f16af54ef Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sat, 31 Dec 2016 18:44:34 -0800 Subject: [PATCH 7/8] SILGen: Fix crash with non-scalar casts requiring re-abstraction This might only come up in invalid code, but for example casting a function type to String would trigger it. --- lib/SILGen/SILGenDynamicCast.cpp | 18 ++++++++---------- ...ng-CheckedCastEmitter-emitConditional.swift | 6 ------ ...ng-CheckedCastEmitter-emitConditional.swift | 4 ++++ ...t-lowering-emitconditionalcheckedcast.swift | 2 +- 4 files changed, 13 insertions(+), 17 deletions(-) delete mode 100644 validation-test/compiler_crashers_2/0038-lowering-CheckedCastEmitter-emitConditional.swift create mode 100644 validation-test/compiler_crashers_2_fixed/0038-lowering-CheckedCastEmitter-emitConditional.swift rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28420-swift-lowering-emitconditionalcheckedcast.swift (89%) diff --git a/lib/SILGen/SILGenDynamicCast.cpp b/lib/SILGen/SILGenDynamicCast.cpp index 11fbf5dcd0178..9bd7522aa1f46 100644 --- a/lib/SILGen/SILGenDynamicCast.cpp +++ b/lib/SILGen/SILGenDynamicCast.cpp @@ -357,20 +357,18 @@ adjustForConditionalCheckedCastOperand(SILLocation loc, ManagedValue src, } std::unique_ptr init; - SGFContext ctx; if (requiresAddress) { init = SGF.emitTemporary(loc, srcAbstractTL); - + + if (hasAbstraction) + src = SGF.emitSubstToOrigValue(loc, src, abstraction, sourceType); + // Okay, if all we need to do is drop the value in an address, // this is easy. - if (!hasAbstraction) { - SGF.B.emitStoreValueOperation(loc, src.forward(SGF), init->getAddress(), - StoreOwnershipQualifier::Init); - init->finishInitialization(SGF); - return init->getManagedAddress(); - } - - ctx = SGFContext(init.get()); + SGF.B.emitStoreValueOperation(loc, src.forward(SGF), init->getAddress(), + StoreOwnershipQualifier::Init); + init->finishInitialization(SGF); + return init->getManagedAddress(); } assert(hasAbstraction); diff --git a/validation-test/compiler_crashers_2/0038-lowering-CheckedCastEmitter-emitConditional.swift b/validation-test/compiler_crashers_2/0038-lowering-CheckedCastEmitter-emitConditional.swift deleted file mode 100644 index 9b9359aece27e..0000000000000 --- a/validation-test/compiler_crashers_2/0038-lowering-CheckedCastEmitter-emitConditional.swift +++ /dev/null @@ -1,6 +0,0 @@ -// RUN: not --crash %target-swift-frontend %s -emit-silgen - -// REQUIRES: asserts - -let a: () -> Int? = { return nil } -a as? Int diff --git a/validation-test/compiler_crashers_2_fixed/0038-lowering-CheckedCastEmitter-emitConditional.swift b/validation-test/compiler_crashers_2_fixed/0038-lowering-CheckedCastEmitter-emitConditional.swift new file mode 100644 index 0000000000000..051a19752dfb8 --- /dev/null +++ b/validation-test/compiler_crashers_2_fixed/0038-lowering-CheckedCastEmitter-emitConditional.swift @@ -0,0 +1,4 @@ +// RUN: %target-swift-frontend %s -emit-silgen + +let a: () -> Int? = { return nil } +a as? Int diff --git a/validation-test/compiler_crashers/28420-swift-lowering-emitconditionalcheckedcast.swift b/validation-test/compiler_crashers_fixed/28420-swift-lowering-emitconditionalcheckedcast.swift similarity index 89% rename from validation-test/compiler_crashers/28420-swift-lowering-emitconditionalcheckedcast.swift rename to validation-test/compiler_crashers_fixed/28420-swift-lowering-emitconditionalcheckedcast.swift index a47123c516478..037c6fdb20e4f 100644 --- a/validation-test/compiler_crashers/28420-swift-lowering-emitconditionalcheckedcast.swift +++ b/validation-test/compiler_crashers_fixed/28420-swift-lowering-emitconditionalcheckedcast.swift @@ -5,7 +5,7 @@ // See https://swift.org/LICENSE.txt for license information // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// RUN: not --crash %target-swift-frontend %s -emit-ir +// RUN: not %target-swift-frontend %s -emit-ir // REQUIRES: asserts struct A{ func a()->String{ From 3b388df6415b4a0466414febd809ffda9e73c6e9 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sun, 1 Jan 2017 20:10:10 -0800 Subject: [PATCH 8/8] SILGen: Don't try using materializeForSet with storage in an @objc protocol Fixes . --- lib/SILGen/SILGenLValue.cpp | 9 +++++---- test/SILGen/objc_protocols.swift | 9 +++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp index 32ea02c6ac99e..980a3ee9c3167 100644 --- a/lib/SILGen/SILGenLValue.cpp +++ b/lib/SILGen/SILGenLValue.cpp @@ -831,10 +831,11 @@ namespace { return true; } - // If the declaration is dynamically dispatched through a protocol, - // we have to use materializeForSet. - if (isa(decl->getDeclContext())) - return true; + // If the declaration is dynamically dispatched through a + // non-ObjC protocol, we have to use materializeForSet. + if (auto *protoDecl = dyn_cast(decl->getDeclContext())) + if (!protoDecl->isObjC()) + return true; return false; } diff --git a/test/SILGen/objc_protocols.swift b/test/SILGen/objc_protocols.swift index e52ab98e9f4dc..0d8f7c8e5dd1c 100644 --- a/test/SILGen/objc_protocols.swift +++ b/test/SILGen/objc_protocols.swift @@ -280,3 +280,12 @@ extension InitializableConformerByExtension: Initializable { } } // CHECK-LABEL: sil hidden [thunk] @_TToFC14objc_protocols33InitializableConformerByExtensionc + +// Make sure we're crashing from trying to use materializeForSet here. +@objc protocol SelectionItem { + var time: Double { get set } +} + +func incrementTime(contents: SelectionItem) { + contents.time += 1.0 +}