[lldb] Run generic expression evaluator first

The generic expression evaluator can be significantly faster than the
regular one since it only needs to import information about the module
the debugger is stopped at to evaluate the expression. This patch
swaps the order of the expression evaluators so that the generic
expression evaluator is tried first.

rdar://139239935

Author: augusto2112

lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h

@@ -160,8 +160,9 @@ static constexpr OptionEnumValueElement g_bind_gen_type_params[] = {
     {
         lldb::eBindAuto,
         "auto",
-        "Attempt to run the expression with bound generic parameters first, "
-        "fallback to unbound generic parameters if binding the type parameters "
+        "Attempt to run the expression with unbound generic parameters first, "
+        "fallback to binding the generic parameters if the expression with "
+        "unbound generic type parameters "
         "fails",
     },
     {lldb::eBind, "true", "Bind generic type parameters."},

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

@@ -878,7 +878,7 @@ swift::FuncDecl *SwiftASTManipulator::GetFunctionToInjectVariableInto(
   // When not binding generic type parameters, we want to inject the metadata
   // pointers in the wrapper, so we can pass them as opaque pointers in the
   // trampoline function later on.
-  if (m_bind_generic_types == lldb::eDontBind &&
+  if (!ShouldBindGenericTypes(m_bind_generic_types) &&
       (variable.IsMetadataPointer() || variable.IsPackCount() ||
        variable.IsUnboundPack()))
     return m_entrypoint_decl;
@@ -896,7 +896,8 @@ llvm::Expected<swift::Type> SwiftASTManipulator::GetSwiftTypeForVariable(
 
   // When injecting a value pack or pack count into the outer
   // lldb_expr function, treat it as an opaque raw pointer.
-  if (m_bind_generic_types == lldb::eDontBind && variable.IsUnboundPack()) {
+  if (!ShouldBindGenericTypes(m_bind_generic_types) &&
+      variable.IsUnboundPack()) {
     auto swift_ast_ctx = type_system_swift->GetSwiftASTContext(m_sc);
     if (!swift_ast_ctx)
       return llvm::createStringError("no typesystem for variable " +

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

@@ -284,6 +284,12 @@ class SwiftASTManipulator : public SwiftASTManipulatorBase {
                                const EvaluateExpressionOptions &options,
                                std::string &expr_source_path);
 
+  // By default expression evaluation should not try to bind the generic types.
+  static bool
+  ShouldBindGenericTypes(lldb::BindGenericTypes bind_generic_types) {
+    return bind_generic_types == lldb::eBind;
+  }
+
   swift::FuncDecl *GetEntrypointDecl() const {
     return m_entrypoint_decl;
   }

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

@@ -448,7 +448,7 @@ static CompilerType GetSwiftTypeForVariableValueObject(
   CompilerType result = valobj_sp->GetCompilerType();
   if (!result)
     return {};
-  if (bind_generic_types != lldb::eDontBind)
+  if (SwiftASTManipulator::ShouldBindGenericTypes(bind_generic_types))
     result = runtime->BindGenericTypeParameters(*stack_frame_sp, result);
   if (!result)
     return {};
@@ -489,7 +489,8 @@ CompilerType SwiftExpressionParser::ResolveVariable(
   // though, as we don't bind the generic parameters in that case.
   if (swift_type_system->IsMeaninglessWithoutDynamicResolution(
           var_type.GetOpaqueQualType()) &&
-      bind_generic_types != lldb::eDontBind && use_dynamic_value) {
+      SwiftASTManipulator::ShouldBindGenericTypes(bind_generic_types) &&
+      use_dynamic_value) {
     var_type = GetSwiftTypeForVariableValueObject(
         valobj_sp->GetDynamicValue(use_dynamic), stack_frame_sp, runtime,
         bind_generic_types);
@@ -583,7 +584,7 @@ AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
         "self type from an import isn't valid.");
 
   auto *stack_frame = stack_frame_sp.get();
-  if (bind_generic_types != lldb::eDontBind) {
+  if (SwiftASTManipulator::ShouldBindGenericTypes(bind_generic_types)) {
     imported_self_type = swift_runtime->BindGenericTypeParameters(
         *stack_frame, imported_self_type);
     if (!imported_self_type)
@@ -1710,6 +1711,11 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
   auto expr_diagnostics = m_swift_ast_ctx.getScopedDiagnosticConsumer();
   m_swift_ast_ctx.GetDiagnosticEngine().resetHadAnyError();
 
+  // The result in case parsing the expression fails. If the option is auto
+  // expression evaluation should retry by binding the generic types.
+  auto parse_result_failure = m_options.GetBindGenericTypes() == lldb::eBindAuto
+                                  ? ParseResult::retry_bind_generic_params
+                                  : ParseResult::unrecoverable_error;
   // Helper function to diagnose errors in m_swift_scratch_context.
   unsigned buffer_id = UINT32_MAX;
   auto DiagnoseSwiftASTContextError = [&]() {
@@ -1726,7 +1732,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
   const bool playground = m_options.GetPlaygroundTransformEnabled();
 
   if (!m_exe_scope)
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
 
   // Parse the expression and import all nececssary swift modules.
   auto parsed_expr = ParseAndImport(*expr_diagnostics, variable_map, buffer_id,
@@ -1767,7 +1773,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
       return ParseResult::retry_fresh_context;
 
     // Unrecoverable error.
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   // If any generics are present, this expression is not parseable.
@@ -1809,11 +1815,11 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
           "Missing type debug information for variable \"%s\": %s",
           var.GetName().str().str().c_str(),
           llvm::toString(std::move(error)).c_str());
-      return ParseResult::unrecoverable_error;
+      return parse_result_failure;
     }
     // Otherwise print the diagnostics from the Swift compiler.
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   if (repl)
@@ -1826,7 +1832,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
     if (error) {
       diagnostic_manager.PutString(eSeverityError,
                                    llvm::toString(std::move(error)));
-      return ParseResult::unrecoverable_error;
+      return parse_result_failure;
     }
   } else {
     swift::performPlaygroundTransform(
@@ -1842,7 +1848,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
     diagnostic_manager.PutString(
         eSeverityError,
         "Cannot evaluate an expression that results in a ~Copyable type");
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   // FIXME: We now should have to do the name binding and type
@@ -1965,10 +1971,9 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
         if (!var_info) {
           auto error_string = llvm::toString(var_info.takeError());
           LLDB_LOG(log, "Variable info failzed to materialize with error: {0}",
-              error_string);
-                    
+                   error_string);
 
-          return ParseResult::unrecoverable_error;
+          return parse_result_failure;
         }
 
         const char *name = ConstString(variable.GetName().get()).GetCString();
@@ -2001,7 +2006,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
 
   if (expr_diagnostics->HasErrors()) {
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   if (log) {
@@ -2025,7 +2030,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
 
   if (expr_diagnostics->HasErrors()) {
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   {
@@ -2063,14 +2068,14 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
     diagnostic_manager.PutString(
         eSeverityInfo, "couldn't IRGen expression: Clang importer error");
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   if (error_kind == ErrorKind::swift) {
     diagnostic_manager.PutString(eSeverityInfo,
                                  "couldn't IRGen expression: Swift error");
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   if (!m_module) {
@@ -2079,7 +2084,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
         "couldn't IRGen expression. Please enable the expression log by "
         "running \"log enable lldb expr\", then run the failing expression "
         "again, and file a bug report with the log output.");
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   if (log) {
@@ -2091,7 +2096,8 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
   }
 
   if (ThreadSafeASTContext ast_ctx = m_swift_ast_ctx.GetASTContext()) {
-    if (m_options.GetBindGenericTypes() == lldb::eDontBind &&
+    if (!SwiftASTManipulator::ShouldBindGenericTypes(
+            m_options.GetBindGenericTypes()) &&
         !RedirectCallFromSinkToTrampolineFunction(
             *m_module.get(), *parsed_expr->code_manipulator.get(), **ast_ctx)) {
       diagnostic_manager.Printf(
@@ -2100,7 +2106,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
           "expression log by running \"log enable lldb "
           "expr\", then run the failing expression again, and file a "
           "bugreport with the log output.");
-      return ParseResult::unrecoverable_error;
+      return parse_result_failure;
     }
   }
 
@@ -2120,14 +2126,14 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
                                        LLVMReturnStatusAction, nullptr);
     if (has_errors) {
       diagnostic_manager.PutString(eSeverityInfo, "LLVM verification error");
-      return ParseResult::unrecoverable_error;
+      return parse_result_failure;
     }
   }
 
   if (expr_diagnostics->HasErrors()) {
     diagnostic_manager.PutString(eSeverityInfo, "post-IRGen error");
     DiagnoseSwiftASTContextError();
-    return ParseResult::unrecoverable_error;
+    return parse_result_failure;
   }
 
   // The Parse succeeded!  Now put this module into the context's list

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

@@ -51,7 +51,10 @@ class SwiftExpressionParser : public ExpressionParser {
   enum class ParseResult {
     success,
     retry_fresh_context,
-    retry_no_bind_generic_params,
+    retry_bind_generic_params,
+    // Retry by binding the generic parameters because the generic expression
+    // evaluator does not support running this expression.
+    retry_bind_generic_params_not_supported,
     unrecoverable_error
   };
   //------------------------------------------------------------------

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

@@ -408,7 +408,8 @@ do {
         weak_self ? "Swift.Optional where Wrapped == " : "";
 
     // The expression text is inserted into the body of $__lldb_user_expr_%u.
-    if (options.GetBindGenericTypes() == lldb::eDontBind) {
+    if (!SwiftASTManipulator::ShouldBindGenericTypes(
+            options.GetBindGenericTypes())) {
       // A Swift program can't have types with non-bound generic type parameters
       // inside a non generic function. For example, the following program would
       // not compile as T is not part of foo's signature.
@@ -511,7 +512,8 @@ func $__lldb_expr(_ $__lldb_arg : UnsafeMutablePointer<Any>) {
                             wrapped_expr_text.GetData(), availability.c_str(),
                             current_counter);
     }
-  } else if (options.GetBindGenericTypes() == lldb::eDontBind) {
+  } else if (!SwiftASTManipulator::ShouldBindGenericTypes(
+                 options.GetBindGenericTypes())) {
     auto c = MakeGenericSignaturesAndCalls(local_variables, generic_sig,
                                            needs_object_ptr);
     if (!c) {

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

@@ -322,16 +322,18 @@ static llvm::Error AddVariableInfo(
     return llvm::Error::success();
 
   CompilerType target_type;
+  bool should_not_bind_generic_types =
+      !SwiftASTManipulator::ShouldBindGenericTypes(bind_generic_types);
   bool is_unbound_pack =
-      bind_generic_types == lldb::eDontBind &&
+      should_not_bind_generic_types &&
       (variable_sp->GetType()->GetForwardCompilerType().GetTypeInfo() &
        lldb::eTypeIsPack);
 
   // If we're not binding the generic types, we need to set the self type as an
   // opaque pointer type. This is necessary because we don't bind the generic
   // parameters, and we can't have a type with unbound generics in a non-generic
   // function.
-  if (bind_generic_types == lldb::eDontBind && is_self)
+  if (should_not_bind_generic_types && is_self)
     target_type = ast_context.GetBuiltinRawPointerType();
   else if (is_unbound_pack)
     target_type = variable_sp->GetType()->GetForwardCompilerType();
@@ -623,7 +625,7 @@ SwiftUserExpression::GetTextAndSetExpressionParser(
         lldb::eSeverityError,
         "Couldn't realize Swift AST type of self. Hint: using `v` to "
         "directly inspect variables and fields may still work.");
-    return ParseResult::retry_no_bind_generic_params;
+    return ParseResult::retry_bind_generic_params;
   }
 
   auto ts = m_swift_ast_ctx->GetTypeSystemSwiftTypeRef();
@@ -636,14 +638,15 @@ SwiftUserExpression::GetTextAndSetExpressionParser(
         func_name.GetStringRef(), *ts);
   }
 
-  if (m_options.GetBindGenericTypes() == lldb::eDontBind &&
+  if (!SwiftASTManipulator::ShouldBindGenericTypes(
+          m_options.GetBindGenericTypes()) &&
       !CanEvaluateExpressionWithoutBindingGenericParams(
           local_variables, m_generic_signature, *m_swift_ast_ctx, sc.block,
           *stack_frame.get())) {
     diagnostic_manager.PutString(
         lldb::eSeverityError,
         "Could not evaluate the expression without binding generic types.");
-    return ParseResult::unrecoverable_error;
+    return ParseResult::retry_bind_generic_params_not_supported;
   }
 
   uint32_t first_body_line = 0;
@@ -807,44 +810,56 @@ bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
   DiagnosticManager second_try_diagnostic_manager;
 
   bool retry = false;
+  bool first_expression_not_supported = false;
   while (true) {
     SwiftExpressionParser::ParseResult parse_result;
     if (!retry) {
       parse_result = GetTextAndSetExpressionParser(
           first_try_diagnostic_manager, source_code, exe_ctx, exe_scope);
-      if (parse_result != SwiftExpressionParser::ParseResult::
-                              retry_no_bind_generic_params ||
+      if ((parse_result !=
+               SwiftExpressionParser::ParseResult::retry_bind_generic_params &&
+           parse_result != SwiftExpressionParser::ParseResult::
+                               retry_bind_generic_params_not_supported) ||
           m_options.GetBindGenericTypes() != lldb::eBindAuto)
         // If we're not retrying, just copy the diagnostics over.
         diagnostic_manager.Consume(std::move(first_try_diagnostic_manager));
     } else {
       parse_result = GetTextAndSetExpressionParser(
           second_try_diagnostic_manager, source_code, exe_ctx, exe_scope);
-      if (parse_result == SwiftExpressionParser::ParseResult::success)
-        // If we succeeded the second time around, copy any diagnostics we
-        // produced in the success case over, and ignore the first attempt's
-        // failures.
+
         diagnostic_manager.Consume(std::move(second_try_diagnostic_manager));
-      else
-        // If we failed though, copy the diagnostics of the first attempt, and
-        // silently ignore any errors produced by the retry, as the retry was
-        // not what the user asked, and any diagnostics produced by it will
-        // most likely confuse the user.
-        diagnostic_manager.Consume(std::move(first_try_diagnostic_manager));
+        if (parse_result == SwiftExpressionParser::ParseResult::success ||
+            first_expression_not_supported)
+          // If we succeeded the second time around, copy any diagnostics we
+          // produced in the success case over, and ignore the first attempt's
+          // failures.
+          // If the generic expression evaluator failed because it doesn't
+          // support the expression the error message won't be very useful, so
+          // print the regular expression evaluator's error instead.
+          diagnostic_manager.Consume(std::move(second_try_diagnostic_manager));
+        else
+          // If we failed though, copy the diagnostics of the first attempt, and
+          // silently ignore any errors produced by the retry, as the retry was
+          // not what the user asked, and any diagnostics produced by it will
+          // most likely confuse the user.
+          diagnostic_manager.Consume(std::move(first_try_diagnostic_manager));
     }
 
     if (parse_result == SwiftExpressionParser::ParseResult::success)
       break;
 
     switch (parse_result) {
-    case ParseResult::retry_no_bind_generic_params:
+    case ParseResult::retry_bind_generic_params_not_supported:
+      first_expression_not_supported = true;
+      LLVM_FALLTHROUGH;
+    case ParseResult::retry_bind_generic_params:
       // Retry running the expression without binding the generic types if
       // BindGenericTypes was in the auto setting, give up otherwise.
       if (m_options.GetBindGenericTypes() != lldb::eBindAuto) 
         return false;
-      // Retry without binding generic parameters, this is the only
+      // Retry binding generic parameters, this is the only
       // case that will loop.
-      m_options.SetBindGenericTypes(lldb::eDontBind);
+      m_options.SetBindGenericTypes(lldb::eBind);
       retry = true;
       break;