[lldb] Allow LLDB to automatically retry a failed expression with an imported std C++ module

By now LLDB can import the 'std' C++ module to improve expression evaluation,
but there are still a few problems to solve before we can do this by default.
One is that importing the C++ module is slightly slower than normal expression
evaluation (mostly because the disk access and loading the initial lookup data
is quite slow in comparison to the barebone Clang setup the rest of the LLDB
expression evaluator is usually doing). Another problem is that some complicated
types in the standard library aren't fully supported yet by the ASTImporter, so
we end up types that fail to import (which usually appears to the user as if the
type is empty or there is just no result variable).

To still allow people to adopt this mode in their daily debugging, this patch
adds a setting that allows LLDB to automatically retry failed expression with a
loaded C++ module. All success expressions will behave exactly as they would do
before this patch. Failed expressions get a another parse attempt if we find a
usable C++ module in the current execution context. This way we shouldn't have
any performance/parsing regressions in normal debugging workflows, while the
debugging workflows involving STL containers benefit from the C++ module type
info.

This setting is off by default for now with the intention to enable it by
default on macOS soon-ish.

The implementation is mostly just extracting the existing parse logic into its
own function and then calling the parse function again if the first evaluation
failed and we have a C++ module to retry the parsing with.

Reviewed By: shafik, JDevlieghere, aprantl

Differential Revision: https://reviews.llvm.org/D92784

(cherry picked from commit 9586082)

Author: Teemperor

lldb/include/lldb/Target/Target.h

@@ -65,6 +65,12 @@ enum LoadDependentFiles {
   eLoadDependentsNo,
 };
 
