RemoteAST: Clean up use of std::pair and std::tuple for opened existentials with named structs

Author: slavapestov

include/swift/Reflection/ReflectionContext.h

@@ -520,15 +520,14 @@ class ReflectionContext
           readMetadataAndValueOpaqueExistential(ExistentialAddress);
       if (!OptMetaAndValue)
         return false;
-      RemoteAddress MetadataAddress = OptMetaAndValue->first;
-      RemoteAddress ValueAddress = OptMetaAndValue->second;
 
-      auto InstanceTR = readTypeFromMetadata(MetadataAddress.getAddressData());
+      auto InstanceTR = readTypeFromMetadata(
+          OptMetaAndValue->MetadataAddress.getAddressData());
       if (!InstanceTR)
         return false;
 
       *OutInstanceTR = InstanceTR;
-      *OutInstanceAddress = ValueAddress;
+      *OutInstanceAddress = OptMetaAndValue->PayloadAddress;
       return true;
     }
     case RecordKind::ErrorExistential: {
@@ -537,17 +536,15 @@ class ReflectionContext
       if (!OptMetaAndValue)
         return false;
 
-      RemoteAddress InstanceMetadataAddress = std::get<0>(*OptMetaAndValue);
-      RemoteAddress InstanceAddress = std::get<1>(*OptMetaAndValue);
-      // FIXME: Check third value, 'isBridged'
+      // FIXME: Check third value, 'IsBridgedError'
 
-      auto InstanceTR =
-          readTypeFromMetadata(InstanceMetadataAddress.getAddressData());
+      auto InstanceTR = readTypeFromMetadata(
+          OptMetaAndValue->MetadataAddress.getAddressData());
       if (!InstanceTR)
         return false;
 
       *OutInstanceTR = InstanceTR;
-      *OutInstanceAddress = RemoteAddress(InstanceAddress);
+      *OutInstanceAddress = OptMetaAndValue->PayloadAddress;
       return true;
     }
     default:

include/swift/Remote/MetadataReader.h

@@ -86,6 +86,26 @@ struct delete_with_free {
   }
 };
 
+/// A structure representing an opened existential type.
+struct RemoteExistential {
+  /// The payload's concrete type metadata.
+  RemoteAddress MetadataAddress;
+
+  /// The address of the payload value.
+  RemoteAddress PayloadAddress;
+
+  /// True if this is an NSError instance transparently bridged to an Error
+  /// existential.
+  bool IsBridgedError;
+
+  RemoteExistential(RemoteAddress MetadataAddress,
+                    RemoteAddress PayloadAddress,
+                    bool IsBridgedError=false)
+    : MetadataAddress(MetadataAddress),
+      PayloadAddress(PayloadAddress),
+      IsBridgedError(IsBridgedError) {}
+};
+
 /// A generic reader of metadata.
 ///
 /// BuilderType must implement a particular interface which is currently
