Merge pull request #9909 from adrian-prantl/143358292

[lldb] Fix Swift.Optional formatter behavior when types cannot be res

Author: adrian-prantl

lldb/source/Plugins/Language/Swift/SwiftOptional.cpp

@@ -38,38 +38,45 @@ std::string lldb_private::formatters::swift::SwiftOptionalSummaryProvider::
   return sstr.GetString().str();
 }
 
-// if this ValueObject is an Optional<T> with the Some(T) case selected,
-// retrieve the value of the Some case..
-static PointerOrSP
+/// If this ValueObject is an Optional<T> with the Some(T) case selected,
+/// retrieve the value of the Some case.
+///
+/// Returns {} on error, nullptr on .none, and a ValueObject on .some.
+/// None of the callees can pass on errors messages, so this function
+/// doesn't return them either.
+static std::optional<ValueObjectSP>
 ExtractSomeIfAny(ValueObject *optional,
                  bool synthetic_value = false) {
   if (!optional)
-    return nullptr;
+    return {};
 
   static ConstString g_Some("some");
   static ConstString g_None("none");
 
   ValueObjectSP non_synth_valobj = optional->GetNonSyntheticValue();
   if (!non_synth_valobj)
-    return nullptr;
+    return {};
 
   ConstString value(non_synth_valobj->GetValueAsCString());
 
-  if (!value || value == g_None)
+  if (!value)
+    return {};
+
+  if (value == g_None)
     return nullptr;
 
-  PointerOrSP value_sp(
-      non_synth_valobj->GetChildMemberWithName(g_Some, true).get());
+  ValueObjectSP value_sp(
+      non_synth_valobj->GetChildMemberWithName(g_Some, true));
   if (!value_sp)
-    return nullptr;
+    return {};
 
   auto process_sp = optional->GetProcessSP();
   auto *swift_runtime = SwiftLanguageRuntime::Get(process_sp);
 
   CompilerType type = non_synth_valobj->GetCompilerType();
   auto type_system = type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>();
   if (!type_system)
-    return nullptr;
+    return {};
   if (auto kind = type_system->GetNonTriviallyManagedReferenceKind(
           type.GetOpaqueQualType())) {
     if (*kind == TypeSystemSwift::NonTriviallyManagedReferenceKind::eWeak) {
@@ -86,10 +93,10 @@ ExtractSomeIfAny(ValueObject *optional,
           DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
                                   process_sp->GetAddressByteSize());
           ExecutionContext exe_ctx(process_sp);
-          value_sp = PointerOrSP(ValueObject::CreateValueObjectFromData(
-              value_sp->GetName().AsCString(), extractor, exe_ctx, value_type));
+          value_sp = ValueObject::CreateValueObjectFromData(
+              value_sp->GetName().AsCString(), extractor, exe_ctx, value_type);
           if (!value_sp)
-            return nullptr;
+            return {};
           else
             value_sp->SetSyntheticChildrenGenerated(true);
         }
@@ -116,12 +123,16 @@ ExtractSomeIfAny(ValueObject *optional,
     value_sp = value_sp->GetSyntheticValue();
 
   return value_sp;
-  }
+}
 
 static bool
 SwiftOptional_SummaryProvider_Impl(ValueObject &valobj, Stream &stream,
                                    const TypeSummaryOptions &options) {
-  PointerOrSP some = ExtractSomeIfAny(&valobj, true);
+  std::optional<ValueObjectSP> maybe_some = ExtractSomeIfAny(&valobj, true);
+  if (!maybe_some)
+    return false;
+
+  ValueObjectSP some = *maybe_some;
   if (!some) {
     stream.Printf("nil");
     return true;
@@ -145,7 +156,7 @@ SwiftOptional_SummaryProvider_Impl(ValueObject &valobj, Stream &stream,
         .SetSkipReferences(false);
     StringSummaryFormat oneliner(oneliner_flags, "");
     std::string buffer;
-    oneliner.FormatObject(some, buffer, options);
+    oneliner.FormatObject(some.get(), buffer, options);
     stream.Printf("%s", buffer.c_str());
   }
 
@@ -172,8 +183,12 @@ bool lldb_private::formatters::swift::SwiftOptionalSummaryProvider::
   if (!target_valobj)
     return false;
 
-  PointerOrSP some = ExtractSomeIfAny(target_valobj, true);
+  std::optional<ValueObjectSP> maybe_some =
+      ExtractSomeIfAny(target_valobj, true);
+  if (!maybe_some)
+    return false;
 
+  ValueObjectSP some = *maybe_some;
   if (!some)
     return true;
 
@@ -191,7 +206,7 @@ bool lldb_private::formatters::swift::SwiftOptionalSummaryProvider::
       return false;
     return some->HasChildren();
   }
-  return some->HasChildren() && summary_sp->DoesPrintChildren(some);
+  return some->HasChildren() && summary_sp->DoesPrintChildren(some.get());
 }
 
 bool lldb_private::formatters::swift::SwiftOptionalSummaryProvider::
@@ -231,7 +246,12 @@ lldb::ChildCacheState lldb_private::formatters::swift::SwiftOptionalSyntheticFro
   m_is_none = true;
   m_children = false;
 
-  m_some = ExtractSomeIfAny(&m_backend, true);
+  std::optional<ValueObjectSP> maybe_some =
+      ExtractSomeIfAny(&m_backend, true);
+  if (!maybe_some)
+    return ChildCacheState::eRefetch;
+
+  m_some = *maybe_some;
 
   if (!m_some) {
     m_is_none = true;

lldb/source/Plugins/Language/Swift/SwiftOptional.h

@@ -22,42 +22,6 @@
 
 namespace lldb_private {
 namespace formatters {
-// ExtractSomeIfAny() can return EITHER a child member or some other long-lived
-// ValueObject
-// OR an entirely consed-up ValueObject
-// The lifetime of these two is radically different, and there is no trivial way
-// to do the right
-// thing for both cases - except have a class that can wrap either and is safe
-// to store and pass around
-class PointerOrSP {
-public:
-  PointerOrSP(std::nullptr_t) : m_raw_ptr(nullptr), m_shared_ptr(nullptr) {}
-
-  PointerOrSP(ValueObject *valobj) : m_raw_ptr(valobj), m_shared_ptr(nullptr) {}
-
-  PointerOrSP(lldb::ValueObjectSP valobj_sp)
-      : m_raw_ptr(nullptr), m_shared_ptr(valobj_sp) {}
-
-  ValueObject *operator->() {
-    if (m_shared_ptr)
-      return m_shared_ptr.get();
-    return m_raw_ptr;
-  }
-
-  ValueObject &operator*() { return *(this->operator->()); }
-
-  operator ValueObject *() { return this->operator->(); }
-
-  explicit operator bool() const {
-    return (m_shared_ptr.get() != nullptr) || (m_raw_ptr != nullptr);
-  }
-
-  bool operator==(std::nullptr_t) const { return !(this->operator bool()); }
-
-protected:
-  ValueObject *m_raw_ptr;
-  lldb::ValueObjectSP m_shared_ptr;
-};
 
 namespace swift {
 struct SwiftOptionalSummaryProvider : public TypeSummaryImpl {
@@ -92,7 +56,7 @@ class SwiftOptionalSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 private:
   bool m_is_none;
   bool m_children;
-  PointerOrSP m_some;
+  lldb::ValueObjectSP m_some;
 
   bool IsEmpty() const;
 };

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

@@ -866,6 +866,10 @@ SwiftLanguageRuntime::GetNumFields(CompilerType type,
       return rti->getNumFields();
     }
   }
+  case TypeInfoKind::Builtin: {
+    // Clang types without debug info may present themselves like this.
+    return {};
+  }
   case TypeInfoKind::Enum: {
     auto *eti = llvm::cast<EnumTypeInfo>(ti);
     return eti->getNumPayloadCases();

lldb/test/API/lang/swift/optional_error_handling/Library.swift

@@ -0,0 +1,5 @@
+public struct WithOpaqueType {
+  public init() {}
+  let opaqueSome : FromC? = FromC(i: 23)
+  let opaqueNone : FromC? = nil
+}

lldb/test/API/lang/swift/optional_error_handling/Makefile

@@ -0,0 +1,22 @@
+SWIFT_SOURCES := main.swift
+SWIFTFLAGS_EXTRAS = -I.
+LD_EXTRAS = -L. -lLibrary
+
+
+all: Library $(EXE)
+
+include Makefile.rules
+
+.PHONY: Library
+Library:
+	$(MAKE) MAKE_DSYM=NO CC=$(CC) SWIFTC=$(SWIFTC) \
+		ARCH=$(ARCH) DSYMUTIL=$(DSYMUTIL) \
+		VPATH=$(SRCDIR) -I $(SRCDIR) SRCDIR=$(SRCDIR) \
+		-f $(THIS_FILE_DIR)/Makefile.rules \
+		DYLIB_SWIFT_SOURCES=Library.swift \
+		SWIFT_BRIDGING_HEADER=bridging.h \
+		SWIFT_PRECOMPILE_BRIDGING_HEADER=NO \
+		DYLIB_NAME=Library \
+		DYLIB_ONLY=YES \
+		DEBUG_INFO_FLAG= \
+		all

lldb/test/API/lang/swift/optional_error_handling/TestSwiftOptionalErrorHandling.py

@@ -0,0 +1,20 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestSwiftOptionalErrorHandling(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @swiftTest
+    def test(self):
+        """Test that errors are surfaced"""
+        self.build()
+        target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.swift"),
+            extra_images=['Library'])
+        self.expect('settings set symbols.use-swift-clangimporter false')
+        self.expect('frame variable x', substrs=[
+            'opaqueSome', 'missing debug info for Clang type', 'FromC',
+            'opaqueNone', 'nil',
+        ])

lldb/test/API/lang/swift/optional_error_handling/bridging.h

@@ -0,0 +1,3 @@
+struct FromC {
+  int i;
+};

lldb/test/API/lang/swift/optional_error_handling/main.swift

@@ -0,0 +1,6 @@
+import Library
+func main() {
+  let x = WithOpaqueType()
+  print(x) // break here
+}
+main()