Merge pull request #3889 from adrian-prantl/non-payload-enums

Eliminate unnecessary SwiftASTContext fallback in GetIndexOfChildMemb…

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