Move ownership of SwiftASTContext to SwiftUserExpression (NFC)

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

@@ -58,7 +58,9 @@ class SwiftExpressionParser : public ExpressionParser {
   /// @param[in] options
   ///     Additional options for the parser.
   //------------------------------------------------------------------
-  SwiftExpressionParser(ExecutionContextScope *exe_scope, Expression &expr,
+  SwiftExpressionParser(ExecutionContextScope *exe_scope,
+                        SwiftASTContextForExpressions &swift_ast_ctx,
+                        Expression &expr,
                         const EvaluateExpressionOptions &options);
 
   //------------------------------------------------------------------
@@ -162,7 +164,7 @@ class SwiftExpressionParser : public ExpressionParser {
   /// The container for the IR, to be JIT-compiled or interpreted.
   lldb::IRExecutionUnitSP m_execution_unit_sp;
   /// The AST context to build the expression into.
-  std::unique_ptr<SwiftScratchContextReader> m_swift_ast_context;
+  SwiftASTContextForExpressions &m_swift_ast_ctx;
   /// Used to manage the memory of a potential on-off context.
   //lldb::TypeSystemSP m_typesystem_sp;
   /// The symbol context to use when parsing.

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

@@ -56,9 +56,18 @@ SwiftUserExpression::SwiftUserExpression(
       m_type_system_helper(*m_target_wp.lock().get()),
       m_result_delegate(exe_scope.CalculateTarget(), *this, false),
       m_error_delegate(exe_scope.CalculateTarget(), *this, true),
-      m_persistent_variable_delegate(*this) {
+      m_persistent_variable_delegate(*this),
+      m_swift_scratch_ctx(
+          exe_scope.CalculateTarget()
+              ? exe_scope.CalculateTarget()->GetSwiftScratchContext(m_err,
+                                                                    exe_scope)
+              : llvm::None) {
   m_runs_in_playground_or_repl =
       options.GetREPLEnabled() || options.GetPlaygroundTransformEnabled();
+  if (m_swift_scratch_ctx)
+    if (m_swift_scratch_ctx->get())
+      m_swift_ast_ctx = llvm::dyn_cast_or_null<SwiftASTContextForExpressions>(
+          m_swift_scratch_ctx->get()->GetSwiftASTContext());
 }
 
 SwiftUserExpression::~SwiftUserExpression() {}
@@ -184,22 +193,16 @@ void SwiftUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) {
     return;
   }
 
-  // Make sure the target's SwiftASTContext has been setup before doing any
-  // Swift name lookups.
-  llvm::Optional<SwiftScratchContextReader> maybe_swift_ast_ctx =
-      m_target->GetSwiftScratchContext(err, *frame);
-  if (!maybe_swift_ast_ctx) {
+  if (!m_swift_ast_ctx) {
     LLDB_LOG(log, "  [SUE::SC] NULL Swift AST Context");
     return;
   }
-  SwiftASTContext *swift_ast_ctx = maybe_swift_ast_ctx->get();
-
-  if (!swift_ast_ctx->GetClangImporter()) {
+  if (!m_swift_ast_ctx->GetClangImporter()) {
     LLDB_LOG(log, "  [SUE::SC] Swift AST Context has no Clang importer");
     return;
   }
 
-  if (swift_ast_ctx->HasFatalErrors()) {
+  if (m_swift_ast_ctx->HasFatalErrors()) {
     LLDB_LOG(log, "  [SUE::SC] Swift AST Context has fatal errors");
     return;
   }
@@ -297,39 +300,35 @@ bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
     return false;
   }
 
-  // Make sure the target's SwiftASTContext has been setup before doing any
-  // Swift name lookups.
-  llvm::Optional<SwiftScratchContextReader> maybe_swift_ast_ctx =
-      target->GetSwiftScratchContext(err, *frame);
-  if (!maybe_swift_ast_ctx) {
+  if (!m_swift_ast_ctx) {
     LLDB_LOG(log, "no Swift AST Context");
     return false;
   }
-
-  SwiftASTContext *swift_ast_ctx = maybe_swift_ast_ctx->get();
-
-  if (auto *persistent_state = GetPersistentState(target, exe_ctx)) {
-
-    Status error;
-    SourceModule module_info;
-    module_info.path.emplace_back("Swift");
-    swift::ModuleDecl *module = swift_ast_ctx->GetModule(module_info, error);
-
-    if (error.Fail() || !module) {
-      LLDB_LOG(log, "couldn't load Swift Standard Library\n");
-      return false;
-    }
-
-    persistent_state->AddHandLoadedModule(ConstString("Swift"),
-                                          swift::ImportedModule(module));
-    m_result_delegate.RegisterPersistentState(persistent_state);
-    m_error_delegate.RegisterPersistentState(persistent_state);
-  } else {
+
+  // This may destroy the scratch context.
+  auto *persistent_state = GetPersistentState(target, exe_ctx);
+  if (!persistent_state) {
     diagnostic_manager.PutString(eDiagnosticSeverityError,
                                  "couldn't start parsing (no persistent data)");
     return false;
   }
 
+  Status error;
+  SourceModule module_info;
+  module_info.path.emplace_back("Swift");
+  swift::ModuleDecl *module_decl =
+      m_swift_ast_ctx->GetModule(module_info, error);
+
+  if (error.Fail() || !module_decl) {
+    LLDB_LOG(log, "couldn't load Swift Standard Library\n");
+    return false;
+  }
+
+  persistent_state->AddHandLoadedModule(ConstString("Swift"),
+                                        swift::ImportedModule(module_decl));
+  m_result_delegate.RegisterPersistentState(persistent_state);
+  m_error_delegate.RegisterPersistentState(persistent_state);
+
   ScanContext(exe_ctx, err);
 
   if (!err.Success()) {
@@ -409,7 +408,8 @@ bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
     exe_scope = exe_ctx.GetTargetPtr();
   } while (0);
 
-  auto *swift_parser = new SwiftExpressionParser(exe_scope, *this, m_options);
+  auto *swift_parser =
+      new SwiftExpressionParser(exe_scope, *m_swift_ast_ctx, *this, m_options);
   unsigned error_code = swift_parser->Parse(
       diagnostic_manager, first_body_line,
       first_body_line + source_code->GetNumBodyLines());

lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.h

@@ -179,8 +179,11 @@ class SwiftUserExpression : public LLVMUserExpression {
     void DidDematerialize(lldb::ExpressionVariableSP &variable) override;
   };
 
+  SwiftASTContextForExpressions *m_swift_ast_ctx;
   PersistentVariableDelegate m_persistent_variable_delegate;
   std::unique_ptr<SwiftExpressionParser> m_parser;
+  Status m_err;
+  llvm::Optional<SwiftScratchContextReader> m_swift_scratch_ctx;
   bool m_runs_in_playground_or_repl;
   bool m_needs_object_ptr = false;
   bool m_in_static_method = false;

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

@@ -1561,9 +1561,8 @@ static bool IsPrivateNSClass(NodePointer node) {
 }
 
 bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Class(
-    ValueObject &in_value, SwiftASTContextForExpressions &scratch_ctx,
-    lldb::DynamicValueType use_dynamic, TypeAndOrName &class_type_or_name,
-    Address &address) {
+    ValueObject &in_value, lldb::DynamicValueType use_dynamic,
+    TypeAndOrName &class_type_or_name, Address &address) {
   AddressType address_type;
   lldb::addr_t instance_ptr = in_value.GetPointerValue(&address_type);
   if (instance_ptr == LLDB_INVALID_ADDRESS || instance_ptr == 0)
@@ -1612,7 +1611,17 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Class(
   class_type_or_name.SetCompilerType(ts.RemangleAsType(dem, node));
 
 #ifndef NDEBUG
-  auto &remote_ast = GetRemoteASTContext(scratch_ctx);
+  // Dynamic type resolution in RemoteAST might pull in other Swift modules, so
+  // use the scratch context where such operations are legal and safe.
+  llvm::Optional<SwiftScratchContextReader> maybe_scratch_ctx =
+      in_value.GetSwiftScratchContext();
+  if (!maybe_scratch_ctx)
+    return false;
+  SwiftASTContextForExpressions *scratch_ctx = maybe_scratch_ctx->get();
+  if (!scratch_ctx)
+    return true;
+
+  auto &remote_ast = GetRemoteASTContext(*scratch_ctx);
   auto remote_ast_metadata_address = remote_ast.getHeapMetadataForObject(
       swift::remote::RemoteAddress(instance_ptr));
   if (remote_ast_metadata_address) {
@@ -1705,14 +1714,24 @@ bool SwiftLanguageRuntimeImpl::IsValidErrorValue(ValueObject &in_value) {
 
 bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Protocol(
     ValueObject &in_value, CompilerType protocol_type,
-    SwiftASTContextForExpressions &scratch_ctx,
     lldb::DynamicValueType use_dynamic, TypeAndOrName &class_type_or_name,
     Address &address) {
   auto remote_ast_impl = [&](bool use_local_buffer,
                              lldb::addr_t existential_address)
       -> llvm::Optional<std::pair<CompilerType, Address>> {
+    // Dynamic type resolution in RemoteAST might pull in other Swift modules,
+    // so
+    // use the scratch context where such operations are legal and safe.
+    llvm::Optional<SwiftScratchContextReader> maybe_scratch_ctx =
+        in_value.GetSwiftScratchContext();
+    if (!maybe_scratch_ctx)
+      return {};
+    SwiftASTContextForExpressions *scratch_ctx = maybe_scratch_ctx->get();
+    if (!scratch_ctx)
+      return {};
+
     swift::remote::RemoteAddress remote_existential(existential_address);
-    auto &remote_ast = GetRemoteASTContext(scratch_ctx);
+    auto &remote_ast = GetRemoteASTContext(*scratch_ctx);
     auto swift_type = GetSwiftType(protocol_type);
     if (!swift_type)
       return {};
@@ -1737,11 +1756,6 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Protocol(
   };
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
-
-  auto &target = m_process.GetTarget();
-  assert(IsScratchContextLocked(target) &&
-         "Swift scratch context not locked ahead");
-
   auto *tss =
       llvm::dyn_cast_or_null<TypeSystemSwift>(protocol_type.GetTypeSystem());
   if (!tss) {
@@ -2017,9 +2031,6 @@ CompilerType
 SwiftLanguageRuntimeImpl::BindGenericTypeParameters(StackFrame &stack_frame,
                                                     CompilerType base_type) {
   LLDB_SCOPED_TIMER();
-   auto &target = m_process.GetTarget();
-  assert(IsScratchContextLocked(target) &&
-         "Swift scratch context not locked ahead of archetype binding");
 
   // If this is a TypeRef type, bind that.
   auto sc = stack_frame.GetSymbolContext(lldb::eSymbolContextEverything);
@@ -2029,6 +2040,10 @@ SwiftLanguageRuntimeImpl::BindGenericTypeParameters(StackFrame &stack_frame,
                                      base_type.GetMangledTypeName());
 
   Status error;
+  auto &target = m_process.GetTarget();
+  assert(IsScratchContextLocked(target) &&
+         "Swift scratch context not locked ahead of archetype binding");
+
   // A failing Clang import in a module context permanently damages
   // that module context.  Binding archetypes can trigger an import of
   // another module, so switch to a scratch context where such an
@@ -2545,59 +2560,26 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress(
   if (!CouldHaveDynamicValue(in_value))
     return false;
 
-  // Dynamic type resolution in RemoteAST might pull in other Swift modules, so
-  // use the scratch context where such operations are legal and safe.
-  assert(IsScratchContextLocked(in_value.GetTargetSP()) &&
-         "Swift scratch context not locked ahead of dynamic type resolution");
-  llvm::Optional<SwiftScratchContextReader> maybe_scratch_ctx =
-      in_value.GetSwiftScratchContext();
-  if (!maybe_scratch_ctx)
-    return false;
-  SwiftASTContextForExpressions *scratch_ctx = maybe_scratch_ctx->get();
-
-  auto retry_once = [&]() {
-    // Retry exactly once using the per-module fallback scratch context.
-    auto &target = m_process.GetTarget();
-    if (!target.UseScratchTypesystemPerModule()) {
-      Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
-      if (log)
-        log->Printf("Dynamic type resolution detected fatal errors in "
-                    "shared Swift state. Falling back to per-module "
-                    "scratch context.\n");
-      target.SetUseScratchTypesystemPerModule(true);
-      return GetDynamicTypeAndAddress(in_value, use_dynamic, class_type_or_name,
-                                      address, value_type);
-    }
-    return false;
-  };
-
-  if (scratch_ctx->HasFatalErrors())
-    return retry_once();
-
-  // Import the type into the scratch context. Any form of dynamic
-  // type resolution may trigger a cross-module import.
   CompilerType val_type(in_value.GetCompilerType());
   Flags type_info(val_type.GetTypeInfo());
   if (!type_info.AnySet(eTypeIsSwift))
     return false;
 
   bool success = false;
   bool is_indirect_enum_case = IsIndirectEnumCase(in_value);
-  // Type kinds with metadata don't need archetype binding.
+  // Type kinds with instance metadata don't need generic type resolution.
   if (is_indirect_enum_case)
-    // ..._IndirectEnumCase() recurses, no need to bind archetypes.
     success = GetDynamicTypeAndAddress_IndirectEnumCase(
         in_value, use_dynamic, class_type_or_name, address);
   else if (type_info.AnySet(eTypeIsClass) ||
            type_info.AllSet(eTypeIsBuiltIn | eTypeIsPointer | eTypeHasValue))
-    success = GetDynamicTypeAndAddress_Class(
-        in_value, *scratch_ctx, use_dynamic, class_type_or_name, address);
+    success = GetDynamicTypeAndAddress_Class(in_value, use_dynamic,
+                                             class_type_or_name, address);
   else if (type_info.AnySet(eTypeIsProtocol))
-    success = GetDynamicTypeAndAddress_Protocol(in_value, val_type,
-                                                *scratch_ctx, use_dynamic,
+    success = GetDynamicTypeAndAddress_Protocol(in_value, val_type, use_dynamic,
                                                 class_type_or_name, address);
   else {
-    // Perform archetype binding in the scratch context.
+    // Perform generic type resolution.
     StackFrameSP frame = in_value.GetExecutionContextRef().GetFrameSP();
     if (!frame)
       return false;
@@ -2608,12 +2590,11 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress(
 
     Flags subst_type_info(bound_type.GetTypeInfo());
     if (subst_type_info.AnySet(eTypeIsClass)) {
-      success = GetDynamicTypeAndAddress_Class(
-          in_value, *scratch_ctx, use_dynamic, class_type_or_name, address);
+      success = GetDynamicTypeAndAddress_Class(in_value, use_dynamic,
+                                               class_type_or_name, address);
     } else if (subst_type_info.AnySet(eTypeIsProtocol)) {
-      success = GetDynamicTypeAndAddress_Protocol(in_value, bound_type,
-                                                  *scratch_ctx, use_dynamic,
-                                                  class_type_or_name, address);
+      success = GetDynamicTypeAndAddress_Protocol(
+          in_value, bound_type, use_dynamic, class_type_or_name, address);
     } else {
       success = GetDynamicTypeAndAddress_Value(
           in_value, bound_type, use_dynamic, class_type_or_name, address);
@@ -2623,8 +2604,6 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress(
   if (success)
     value_type = GetValueType(in_value, class_type_or_name.GetCompilerType(),
                               is_indirect_enum_case);
-  else if (scratch_ctx->HasFatalErrors())
-    return retry_once();
   return success;
 }
 

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

@@ -257,16 +257,16 @@ class SwiftLanguageRuntimeImpl {
                                 CompilerType dynamic_type,
                                 bool is_indirect_enum_case);
 
-  bool GetDynamicTypeAndAddress_Class(
-      ValueObject &in_value, SwiftASTContextForExpressions &scratch_ctx,
-      lldb::DynamicValueType use_dynamic, TypeAndOrName &class_type_or_name,
-      Address &address);
-
-  bool GetDynamicTypeAndAddress_Protocol(
-      ValueObject &in_value, CompilerType protocol_type,
-      SwiftASTContextForExpressions &scratch_ctx,
-      lldb::DynamicValueType use_dynamic, TypeAndOrName &class_type_or_name,
-      Address &address);
+  bool GetDynamicTypeAndAddress_Class(ValueObject &in_value,
+                                      lldb::DynamicValueType use_dynamic,
+                                      TypeAndOrName &class_type_or_name,
+                                      Address &address);
+
+  bool GetDynamicTypeAndAddress_Protocol(ValueObject &in_value,
+                                         CompilerType protocol_type,
+                                         lldb::DynamicValueType use_dynamic,
+                                         TypeAndOrName &class_type_or_name,
+                                         Address &address);
 
   bool GetDynamicTypeAndAddress_Value(ValueObject &in_value,
                                       CompilerType &bound_type,