+enum ImportStdModule {
+  eImportStdModuleFalse,
+  eImportStdModuleFallback,
+  eImportStdModuleTrue,
+};
+
 class TargetExperimentalProperties : public Properties {
 public:
   TargetExperimentalProperties();
@@ -135,7 +141,7 @@ class TargetProperties : public Properties {
 
   bool GetEnableAutoImportClangModules() const;
 
-  bool GetEnableImportStdModule() const;
+  ImportStdModule GetImportStdModule() const;
 
   bool GetEnableAutoApplyFixIts() const;
 

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

@@ -417,7 +417,6 @@ void ClangUserExpression::CreateSourceCode(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
     std::vector<std::string> modules_to_import, bool for_completion) {
 
-  m_filename = m_clang_state->GetNextExprFileName();
   std::string prefix = m_expr_prefix;
 
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
@@ -477,9 +476,6 @@ CppModuleConfiguration GetModuleConfig(lldb::LanguageType language,
   if (!target)
     return LogConfigError("No target");
 
-  if (!target->GetEnableImportStdModule())
-    return LogConfigError("Importing std module not enabled in settings");
-
   StackFrame *frame = exe_ctx.GetFramePtr();
   if (!frame)
     return LogConfigError("No frame");
@@ -529,8 +525,6 @@ CppModuleConfiguration GetModuleConfig(lldb::LanguageType language,
 bool ClangUserExpression::PrepareForParsing(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
     bool for_completion) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
   InstallContext(exe_ctx);
 
   if (!SetupPersistentState(diagnostic_manager, exe_ctx))
@@ -551,50 +545,20 @@ bool ClangUserExpression::PrepareForParsing(
 
   SetupDeclVendor(exe_ctx, m_target, diagnostic_manager);
 
-  CppModuleConfiguration module_config = GetModuleConfig(m_language, exe_ctx);
-  llvm::ArrayRef<std::string> imported_modules =
-      module_config.GetImportedModules();
-  m_imported_cpp_modules = !imported_modules.empty();
-  m_include_directories = module_config.GetIncludeDirs();
+  m_filename = m_clang_state->GetNextExprFileName();
 
-  LLDB_LOG(log, "List of imported modules in expression: {0}",
-           llvm::make_range(imported_modules.begin(), imported_modules.end()));
-  LLDB_LOG(log, "List of include directories gathered for modules: {0}",
-           llvm::make_range(m_include_directories.begin(),
-                            m_include_directories.end()));
+  if (m_target->GetImportStdModule() == eImportStdModuleTrue)
+    SetupCppModuleImports(exe_ctx);
 
-  CreateSourceCode(diagnostic_manager, exe_ctx, imported_modules,
+  CreateSourceCode(diagnostic_manager, exe_ctx, m_imported_cpp_modules,
                    for_completion);
   return true;
 }
 
-bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
-                                ExecutionContext &exe_ctx,
-                                lldb_private::ExecutionPolicy execution_policy,
-                                bool keep_result_in_memory,
-                                bool generate_debug_info) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
-  if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ false))
-    return false;
-
-  LLDB_LOGF(log, "Parsing the following code:\n%s", m_transformed_text.c_str());
-
-  ////////////////////////////////////
-  // Set up the target and compiler
-  //
-
-  Target *target = exe_ctx.GetTargetPtr();
-
-  if (!target) {
-    diagnostic_manager.PutString(eDiagnosticSeverityError, "invalid target");
-    return false;
-  }
-
-  //////////////////////////
-  // Parse the expression
-  //
-
+bool ClangUserExpression::TryParse(
+    DiagnosticManager &diagnostic_manager, ExecutionContextScope *exe_scope,
+    ExecutionContext &exe_ctx, lldb_private::ExecutionPolicy execution_policy,
+    bool keep_result_in_memory, bool generate_debug_info) {
   m_materializer_up = std::make_unique<Materializer>();
 
   ResetDeclMap(exe_ctx, m_result_delegate, keep_result_in_memory);
@@ -612,26 +576,16 @@ bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
     DeclMap()->SetLookupsEnabled(true);
   }
 
-  Process *process = exe_ctx.GetProcessPtr();
-  ExecutionContextScope *exe_scope = process;
-
-  if (!exe_scope)
-    exe_scope = exe_ctx.GetTargetPtr();
-
-  // We use a shared pointer here so we can use the original parser - if it
-  // succeeds or the rewrite parser we might make if it fails.  But the
-  // parser_sp will never be empty.
-
-  ClangExpressionParser parser(exe_scope, *this, generate_debug_info,
-                               m_include_directories, m_filename);
+  m_parser = std::make_unique<ClangExpressionParser>(
+      exe_scope, *this, generate_debug_info, m_include_directories, m_filename);
 
-  unsigned num_errors = parser.Parse(diagnostic_manager);
+  unsigned num_errors = m_parser->Parse(diagnostic_manager);
 
   // Check here for FixItHints.  If there are any try to apply the fixits and
   // set the fixed text in m_fixed_text before returning an error.
   if (num_errors) {
     if (diagnostic_manager.HasFixIts()) {
-      if (parser.RewriteExpression(diagnostic_manager)) {
+      if (m_parser->RewriteExpression(diagnostic_manager)) {
         size_t fixed_start;
         size_t fixed_end;
         m_fixed_text = diagnostic_manager.GetFixedExpression();
@@ -652,7 +606,7 @@ bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
   //
 
   {
-    Status jit_error = parser.PrepareForExecution(
+    Status jit_error = m_parser->PrepareForExecution(
         m_jit_start_addr, m_jit_end_addr, m_execution_unit_sp, exe_ctx,
         m_can_interpret, execution_policy);
 
@@ -666,10 +620,91 @@ bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
       return false;
     }
   }
+  return true;
+}
+
+void ClangUserExpression::SetupCppModuleImports(ExecutionContext &exe_ctx) {
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+
+  CppModuleConfiguration module_config = GetModuleConfig(m_language, exe_ctx);
+  m_imported_cpp_modules = module_config.GetImportedModules();
+  m_include_directories = module_config.GetIncludeDirs();
+
+  LLDB_LOG(log, "List of imported modules in expression: {0}",
+           llvm::make_range(m_imported_cpp_modules.begin(),
+                            m_imported_cpp_modules.end()));
+  LLDB_LOG(log, "List of include directories gathered for modules: {0}",
+           llvm::make_range(m_include_directories.begin(),
+                            m_include_directories.end()));
+}
+
+static bool shouldRetryWithCppModule(Target &target, ExecutionPolicy exe_policy) {
+  // Top-level expression don't yet support importing C++ modules.
+  if (exe_policy == ExecutionPolicy::eExecutionPolicyTopLevel)
+    return false;
+  return target.GetImportStdModule() == eImportStdModuleFallback;
+}
+
+bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
+                                ExecutionContext &exe_ctx,
+                                lldb_private::ExecutionPolicy execution_policy,
+                                bool keep_result_in_memory,
+                                bool generate_debug_info) {
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+
+  if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ false))
+    return false;
+
+  LLDB_LOGF(log, "Parsing the following code:\n%s", m_transformed_text.c_str());
+
+  ////////////////////////////////////
+  // Set up the target and compiler
+  //
+
+  Target *target = exe_ctx.GetTargetPtr();
+
+  if (!target) {
+    diagnostic_manager.PutString(eDiagnosticSeverityError, "invalid target");
+    return false;
+  }
+
+  //////////////////////////
+  // Parse the expression
+  //
+
+  Process *process = exe_ctx.GetProcessPtr();
+  ExecutionContextScope *exe_scope = process;
+
+  if (!exe_scope)
+    exe_scope = exe_ctx.GetTargetPtr();
+
+  bool parse_success = TryParse(diagnostic_manager, exe_scope, exe_ctx,
+                                execution_policy, keep_result_in_memory,
+                                generate_debug_info);
+  // If the expression failed to parse, check if retrying parsing with a loaded
+  // C++ module is possible.
+  if (!parse_success && shouldRetryWithCppModule(*target, execution_policy)) {
+    // Load the loaded C++ modules.
+    SetupCppModuleImports(exe_ctx);
+    // If we did load any modules, then retry parsing.
+    if (!m_imported_cpp_modules.empty()) {
+      // The module imports are injected into the source code wrapper,
+      // so recreate those.
+      CreateSourceCode(diagnostic_manager, exe_ctx, m_imported_cpp_modules,
+                       /*for_completion*/ false);
+      // Clear the error diagnostics from the previous parse attempt.
+      diagnostic_manager.Clear();
+      parse_success = TryParse(diagnostic_manager, exe_scope, exe_ctx,
+                               execution_policy, keep_result_in_memory,
+                               generate_debug_info);
+    }
+  }
+  if (!parse_success)
+    return false;
 
   if (exe_ctx.GetProcessPtr() && execution_policy == eExecutionPolicyTopLevel) {
     Status static_init_error =
-        parser.RunStaticInitializers(m_execution_unit_sp, exe_ctx);
+        m_parser->RunStaticInitializers(m_execution_unit_sp, exe_ctx);
 
     if (!static_init_error.Success()) {
       const char *error_cstr = static_init_error.AsCString();

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

@@ -168,12 +168,23 @@ class ClangUserExpression : public LLVMUserExpression {
   lldb::ExpressionVariableSP
   GetResultAfterDematerialization(ExecutionContextScope *exe_scope) override;
 
-  bool DidImportCxxModules() const { return m_imported_cpp_modules; }
+  /// Returns true iff this expression is using any imported C++ modules.
+  bool DidImportCxxModules() const { return !m_imported_cpp_modules.empty(); }
 
 private:
   /// Populate m_in_cplusplus_method and m_in_objectivec_method based on the
   /// environment.
 
+  /// Contains the actual parsing implementation.
+  /// The parameter have the same meaning as in ClangUserExpression::Parse.
+  /// \see ClangUserExpression::Parse
+  bool TryParse(DiagnosticManager &diagnostic_manager,
+                ExecutionContextScope *exe_scope, ExecutionContext &exe_ctx,
+                lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
+                bool generate_debug_info);
+
+  void SetupCppModuleImports(ExecutionContext &exe_ctx);
+
   void ScanContext(ExecutionContext &exe_ctx,
                    lldb_private::Status &err) override;
 
@@ -219,15 +230,18 @@ class ClangUserExpression : public LLVMUserExpression {
   ResultDelegate m_result_delegate;
   ClangPersistentVariables *m_clang_state;
   std::unique_ptr<ClangExpressionSourceCode> m_source_code;
+  /// The parser instance we used to parse the expression.
+  std::unique_ptr<ClangExpressionParser> m_parser;
   /// File name used for the expression.
   std::string m_filename;
 
   /// The object (if any) in which context the expression is evaluated.
   /// See the comment to `UserExpression::Evaluate` for details.
   ValueObject *m_ctx_obj;
 
-  /// True iff this expression explicitly imported C++ modules.
-  bool m_imported_cpp_modules = false;
+  /// A list of module names that should be imported when parsing.
+  /// \see CppModuleConfiguration::GetImportedModules
+  std::vector<std::string> m_imported_cpp_modules;
 
   /// True if the expression parser should enforce the presence of a valid class
   /// pointer in order to generate the expression as a method.

lldb/source/Target/Target.cpp

@@ -3461,6 +3461,28 @@ static constexpr OptionEnumValueElement g_x86_dis_flavor_value_types[] = {
     },
 };
 
+static constexpr OptionEnumValueElement g_import_std_module_value_types[] = {
+    {
+        eImportStdModuleFalse,
+        "false",
+        "Never import the 'std' C++ module in the expression parser.",
+    },
+    {
+        eImportStdModuleFallback,
+        "fallback",
+        "Retry evaluating expressions with an imported 'std' C++ module if they"
+        " failed to parse without the module. This allows evaluating more "
+        "complex expressions involving C++ standard library types."
+    },
+    {
+        eImportStdModuleTrue,
+        "true",
+        "Always import the 'std' C++ module. This allows evaluating more "
+        "complex expressions involving C++ standard library types. This feature"
+        " is experimental."
+    },
+};
+
 static constexpr OptionEnumValueElement g_hex_immediate_style_values[] = {
     {
         Disassembler::eHexStyleC,
@@ -3914,10 +3936,10 @@ bool TargetProperties::GetEnableAutoImportClangModules() const {
       nullptr, idx, g_target_properties[idx].default_uint_value != 0);
 }
 
-bool TargetProperties::GetEnableImportStdModule() const {
+ImportStdModule TargetProperties::GetImportStdModule() const {
   const uint32_t idx = ePropertyImportStdModule;
-  return m_collection_sp->GetPropertyAtIndexAsBoolean(
-      nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+  return (ImportStdModule)m_collection_sp->GetPropertyAtIndexAsEnumeration(
+      nullptr, idx, g_target_properties[idx].default_uint_value);
 }
 
 bool TargetProperties::GetEnableAutoApplyFixIts() const {

lldb/source/Target/TargetProperties.td

@@ -45,9 +45,11 @@ let Definition = "target" in {
   def AutoImportClangModules: Property<"auto-import-clang-modules", "Boolean">,
     DefaultTrue,
     Desc<"Automatically load Clang modules referred to by the program.">;
-  def ImportStdModule: Property<"import-std-module", "Boolean">,
-    DefaultFalse,
-    Desc<"Import the C++ std module to improve debugging STL containers.">;
+  def ImportStdModule: Property<"import-std-module", "Enum">,
+    DefaultEnumValue<"eImportStdModuleFalse">,
+    EnumValues<"OptionEnumValues(g_import_std_module_value_types)">,
+    Desc<"Import the 'std' C++ module to improve expression parsing involving "
+         " C++ standard library types.">;
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
     DefaultTrue,
     Desc<"Automatically apply fix-it hints to expressions.">;

lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile

@@ -0,0 +1,3 @@
+USE_LIBCPP := 1
+CXX_SOURCES := main.cpp
+include Makefile.rules