Eliminate unnecessary SwiftASTContext fallback in GetIndexOfChildMemberWithName().

Enums without payloads are not supposed to have children in the LLDB
data formatter sense, so GetIndexOfChildMemberWithName should return
0. However previously we couldn't distinguish between
SwiftLanguageRuntime failing to look up a type and a success where the
match was a non-payload enum member.

Author: adrian-prantl

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -299,7 +299,7 @@ class SwiftLanguageRuntimeStub {
     return {};
   }
 
-  llvm::Optional<size_t> GetIndexOfChildMemberWithName(
+  std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
       CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
       bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
     STUB_LOG();
@@ -2209,7 +2209,8 @@ llvm::Optional<std::string> SwiftLanguageRuntime::GetEnumCaseName(
   FORWARD(GetEnumCaseName, type, data, exe_ctx);
 }
 
-llvm::Optional<size_t> SwiftLanguageRuntime::GetIndexOfChildMemberWithName(
+std::pair<bool, llvm::Optional<size_t>>
+SwiftLanguageRuntime::GetIndexOfChildMemberWithName(
     CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
     bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
   FORWARD(GetIndexOfChildMemberWithName, type, name, exe_ctx,

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h

@@ -280,7 +280,17 @@ class SwiftLanguageRuntime : public LanguageRuntime {
                                               const DataExtractor &data,
                                               ExecutionContext *exe_ctx);
 
-  llvm::Optional<size_t> GetIndexOfChildMemberWithName(
+  /// Behaves like the CompilerType::GetIndexOfChildMemberWithName()
+  /// except for the more nuanced return value.
+  ///
+  /// \returns {false, {}} on error.
+  //
+  /// \returns {true, {}} if the member exists, but it is an enum case
+  ///                     without payload. Enum cases without payload
+  ///                     don't have an index.
+  ///
+  /// \returns {true, {num_idexes}} on success.
+  std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
       CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
       bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes);
 

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp

@@ -1073,26 +1073,32 @@ GetTypeFromTypeRef(TypeSystemSwiftTypeRef &ts,
   return ts.RemangleAsType(dem, node);
 }
 
-static llvm::Optional<size_t>
+static std::pair<bool, llvm::Optional<size_t>>
 findFieldWithName(const std::vector<swift::reflection::FieldInfo> &fields,
-                  llvm::StringRef name, std::vector<uint32_t> &child_indexes,
-                  uint32_t offset = 0) {
+                  llvm::StringRef name, bool is_enum,
+                  std::vector<uint32_t> &child_indexes, uint32_t offset = 0) {
   uint32_t index = 0;
+  bool is_nonpayload_enum_case = false;
   auto it = std::find_if(fields.begin(), fields.end(), [&](const auto &field) {
-    // A nonnull TypeRef is required for enum cases, where it represents cases
-    // that have a payload. In other types it will be true anyway.
-    if (field.TR == nullptr)
-      return false;
     if (name != field.Name) {
-      ++index;
+      // A nonnull TypeRef is required for enum cases, where it represents cases
+      // that have a payload. In other types it will be true anyway.
+      if (field.TR)
+        ++index;
       return false;
     }
+    if (is_enum)
+      is_nonpayload_enum_case = (field.TR == nullptr);
     return true;
   });
+  // Not found.
   if (it == fields.end())
-    return {};
+    return {false, {}};
+  // Found, but no index to report.
+  if (is_nonpayload_enum_case)
+    return {true, {}};
   child_indexes.push_back(offset + index);
-  return child_indexes.size();
+  return {true, child_indexes.size()};
 }
 
 static llvm::Optional<std::string>
@@ -1136,21 +1142,22 @@ llvm::Optional<std::string> SwiftLanguageRuntimeImpl::GetEnumCaseName(
   return {};
 }
 
-llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
+std::pair<bool, llvm::Optional<size_t>>
+SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
     CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
     bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) {
   LLDB_SCOPED_TIMER();
   auto *ts =
       llvm::dyn_cast_or_null<TypeSystemSwiftTypeRef>(type.GetTypeSystem());
   if (!ts)
-    return {};
+    return {false, {}};
 
   using namespace swift::reflection;
   // Try the static type metadata.
   const TypeRef *tr = nullptr;
   auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr(), &tr);
   if (!ti)
-    return {};
+    return {false, {}};
   switch (ti->getKind()) {
   case TypeInfoKind::Record: {
     // Structs and Tuples.
@@ -1160,7 +1167,7 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
     case RecordKind::ThickFunction:
       // There are two fields, `function` and `context`, but they're not exposed
       // by lldb.
-      return 0;
+      return {true, {0}};
     case RecordKind::OpaqueExistential:
       // `OpaqueExistential` is documented as:
       //     An existential is a three-word buffer followed by value metadata...
@@ -1170,17 +1177,17 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
         uint32_t index;
         if (name.take_back().getAsInteger(10, index) && index < 3) {
           child_indexes.push_back(index);
-          return child_indexes.size();
+          return {true, child_indexes.size()};
         }
       }
-      return findFieldWithName(rti->getFields(), name, child_indexes, 3);
+      return findFieldWithName(rti->getFields(), name, false, child_indexes, 3);
     default:
-      return findFieldWithName(rti->getFields(), name, child_indexes);
+      return findFieldWithName(rti->getFields(), name, false, child_indexes);
     }
   }
   case TypeInfoKind::Enum: {
     auto *eti = llvm::cast<EnumTypeInfo>(ti);
-    return findFieldWithName(eti->getCases(), name, child_indexes);
+    return findFieldWithName(eti->getCases(), name, true, child_indexes);
   }
   case TypeInfoKind::Reference: {
     // Objects.
@@ -1207,20 +1214,21 @@ llvm::Optional<size_t> SwiftLanguageRuntimeImpl::GetIndexOfChildMemberWithName(
           break;
         auto *super_tr = builder.lookupSuperclass(current_tr);
         uint32_t offset = super_tr ? 1 : 0;
-        if (auto size = findFieldWithName(record_ti->getFields(), name,
-                                          child_indexes, offset))
-          return size;
+        auto found_size = findFieldWithName(record_ti->getFields(), name, false,
+                                            child_indexes, offset);
+        if (found_size.first)
+          return found_size;
         current_tr = super_tr;
         child_indexes.push_back(0);
       }
       child_indexes.clear();
-      return {};
+      return {false, {}};
     }
     }
   }
   default:
     // FIXME: Implement more cases.