@@ -330,7 +350,7 @@ class MetadataReader {
   /// pointer to its metadata address, its value address, and whether this
   /// is a toll-free-bridged NSError or an actual Error existential wrapper
   /// around a native Swift value.
-  Optional<std::tuple<RemoteAddress, RemoteAddress, bool>>
+  Optional<RemoteExistential>
   readMetadataAndValueErrorExistential(RemoteAddress ExistentialAddress) {
     // An pointer to an error existential is always an heap object.
     auto MetadataAddress =
@@ -363,10 +383,9 @@ class MetadataReader {
 
     if (isBridged) {
       // NSError instances don't need to be unwrapped.
-      return Optional<std::tuple<RemoteAddress, RemoteAddress, bool>>(
-          {RemoteAddress(*MetadataAddress),
-          ExistentialAddress,
-          isBridged});
+      return RemoteExistential(RemoteAddress(*MetadataAddress),
+                               ExistentialAddress,
+                               isBridged);
     }
 
     // In addition to the isa pointer and two 32-bit reference counts, if the
@@ -400,15 +419,15 @@ class MetadataReader {
     auto Offset = (sizeof(HeapObject) + AlignmentMask) & ~AlignmentMask;
     InstanceAddress += Offset;
 
-    return Optional<std::tuple<RemoteAddress, RemoteAddress, bool>>(
-        {RemoteAddress(*InstanceMetadataAddress),
-         RemoteAddress(InstanceAddress),
-         isBridged});
+    return RemoteExistential(
+        RemoteAddress(*InstanceMetadataAddress),
+        RemoteAddress(InstanceAddress),
+        isBridged);
   }
 
   /// Given a known-opaque existential, attemp to discover the pointer to its
   /// metadata address and its value.
-  Optional<std::pair<RemoteAddress, RemoteAddress>>
+  Optional<RemoteExistential>
   readMetadataAndValueOpaqueExistential(RemoteAddress ExistentialAddress) {
     // OpaqueExistentialContainer is the layout of an opaque existential.
     // `Type` is the pointer to the metadata.
@@ -428,8 +447,8 @@ class MetadataReader {
     // Inline representation (the value fits in the existential container).
     // So, the value starts at the first word of the container.
     if (VWT->isValueInline())
-      return Optional<std::pair<RemoteAddress, RemoteAddress>>(
-          {RemoteAddress(MetadataAddress), ExistentialAddress});
+      return RemoteExistential(RemoteAddress(MetadataAddress),
+                               ExistentialAddress);
 
     // Non-inline (box'ed) representation.
     // The first word of the container stores the address to the box.
@@ -440,8 +459,8 @@ class MetadataReader {
     auto AlignmentMask = VWT->getAlignmentMask();
     auto Offset = (sizeof(HeapObject) + AlignmentMask) & ~AlignmentMask;
     auto StartOfValue = BoxAddress + Offset;
-    return Optional<std::pair<RemoteAddress, RemoteAddress>>(
-        {RemoteAddress(MetadataAddress), RemoteAddress(StartOfValue)});
+    return RemoteExistential(RemoteAddress(MetadataAddress),
+                             RemoteAddress(StartOfValue));
   }
 
   /// Read a protocol from a reference to said protocol.

include/swift/RemoteAST/RemoteAST.h

@@ -23,6 +23,7 @@
 #include "swift/Remote/MemoryReader.h"
 #include "swift/Basic/LLVM.h"
 #include "swift/ABI/MetadataValues.h"
+#include "swift/AST/Type.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -31,7 +32,6 @@
 namespace swift {
 class ASTContext;
 class NominalTypeDecl;
-class Type;
 
 namespace remoteAST {
 
@@ -136,7 +136,20 @@ class Result {
   }
 };
 
-using OpenedExistential = Result<std::pair<Type, remote::RemoteAddress>>;
+/// A structure representing an opened existential value.
+struct OpenedExistential {
+  /// The concrete type of the value inside the existential.
+  Type InstanceType;
+
+  /// The address of the payload.
+  ///
+  /// Note: If the concrete type is a class type, this is the address of the
+  /// instance and not the address of the reference to the instance.
+  remote::RemoteAddress PayloadAddress;
+
+  OpenedExistential(Type InstanceType, remote::RemoteAddress PayloadAddress)
+      : InstanceType(InstanceType), PayloadAddress(PayloadAddress) {}
+};
 
 /// A context for performing an operation relating the remote process with
 /// the AST.  This may be discarded and recreated at any time without danger,
@@ -217,14 +230,14 @@ class RemoteASTContext {
   /// Resolve the dynamic type and the value address of an error existential
   /// object, Unlike getDynamicTypeAndAddressForExistential(), this function
   /// takes the address of the instance and not the address of the reference.
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressForError(remote::RemoteAddress object);
 
   /// Given an existential and its static type, resolve its dynamic
   /// type and address. A single step of unwrapping is performed, i.e. if the
   /// value stored inside the existential is itself an existential, the
   /// caller can decide whether to iterate itself.
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressForExistential(remote::RemoteAddress address,
                                          Type staticType);
 };

lib/RemoteAST/RemoteAST.cpp

@@ -104,9 +104,9 @@ class RemoteASTContextImpl {
   getDeclForRemoteNominalTypeDescriptor(RemoteAddress descriptor) = 0;
   virtual Result<RemoteAddress>
   getHeapMetadataForObject(RemoteAddress object) = 0;
-  virtual OpenedExistential
+  virtual Result<OpenedExistential>
   getDynamicTypeAndAddressForError(RemoteAddress object) = 0;
-  virtual OpenedExistential
+  virtual Result<OpenedExistential>
   getDynamicTypeAndAddressForExistential(RemoteAddress object,
                                          Type staticType) = 0;
 
@@ -478,89 +478,89 @@ class RemoteASTContextConcreteImpl final : public RemoteASTContextImpl {
     return getFailure<RemoteAddress>();
   }
 
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressClassExistential(RemoteAddress object) {
     auto pointerval = Reader.readPointerValue(object.getAddressData());
     if (!pointerval)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
     auto result = Reader.readMetadataFromInstance(*pointerval);
     if (!result)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
     auto typeResult = Reader.readTypeFromMetadata(result.getValue());
     if (!typeResult)
-      return getFailure<std::pair<Type, RemoteAddress>>();
-    return std::make_pair<Type, RemoteAddress>(std::move(typeResult),
-                                               RemoteAddress(*pointerval));
+      return getFailure<OpenedExistential>();
+    return OpenedExistential(std::move(typeResult),
+                             RemoteAddress(*pointerval));
   }
 
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressErrorExistential(RemoteAddress object,
                                            bool dereference=true) {
     if (dereference) {
       auto pointerval = Reader.readPointerValue(object.getAddressData());
       if (!pointerval)
-        return getFailure<std::pair<Type, RemoteAddress>>();
+        return getFailure<OpenedExistential>();
       object = RemoteAddress(*pointerval);
     }
 
     auto result =
         Reader.readMetadataAndValueErrorExistential(object);
     if (!result)
-      return getFailure<std::pair<Type, RemoteAddress>>();
-
-    RemoteAddress metadataAddress = std::get<0>(*result);
-    RemoteAddress valueAddress = std::get<1>(*result);
-    bool isBridged = std::get<2>(*result);
+      return getFailure<OpenedExistential>();
 
     auto typeResult =
-        Reader.readTypeFromMetadata(metadataAddress.getAddressData());
+        Reader.readTypeFromMetadata(result->MetadataAddress.getAddressData());
     if (!typeResult)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
+
 
     // When the existential wraps a class type, LLDB expects that the
     // address returned is the class instance itself and not the address
     // of the reference.
-    if (!isBridged && typeResult->getClassOrBoundGenericClass()) {
-      auto pointerval = Reader.readPointerValue(valueAddress.getAddressData());
+    auto payloadAddress = result->PayloadAddress;
+    if (!result->IsBridgedError &&
+        typeResult->getClassOrBoundGenericClass()) {
+      auto pointerval = Reader.readPointerValue(
+          payloadAddress.getAddressData());
       if (!pointerval)
-        return getFailure<std::pair<Type, RemoteAddress>>();
+        return getFailure<OpenedExistential>();
 
-      valueAddress = RemoteAddress(*pointerval);
+      payloadAddress = RemoteAddress(*pointerval);
     }
 
-    return std::make_pair<Type, RemoteAddress>(std::move(typeResult),
-                                               std::move(valueAddress));
+    return OpenedExistential(std::move(typeResult),
+                             std::move(payloadAddress));
   }
 
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressOpaqueExistential(RemoteAddress object) {
     auto result = Reader.readMetadataAndValueOpaqueExistential(object);
     if (!result)
-      return getFailure<std::pair<Type, RemoteAddress>>();
-    RemoteAddress metadataAddress = result->first;
-    RemoteAddress valueAddress = result->second;
+      return getFailure<OpenedExistential>();
 
     auto typeResult =
-        Reader.readTypeFromMetadata(metadataAddress.getAddressData());
+        Reader.readTypeFromMetadata(result->MetadataAddress.getAddressData());
     if (!typeResult)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
 
     // When the existential wraps a class type, LLDB expects that the
     // address returned is the class instance itself and not the address
     // of the reference.
+    auto payloadAddress = result->PayloadAddress;
     if (typeResult->getClassOrBoundGenericClass()) {
-      auto pointerval = Reader.readPointerValue(valueAddress.getAddressData());
+      auto pointerval = Reader.readPointerValue(
+          payloadAddress.getAddressData());
       if (!pointerval)
-        return getFailure<std::pair<Type, RemoteAddress>>();
+        return getFailure<OpenedExistential>();
 
-      valueAddress = RemoteAddress(*pointerval);
+      payloadAddress = RemoteAddress(*pointerval);
     }
 
-    return std::make_pair<Type, RemoteAddress>(std::move(typeResult),
-                                               std::move(valueAddress));
+    return OpenedExistential(std::move(typeResult),
+                             std::move(payloadAddress));
   }
 
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressExistentialMetatype(RemoteAddress object) {
     // The value of the address is just the input address.
     // The type is obtained through the following sequence of steps:
@@ -569,21 +569,21 @@ class RemoteASTContextConcreteImpl final : public RemoteASTContextImpl {
     // 3) Wrapping the resolved type in an existential metatype.
     auto pointerval = Reader.readPointerValue(object.getAddressData());
     if (!pointerval)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
     auto typeResult = Reader.readTypeFromMetadata(*pointerval);
     if (!typeResult)
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
     auto wrappedType = ExistentialMetatypeType::get(typeResult);
     if (!wrappedType)
-      return getFailure<std::pair<Type, RemoteAddress>>();
-    return std::make_pair<Type, RemoteAddress>(std::move(wrappedType),
-                                               std::move(object));
+      return getFailure<OpenedExistential>();
+    return OpenedExistential(std::move(wrappedType),
+                             std::move(object));
   }
 
   /// Resolve the dynamic type and the value address of an error existential
   /// object, Unlike getDynamicTypeAndAddressForExistential(), this function
   /// takes the address of the instance and not the address of the reference.
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressForError(RemoteAddress object) override {
     return getDynamicTypeAndAddressErrorExistential(object,
                                                     /*dereference=*/false);
@@ -593,12 +593,12 @@ class RemoteASTContextConcreteImpl final : public RemoteASTContextImpl {
   /// given its address and its static type. For class and error existentials,
   /// this API takes a pointer to the instance reference rather than the
   /// instance reference itself.
-  OpenedExistential
+  Result<OpenedExistential>
   getDynamicTypeAndAddressForExistential(RemoteAddress object,
                                          Type staticType) override {
     // If this is not an existential, give up.
     if (!staticType->isAnyExistentialType())
-      return getFailure<std::pair<Type, RemoteAddress>>();
+      return getFailure<OpenedExistential>();
 
     // Handle the case where this is an ExistentialMetatype.
     if (!staticType->isExistentialType())
@@ -674,13 +674,13 @@ RemoteASTContext::getHeapMetadataForObject(remote::RemoteAddress address) {
   return asImpl(Impl)->getHeapMetadataForObject(address);
 }
 
-OpenedExistential
+Result<OpenedExistential>
 RemoteASTContext::getDynamicTypeAndAddressForError(
     remote::RemoteAddress address) {
   return asImpl(Impl)->getDynamicTypeAndAddressForError(address);
 }
 
-OpenedExistential
+Result<OpenedExistential>
 RemoteASTContext::getDynamicTypeAndAddressForExistential(
     remote::RemoteAddress address, Type staticType) {
   return asImpl(Impl)->getDynamicTypeAndAddressForExistential(address,