Merge pull request #10005 from augusto2112/swap-order-expr-eval

[lldb] Run generic expression evaluator first

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;