-    return {};
+    return {false, {}};
   }
 }
 

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeImpl.h

@@ -131,7 +131,7 @@ class SwiftLanguageRuntimeImpl {
                                               const DataExtractor &data,
                                               ExecutionContext *exe_ctx);
 
-  llvm::Optional<size_t> GetIndexOfChildMemberWithName(
+  std::pair<bool, llvm::Optional<size_t>> GetIndexOfChildMemberWithName(
       CompilerType type, llvm::StringRef name, ExecutionContext *exe_ctx,
       bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes);
 

lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp

@@ -2802,27 +2802,29 @@ size_t TypeSystemSwiftTypeRef::GetIndexOfChildMemberWithName(
             child_indexes));
   if (auto *exe_scope = exe_ctx->GetBestExecutionContextScope())
     if (auto *runtime =
-            SwiftLanguageRuntime::Get(exe_scope->CalculateProcess()))
-      if (auto index_size = runtime->GetIndexOfChildMemberWithName(
-              GetCanonicalType(type), name, exe_ctx, omit_empty_base_classes,
-              child_indexes)) {
+            SwiftLanguageRuntime::Get(exe_scope->CalculateProcess())) {
+      auto found_numidx = runtime->GetIndexOfChildMemberWithName(
+          GetCanonicalType(type), name, exe_ctx, omit_empty_base_classes,
+          child_indexes);
+      if (found_numidx.first) {
+        size_t index_size = found_numidx.second.getValueOr(0);
 #ifndef NDEBUG
         // This block is a custom VALIDATE_AND_RETURN implementation to support
         // checking the return value, plus the by-ref `child_indexes`.
         if (!GetSwiftASTContext())
-          return *index_size;
+          return index_size;
         auto ast_type = ReconstructType(type);
         if (!ast_type)
-          return *index_size;
+          return index_size;
         std::vector<uint32_t> ast_child_indexes;
         auto ast_index_size =
             GetSwiftASTContext()->GetIndexOfChildMemberWithName(
                 ast_type, name, exe_ctx, omit_empty_base_classes,
                 ast_child_indexes);
         // The runtime has more info than the AST. No useful validation can be
         // done.
-        if (*index_size > ast_index_size)
-          return *index_size;
+        if (index_size > ast_index_size)
+          return index_size;
 
         auto fail = [&]() {
           auto join = [](const auto &v) {
@@ -2841,17 +2843,20 @@ size_t TypeSystemSwiftTypeRef::GetIndexOfChildMemberWithName(
           assert(false &&
                  "TypeSystemSwiftTypeRef diverges from SwiftASTContext");
         };
-        if (*index_size != ast_index_size)
+        if (index_size != ast_index_size)
           fail();
-        for (unsigned i = 0; i < *index_size; ++i)
+        for (unsigned i = 0; i < index_size; ++i)
           if (child_indexes[i] < ast_child_indexes[i])
             // When the runtime may know know about more children. When this
             // happens, indexes will be larger. But if an index is smaller, that
             // means the runtime has dropped info somehow.
             fail();
 #endif
-        return *index_size;
+        return index_size;
       }
+      // If we're here, the runtime didn't find type info.
+      assert(!found_numidx.first);
+    }
 
   LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES),
             "Using SwiftASTContext::GetIndexOfChildMemberWithName fallback for "

lldb/test/Shell/Swift/Inputs/No.swiftmodule.swift

@@ -1,22 +1,36 @@
 import NoSwiftmoduleHelper
 
 // The struct is resolved using type metadata and the Swift runtime.
-struct s { let i = 0 }
+struct S { let i = 0 }
 
 func useTypeFromOtherModule(x: S2) {
   // break here
 }
 
+enum NoPayload {
+  case first
+  case second
+}
+
+enum WithPayload {
+  case empty
+  case with(i: Int)
+}
+
 func f<T>(_ t: T) {
-  let number = 1                   // CHECK-DAG: (Int) number = 1
-  let array = [1, 2, 3]            // CHECK-DAG: ([Int]) array = 3 values
-  let string = "hello"             // CHECK-DAG: (String) string = "hello"
-  let tuple = (0, 1)               // CHECK-DAG: (Int, Int) tuple = (0 = 0, 1 = 1)
-  let strct = s()                  // CHECK-DAG: strct = (i = 0)
-  let strct2 = S2()                // CHECK-DAG: strct2 = {}{{$}}
-  let generic = t                  // CHECK-DAG: (Int) generic = 23
-  let generic_tuple = (t, t)       // CHECK-DAG: generic_tuple = (0 = 23, 1 = 23)
-  let word = 0._builtinWordValue   // CHECK-DAG: word = 0
+  let number = 1                     // CHECK-DAG: (Int) number {{=}} 1
+  let array = [1, 2, 3]              // CHECK-DAG: ([Int]) array {{=}} 3 values
+  let string = "hello"               // CHECK-DAG: (String) string {{=}} "hello"
+  let tuple = (0, 1)                 // CHECK-DAG: (Int, Int) tuple {{=}} (0 = 0, 1 = 1)
+  let strct = S()                    // CHECK-DAG: strct {{=}} (i = 0)
+  let strct2 = S2()                  // CHECK-DAG: strct2 {{=}} {}{{$}}
+  let generic = t                    // CHECK-DAG: (Int) generic {{=}} 23
+  let generic_tuple = (t, t)         // CHECK-DAG: generic_tuple {{=}} (0 = 23, 1 = 23)
+  let word = 0._builtinWordValue     // CHECK-DAG: word {{=}} 0
+  let enum1 = NoPayload.second       // CHECK-DAG: enum1 {{=}}
+                                     // FIXME: Fails in swift::reflection::NoPayloadEnumTypeInfo::projectEnumValue: .second
+  let enum2 = WithPayload.with(i:42) // CHECK-DAG: enum2 {{=}} with
+                                     // CHECK-DAG: i {{=}} 42
   print(number)
   useTypeFromOtherModule(x: S2())
 }

lldb/test/Shell/Swift/No.swiftmodule.test

@@ -27,3 +27,4 @@ run
 fr var
 up
 fr var
+quit