🎄 Improved tuple key path support in SIL ~ Merry Christmas 🎄

Using an anonymous union in KeyPathPatternComponent instead of the weird void * in SetterAndIdKind
Added TupleElement kind to KeyPathComponentKindEncoding
Written basic SIL keypath serialization tests
Deleted or edited some old Swift-level tuple key path tests

Author: technicated

include/swift/AST/DiagnosticsParse.def

@@ -485,6 +485,8 @@ ERROR(expected_sil_value_ownership_kind,none,
       "expected value ownership kind in SIL code", ())
 ERROR(expected_sil_colon,none,
       "expected ':' before %0", (StringRef))
+ERROR(expected_sil_tuple_index,none,
+      "expected tuple element index", ())
 
 // SIL Values
 ERROR(sil_value_redefinition,none,

include/swift/SIL/SILInstruction.h

@@ -2439,12 +2439,17 @@ class KeyPathPatternComponent {
   // Value is the VarDecl* for StoredProperty, the SILFunction* of the
   // Getter for computed properties, or the Kind for other kinds
   llvm::PointerIntPair<void *, KindPackingBits, unsigned> ValueAndKind;
-  
-  // Setter is the SILFunction* of the Setter for computed properties, or the
-  // tuple index for tuple elements
-  llvm::PointerIntPair<void *, 2,
+  llvm::PointerIntPair<SILFunction *, 2,
                        ComputedPropertyId::KindType> SetterAndIdKind;
-  ComputedPropertyId::ValueType IdValue;
+
+  // If this component refers to a tuple element then TupleIndex is the
+  // 1-based index of the element in the tuple, in order to allow the
+  // discrimination of the TupleElement Kind from the StoredProperty Kind
+  union {
+    unsigned TupleIndex = 0;
+    ComputedPropertyId::ValueType IdValue;
+  };
+
   ArrayRef<Index> Indices;
   struct {
     SILFunction *Equal;
@@ -2492,10 +2497,9 @@ class KeyPathPatternComponent {
   /// Constructor for tuple element.
   KeyPathPatternComponent(unsigned tupleIndex, CanType componentType)
     : ValueAndKind((void*)((uintptr_t)Kind::TupleElement << KindPackingBits), PackedStored),
-    SetterAndIdKind((void*)((uintptr_t)tupleIndex << 2), (ComputedPropertyId::KindType)0),
+    TupleIndex(tupleIndex + 1),
     ComponentType(componentType)
   {
-    // fixme: [technicated] magic << 2 shift
   }
 
 public:
@@ -2509,7 +2513,7 @@ class KeyPathPatternComponent {
     auto packedKind = ValueAndKind.getInt();
     switch ((PackedKind)packedKind) {
     case PackedStored:
-      return SetterAndIdKind.getPointer()
+      return TupleIndex
         ? Kind::TupleElement : Kind::StoredProperty;
     case PackedComputed:
       return SetterAndIdKind.getPointer()
@@ -2580,7 +2584,7 @@ class KeyPathPatternComponent {
     case Kind::TupleElement:
       llvm_unreachable("not a settable computed property");
     case Kind::SettableProperty:
-      return static_cast<SILFunction*>(SetterAndIdKind.getPointer());
+      return SetterAndIdKind.getPointer();
     }
     llvm_unreachable("unhandled kind");
   }
@@ -2676,8 +2680,7 @@ class KeyPathPatternComponent {
     case Kind::SettableProperty:
       llvm_unreachable("not a tuple element");
     case Kind::TupleElement:
-      // fixme: [technicated] magic >> 2 shift
-      return (uintptr_t)SetterAndIdKind.getPointer() >> 2;
+      return TupleIndex - 1;
     }
     llvm_unreachable("unhandled kind");
   }

lib/ParseSIL/ParseSIL.cpp

@@ -2261,6 +2261,19 @@ SILParser::parseKeyPathPatternComponent(KeyPathPatternComponent &component,
 
     component = KeyPathPatternComponent::forOptional(kind, ty);
     return false;
+  } else if (componentKind.str() == "tuple_element") {
+    unsigned tupleIndex;
+    CanType ty;
+
+    if (P.parseToken(tok::pound, diag::expected_sil_constant)
+        || parseInteger(tupleIndex, diag::expected_sil_tuple_index)
+        || P.parseToken(tok::colon, diag::expected_tok_in_sil_instr, ":")
+        || P.parseToken(tok::sil_dollar, diag::expected_tok_in_sil_instr, "$")
+        || parseASTType(ty, patternEnv))
+      return true;
+      
+    component = KeyPathPatternComponent::forTupleElement(tupleIndex, ty);
+    return false;
   } else {
     P.diagnose(componentLoc, diag::sil_keypath_unknown_component_kind,
                componentKind);

lib/Serialization/DeserializeSIL.cpp

@@ -936,6 +936,9 @@ SILDeserializer::readKeyPathComponent(ArrayRef<uint64_t> ListOfValues,
   case KeyPathComponentKindEncoding::OptionalWrap:
     return KeyPathPatternComponent::forOptional(
         KeyPathPatternComponent::Kind::OptionalWrap, type);
+  case KeyPathComponentKindEncoding::TupleElement:
+    return KeyPathPatternComponent::forTupleElement(
+        ListOfValues[nextValue++], type);
   case KeyPathComponentKindEncoding::Trivial:
     llvm_unreachable("handled above");
   }

lib/Serialization/SILFormat.h

@@ -81,6 +81,7 @@ enum CastConsumptionKindEncoding : uint8_t {
 
 enum class KeyPathComponentKindEncoding : uint8_t {
   StoredProperty,
+  TupleElement,
   GettableProperty,
   SettableProperty,
   OptionalChain,

lib/Serialization/SerializeSIL.cpp

@@ -685,7 +685,8 @@ SILSerializer::writeKeyPathPatternComponent(
     handleComponentCommon(KeyPathComponentKindEncoding::OptionalWrap);
     break;
   case KeyPathPatternComponent::Kind::TupleElement:
-    llvm_unreachable("[technicated]");
+    handleComponentCommon(KeyPathComponentKindEncoding::TupleElement);
+    ListOfValues.push_back((unsigned)component.getTupleIndex());
     break;
   }
 }

test/SIL/Serialization/keypath.sil

@@ -49,6 +49,13 @@ public struct External<T> {
   init()
 }
 
+struct T {
+  var x: (Int, Int)
+  var y: (a: Int, b: Int, c: Int)
+  let z: (Int, Int)
+  let w: (Int, C)
+}
+
 // CHECK-LABEL: sil shared [serialized] [ossa] @stored_properties
 sil shared [serialized] [ossa] @stored_properties : $@convention(thin) () -> () {
 entry:
@@ -75,6 +82,17 @@ entry:
   return undef : $()
 }
 
+// CHECK-LABEL: sil shared [serialized] [ossa] @tuple_elements
+sil shared [serialized] [ossa] @tuple_elements : $@convention(thin) () -> () {
+entry:
+  // CHECK: keypath $WritableKeyPath<T, Int>, (root $T; stored_property #T.x : $(Int, Int); tuple_element #0 : $Int)
+  %a = keypath $WritableKeyPath<T, Int>, (root $T; stored_property #T.x : $(Int, Int); tuple_element #0 : $Int)
+  // CHECK: keypath $WritableKeyPath<T, Int>, (root $T; stored_property #T.y : $(a: Int, b: Int, c: Int); tuple_element #2 : $Int)
+  %b = keypath $WritableKeyPath<T, Int>, (root $T; stored_property #T.y : $(a: Int, b: Int, c: Int); tuple_element #2 : $Int)
+
+  return undef : $()
+}
+
 sil @id_a : $@convention(thin) () -> ()
 sil @get_s_int : $@convention(thin) (@in_guaranteed S) -> @out Int
 sil @set_s_int : $@convention(thin) (@in_guaranteed Int, @in_guaranteed S) -> ()
@@ -160,12 +178,13 @@ entry:
 sil [serialized] [ossa] @serialize_all : $@convention(thin) () -> () {
 entry:
   %0 = function_ref @stored_properties : $@convention(thin) () -> ()
-  %1 = function_ref @stored_properties_generic : $@convention(thin) <D: P, E: Q, F: R> () -> ()
-  %2 = function_ref @computed_properties : $@convention(thin) () -> ()
-  %3 = function_ref @computed_properties_generic : $@convention(thin) <D: P, E: Q, F: R> () -> ()
-  %4 = function_ref @optional : $@convention(thin) () -> ()
-  %5 = function_ref @indexes : $@convention(thin) (S, C) -> ()
-  %6 = function_ref @external : $@convention(thin) <D: P, E: Q, F: R> () -> ()
+  %1 = function_ref @tuple_elements : $@convention(thin) () -> ()
+  %2 = function_ref @stored_properties_generic : $@convention(thin) <D: P, E: Q, F: R> () -> ()
+  %3 = function_ref @computed_properties : $@convention(thin) () -> ()
+  %4 = function_ref @computed_properties_generic : $@convention(thin) <D: P, E: Q, F: R> () -> ()
+  %5 = function_ref @optional : $@convention(thin) () -> ()
+  %6 = function_ref @indexes : $@convention(thin) (S, C) -> ()
+  %7 = function_ref @external : $@convention(thin) <D: P, E: Q, F: R> () -> ()
 
   unreachable
 }

test/expr/unary/keypath/keypath-unimplemented.swift

@@ -220,8 +220,6 @@ func testKeyPathInGenericContext<H: Hashable, X>(hashable: H, anything: X) {
 func testDisembodiedStringInterpolation(x: Int) {
   \(x) // expected-error{{string interpolation}} expected-error{{}}
   \(x, radix: 16) // expected-error{{string interpolation}} expected-error{{}}
-
-  _ = \(Int, Int).0 // expected-error{{cannot reference tuple elements}}
 }
 
 func testNoComponents() {
@@ -235,20 +233,19 @@ struct TupleStruct {
 }
 
 func tupleComponent() {
-  // TODO: Customized diagnostic
-  let _ = \(Int, String).0 // expected-error{{}}
-  let _ = \(Int, String).1 // expected-error{{}}
-  let _ = \TupleStruct.unlabeled.0 // expected-error{{}}
-  let _ = \TupleStruct.unlabeled.1 // expected-error{{}}
-
-  let _ = \(foo: Int, bar: String).0 // expected-error{{}}
-  let _ = \(foo: Int, bar: String).1 // expected-error{{}}
-  let _ = \(foo: Int, bar: String).foo // expected-error{{}}
-  let _ = \(foo: Int, bar: String).bar // expected-error{{}}
-  let _ = \TupleStruct.labeled.0 // expected-error{{}}
-  let _ = \TupleStruct.labeled.1 // expected-error{{}}
-  let _ = \TupleStruct.labeled.foo // expected-error{{}}
-  let _ = \TupleStruct.labeled.bar // expected-error{{}}
+  let _ = \(Int, String).0
+  let _ = \(Int, String).1
+  let _ = \TupleStruct.unlabeled.0
+  let _ = \TupleStruct.unlabeled.1
+
+  let _ = \(foo: Int, bar: String).0
+  let _ = \(foo: Int, bar: String).1
+  let _ = \(foo: Int, bar: String).foo
+  let _ = \(foo: Int, bar: String).bar
+  let _ = \TupleStruct.labeled.0
+  let _ = \TupleStruct.labeled.1
+  let _ = \TupleStruct.labeled.foo
+  let _ = \TupleStruct.labeled.bar
 }
 
 struct Z { }