Move HandloadedModules from SwiftPersistentExprState to SwiftASTContextFE (NFC)

This change is (somewhat surprisingly) NFC, since the list is only
consumed by SwiftExpressionParser and only populated by it as well.
When SwiftASTContextForExpressions is destroyed the persistent state is too.

Author: adrian-prantl

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

@@ -1261,9 +1261,6 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
   // FIXME: We won't have to do this once the playground adds import
   //        statements for the things it needs itself.
   if (playground) {
-    auto *persistent_state =
-        sc.target_sp->GetSwiftPersistentExpressionState(exe_scope);
-
     Status error;
     SourceModule module_info;
     module_info.path.emplace_back("Swift");
@@ -1274,7 +1271,7 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
       return error.ToError();
     }
 
-    persistent_state->AddHandLoadedModule(ConstString("Swift"),
+    swift_ast_context.AddHandLoadedModule(ConstString("Swift"),
                                           swift::ImportedModule(module));
   }
 
@@ -1297,9 +1294,8 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
   if (lldb::StackFrameSP this_frame_sp = stack_frame_wp.lock())
     process_sp = this_frame_sp->CalculateProcess();
   swift_ast_context.LoadImplicitModules(sc.target_sp, process_sp, exe_scope);
-  if (!SwiftASTContext::GetImplicitImports(swift_ast_context, sc, exe_scope,
-                                           process_sp, additional_imports,
-                                           implicit_import_error)) {
+  if (!swift_ast_context.GetImplicitImports(sc, process_sp, additional_imports,
+                                            implicit_import_error)) {
     const char *msg = implicit_import_error.AsCString();
     if (!msg)
       msg = "error status positive, but import still failed";
@@ -1449,9 +1445,8 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
         IRExecutionUnit::GetLLVMGlobalContextMutex());
 
     Status auto_import_error;
-    if (!SwiftASTContext::CacheUserImports(swift_ast_context, sc, exe_scope,
-                                           process_sp, *source_file,
-                                           auto_import_error)) {
+    if (!swift_ast_context.CacheUserImports(process_sp, *source_file,
+                                            auto_import_error)) {
       const char *msg = auto_import_error.AsCString();
       if (!msg) {
         // The import itself succeeded, but the AST context is in a

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

@@ -15,10 +15,6 @@
 
 #include "SwiftExpressionVariable.h"
 
-#include "swift/AST/Import.h"
-#include "swift/AST/Module.h"
-
-#include "lldb/Core/SwiftForward.h"
 #include "lldb/Expression/ExpressionVariable.h"
 
 #include "llvm/ADT/StringMap.h"
@@ -38,10 +34,6 @@ namespace lldb_private {
 /// Also provides an increasing, 0-based counter for naming result
 /// variables.
 class SwiftPersistentExpressionState : public PersistentExpressionState {
-
-  typedef llvm::StringMap<swift::AttributedImport<swift::ImportedModule>>
-      HandLoadedModuleSet;
-
 public:
   class SwiftDeclMap {
   public:
@@ -113,28 +105,13 @@ class SwiftPersistentExpressionState : public PersistentExpressionState {
       const std::vector<CompilerDecl> &excluding_equivalents,
       std::vector<CompilerDecl> &matches);
 
-  // This just adds this module to the list of hand-loaded modules, it doesn't
-  // actually load it.
-  void AddHandLoadedModule(
-      ConstString module_name,
-      swift::AttributedImport<swift::ImportedModule> attributed_import) {
-    m_hand_loaded_modules.insert_or_assign(module_name.GetStringRef(),
-                                           attributed_import);
-  }
-
-  /// This returns the list of hand-loaded modules.
-  HandLoadedModuleSet GetHandLoadedModules() { return m_hand_loaded_modules; }
-
 private:
   /// The counter used by GetNextResultName().
   uint32_t m_next_persistent_variable_id;
   /// The counter used by GetNextResultName() when is_error is true.
   uint32_t m_next_persistent_error_id;
   /// The persistent functions declared by the user.
   SwiftDeclMap m_swift_persistent_decls;
-  /// These are the names of modules that we have loaded by hand into
-  /// the Contexts we make for parsing.
-  HandLoadedModuleSet m_hand_loaded_modules;
 };
 } // namespace lldb_private
 

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

@@ -712,8 +712,8 @@ bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
   if (status.Fail() || !module_decl)
     return error("could not load Swift Standard Library", status.AsCString());
 
-  persistent_state->AddHandLoadedModule(ConstString("Swift"),
-                                        swift::ImportedModule(module_decl));
+  m_swift_ast_ctx->AddHandLoadedModule(ConstString("Swift"),
+                                       swift::ImportedModule(module_decl));
   m_result_delegate.RegisterPersistentState(persistent_state);
   m_error_delegate.RegisterPersistentState(persistent_state);
 

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

@@ -8226,24 +8226,18 @@ static swift::ModuleDecl *LoadOneModule(const SourceModule &module,
   return swift_module;
 }
 
-bool SwiftASTContext::GetImplicitImports(
-    SwiftASTContext &swift_ast_context, SymbolContext &sc,
-    ExecutionContextScope &exe_scope, lldb::ProcessSP process_sp,
+bool SwiftASTContextForExpressions::GetImplicitImports(
+    SymbolContext &sc, lldb::ProcessSP process_sp,
     llvm::SmallVectorImpl<swift::AttributedImport<swift::ImportedModule>>
         &modules,
     Status &error) {
   LLDB_SCOPED_TIMER();
-  if (!swift_ast_context.GetCompileUnitImports(sc, process_sp, modules,
-                                               error)) {
+  if (!GetCompileUnitImports(sc, process_sp, modules, error)) {
     return false;
   }
 
-  auto *persistent_expression_state =
-      sc.target_sp->GetSwiftPersistentExpressionState(exe_scope);
-
   // Get the hand-loaded modules from the SwiftPersistentExpressionState.
-  for (auto &module_pair :
-       persistent_expression_state->GetHandLoadedModules()) {
+  for (auto &module_pair : m_hand_loaded_modules) {
 
     auto &attributed_import = module_pair.second;
 
@@ -8257,7 +8251,7 @@ bool SwiftASTContext::GetImplicitImports(
     // Otherwise, try reloading the ModuleDecl using the module name.
     SourceModule module_info;
     module_info.path.emplace_back(module_pair.first());
-    auto *module = LoadOneModule(module_info, swift_ast_context, process_sp,
+    auto *module = LoadOneModule(module_info, *this, process_sp,
                                  /*import_dylibs=*/false, error);
     if (!module)
       return false;
@@ -8268,8 +8262,8 @@ bool SwiftASTContext::GetImplicitImports(
   return true;
 }
 
-void SwiftASTContext::LoadImplicitModules(TargetSP target, ProcessSP process,
-                                          ExecutionContextScope &exe_scope) {
+void SwiftASTContextForExpressions::LoadImplicitModules(
+    TargetSP target, ProcessSP process, ExecutionContextScope &exe_scope) {
   auto load_module = [&](ConstString module_name) {
     SourceModule module_info;
     module_info.path.push_back(module_name);
@@ -8282,8 +8276,6 @@ void SwiftASTContext::LoadImplicitModules(TargetSP target, ProcessSP process,
       return;
     }
 
-    auto *persistent_expression_state =
-        target->GetSwiftPersistentExpressionState(exe_scope);
     swift::ModuleDecl *module = GetModule(module_info, err);
     if (err.Fail()) {
       LOG_PRINTF(
@@ -8293,25 +8285,17 @@ void SwiftASTContext::LoadImplicitModules(TargetSP target, ProcessSP process,
       return;
     }
 
-    persistent_expression_state->AddHandLoadedModule(
-        module_name, swift::ImportedModule(module));
+    AddHandLoadedModule(module_name, swift::ImportedModule(module));
   };
 
   load_module(ConstString(swift::SWIFT_STRING_PROCESSING_NAME));
   load_module(ConstString(swift::SWIFT_CONCURRENCY_NAME));
 }
 
-bool SwiftASTContext::CacheUserImports(SwiftASTContext &swift_ast_context,
-                                       SymbolContext &sc,
-                                       ExecutionContextScope &exe_scope,
-                                       lldb::ProcessSP process_sp,
-                                       swift::SourceFile &source_file,
-                                       Status &error) {
+bool SwiftASTContextForExpressions::CacheUserImports(
+    lldb::ProcessSP process_sp, swift::SourceFile &source_file, Status &error) {
   llvm::SmallString<1> m_description;
 
-  auto *persistent_expression_state =
-      sc.target_sp->GetSwiftPersistentExpressionState(exe_scope);
-
   auto src_file_imports = source_file.getImports();
 
   Progress progress(llvm::formatv("Caching Swift user imports from '{0}'",
@@ -8347,13 +8331,12 @@ bool SwiftASTContext::CacheUserImports(SwiftASTContext &swift_ast_context,
         LOG_PRINTF(GetLog(LLDBLog::Types | LLDBLog::Expressions),
                    "Performing auto import on found module: %s.\n",
                    module_name.c_str());
-        if (!LoadOneModule(module_info, swift_ast_context, process_sp,
+        if (!LoadOneModule(module_info, *this, process_sp,
                            /*import_dylibs=*/true, error))
           return false;
 
         // How do we tell we are in REPL or playground mode?
-        persistent_expression_state->AddHandLoadedModule(module_const_str,
-                                                         attributed_import);
+        AddHandLoadedModule(module_const_str, attributed_import);
       }
     }
   }

lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h

@@ -22,6 +22,8 @@
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Utility/Either.h"
 
+#include "swift/AST/Import.h"
+#include "swift/AST/Module.h"
 #include "swift/Parse/ParseVersion.h"
 #include "swift/SymbolGraphGen/SymbolGraphOptions.h"
 
@@ -806,30 +808,6 @@ class SwiftASTContext : public TypeSystemSwift {
 
   CompilerType GetReferentType(lldb::opaque_compiler_type_t type) override;
 
-  /// Retrieves the modules that need to be implicitly imported in a given
-  /// execution scope. This includes the modules imported by both the compile
-  /// unit as well as any imports from previous expression evaluations.
-  static bool GetImplicitImports(
-      SwiftASTContext &swift_ast_context, SymbolContext &sc,
-      ExecutionContextScope &exe_scope, lldb::ProcessSP process_sp,
-      llvm::SmallVectorImpl<swift::AttributedImport<swift::ImportedModule>>
-          &modules,
-      Status &error);
-
-  // FIXME: the correct thing to do would be to get the modules by calling
-  // CompilerInstance::getImplicitImportInfo, instead of loading these
-  // modules manually. However, we currently don't have  access to a
-  // CompilerInstance, which is why this function is needed.
-  void LoadImplicitModules(lldb::TargetSP target, lldb::ProcessSP process,
-                           ExecutionContextScope &exe_scope);
-  /// Cache the user's imports from a SourceFile in a given execution scope such
-  /// that they are carried over into future expression evaluations.
-  static bool CacheUserImports(SwiftASTContext &swift_ast_context,
-                               SymbolContext &sc,
-                               ExecutionContextScope &exe_scope,
-                               lldb::ProcessSP process_sp,
-                               swift::SourceFile &source_file, Status &error);
-
   /// Retrieve/import the modules imported by the compilation
   /// unit. Early-exists with false if there was an import failure.
   bool GetCompileUnitImports(
@@ -1037,6 +1015,42 @@ class SwiftASTContextForExpressions : public SwiftASTContext {
 
   void ModulesDidLoad(ModuleList &module_list);
 
+  typedef llvm::StringMap<swift::AttributedImport<swift::ImportedModule>>
+      HandLoadedModuleSet;
+
+  // Insert to the list of hand-loaded modules, (no actual loading occurs).
+  void AddHandLoadedModule(
+      ConstString module_name,
+      swift::AttributedImport<swift::ImportedModule> attributed_import) {
+    m_hand_loaded_modules.insert_or_assign(module_name.GetStringRef(),
+                                           attributed_import);
+  }
+
+  /// Retrieves the modules that need to be implicitly imported in a given
+  /// execution scope. This includes the modules imported by both the compile
+  /// unit as well as any imports from previous expression evaluations.
+  bool GetImplicitImports(
+      SymbolContext &sc, lldb::ProcessSP process_sp,
+      llvm::SmallVectorImpl<swift::AttributedImport<swift::ImportedModule>>
+          &modules,
+      Status &error);
+
+  // FIXME: the correct thing to do would be to get the modules by calling
+  // CompilerInstance::getImplicitImportInfo, instead of loading these
+  // modules manually. However, we currently don't have  access to a
+  // CompilerInstance, which is why this function is needed.
+  void LoadImplicitModules(lldb::TargetSP target, lldb::ProcessSP process,
+                           ExecutionContextScope &exe_scope);
+  /// Cache the user's imports from a SourceFile in a given execution scope such
+  /// that they are carried over into future expression evaluations.
+  bool CacheUserImports(lldb::ProcessSP process_sp,
+                        swift::SourceFile &source_file, Status &error);
+
+protected:
+  /// These are the names of modules that we have loaded by hand into
+  /// the Contexts we make for parsing.
+  HandLoadedModuleSet m_hand_loaded_modules;
+
 private:
   std::unique_ptr<SwiftPersistentExpressionState> m_persistent_state_up;
 };