[lldb][swift] Use dynamic types as a fallback in AddVariableInfo

Since commit eaceb46 AddVariableInfo is
always using the dynamic type if requested. The dynamic types appear
to not always be usable and caused a few tests to fail(1), but as the swiftTest
decorator was always skipping all Swift tests since commit
2c911bc this wasn't discovered before landing.

This patch restores the old behaviour where possible by first trying the
non-dynamic type and falling back to the dynamic type in case the non-dynamic
type can't be fully realized.

(1) All the failures seem to be related to us reading corrupt data from memory
when using the dynamic type in the expression parser. The dynamic types itself
appear to be perfectly fine, so it's not clear why the new behaviour doesn't work.

Author: Teemperor

lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp

@@ -576,6 +576,22 @@ static void AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
   }
 }
 
+/// Returns the Swift type for a ValueObject representing a variable.
+/// An invalid CompilerType is returned on error.
+static CompilerType GetSwiftTypeForVariableValueObject(
+    lldb::ValueObjectSP valobj_sp, lldb::StackFrameSP &stack_frame_sp,
+    SwiftLanguageRuntime *runtime, bool use_dynamic_value) {
+  // Check that the passed ValueObject is valid.
+  if (!valobj_sp || valobj_sp->GetError().Fail())
+    return CompilerType();
+  CompilerType result = valobj_sp->GetCompilerType();
+  if (use_dynamic_value)
+    result = runtime->BindGenericTypeParameters(*stack_frame_sp, result);
+  if (!result.GetTypeSystem()->SupportsLanguage(lldb::eLanguageTypeSwift))
+    return CompilerType();
+  return result;
+}
+
 /// Create a \c VariableInfo record for \c variable if there isn't
 /// already shadowing inner declaration in \c processed_variables.
 static llvm::Optional<llvm::Error> AddVariableInfo(
@@ -602,31 +618,30 @@ static llvm::Optional<llvm::Error> AddVariableInfo(
   if (processed_variables.count(overridden_name))
     return {};
 
-  CompilerType var_type;
-  if (stack_frame_sp) {
-    lldb::ValueObjectSP valobj_sp =
-        stack_frame_sp->GetValueObjectForFrameVariable(variable_sp,
-                                                       use_dynamic);
+  if (!stack_frame_sp)
+    return llvm::None;
 
-    if (!valobj_sp || valobj_sp->GetError().Fail()) {
-      // Ignore the variable if we couldn't find its corresponding
-      // value object.  TODO if the expression tries to use an
-      // ignored variable, produce a sensible error.
-      return {};
-    }
-    var_type = valobj_sp->GetCompilerType();
-    if (use_dynamic > lldb::eNoDynamicValues) {
-      if (auto *stack_frame = stack_frame_sp.get())
-        var_type =
-            runtime->BindGenericTypeParameters(*stack_frame, var_type);
-    }
-  }
+  lldb::ValueObjectSP valobj_sp =
+      stack_frame_sp->GetValueObjectForFrameVariable(variable_sp,
+                                                     lldb::eNoDynamicValues);
+
+  const bool use_dynamic_value = use_dynamic > lldb::eNoDynamicValues;
+
+  CompilerType var_type = GetSwiftTypeForVariableValueObject(
+      valobj_sp, stack_frame_sp, runtime, use_dynamic_value);
 
   if (!var_type.IsValid())
     return {};
 
-  if (!var_type.GetTypeSystem()->SupportsLanguage(lldb::eLanguageTypeSwift))
-    return {};
+  // If the type can't be realized and dynamic types are allowed, fall back to
+  // the dynamic type.
+  if (!SwiftASTContext::IsFullyRealized(var_type) && use_dynamic_value) {
+    var_type = GetSwiftTypeForVariableValueObject(
+        valobj_sp->GetDynamicValue(use_dynamic), stack_frame_sp, runtime,
+        use_dynamic_value);
+    if (!var_type.IsValid())
+      return {};
+  }
 
   Status error;
   CompilerType target_type = ast_context.ImportType(var_type, error);

lldb/test/API/lang/swift/generic_existentials/TestGenericExistentials.py

@@ -1,6 +1,4 @@
 import lldbsuite.test.lldbinline as lldbinline
 from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest,
-                          expectedFailureAll #FIXME: This regressed silently due to 2c911bceb06ed376801251bdfd992905a66f276c
-                          ])
+lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest])

lldb/test/API/lang/swift/opaque_existentials/TestOpaqueExistentials.py

@@ -1,6 +1,4 @@
 import lldbsuite.test.lldbinline as lldbinline
 from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest,
-                          expectedFailureAll #FIXME: This regressed silently due to 2c911bceb06ed376801251bdfd992905a66f276c
-                          ])
+lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest])

lldb/test/API/lang/swift/unknown_self/TestSwiftUnknownSelf.py

@@ -33,7 +33,6 @@ def check_class(self, var_self, broken):
         self.expect("fr v self", substrs=["hello", "world"])
 
 
-    @skipIf #FIXME: This regressed silently due to 2c911bceb06ed376801251bdfd992905a66f276c
     @skipIf(bugnumber="SR-10216", archs=['ppc64le'])
     @swiftTest
     def test_unknown_self_objc_ref(self):

lldb/test/API/lang/swift/variables/error_type/TestSwiftErrorType.py

@@ -12,7 +12,4 @@
 import lldbsuite.test.lldbinline as lldbinline
 from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals(), decorators=[
-#FIXME: This regressed silently due to 2c911bceb06ed376801251bdfd992905a66f276c
-                          expectedFailureAll,
-                          swiftTest])
+lldbinline.MakeInlineTest(__file__, globals(), decorators=[swiftTest])