[Lldb-commits] [lldb] [lldb-dap] Adjusting how repl-mode auto determines commands vs variable expressions. (PR #78005)

[Walter Erquinigo via lldb-commits]

================
@@ -395,43 +402,56 @@ ExpressionContext DAP::DetectExpressionContext(lldb::SBFrame &frame,
   case ReplMode::Command:
     return ExpressionContext::Command;
   case ReplMode::Auto:
-    // If the frame is invalid then there is no variables to complete, assume
-    // this is an lldb command instead.
-    if (!frame.IsValid()) {
-      return ExpressionContext::Command;
-    }
-
+    // To determine if the expression is a command or not, check if the first
+    // term is a variable or command. If its a variable in scope we will prefer
+    // that behavior and give a warning to the user if they meant to invoke the
+    // operation as a command.
+    //
+    // Example use case:
+    //   int p and expression "p + 1" > variable
+    //   int i and expression "i" > variable
+    //   int var and expression "va" > command
+    const auto term = FirstTerm(expression);
+    auto ci = debugger.GetCommandInterpreter();
     lldb::SBCommandReturnObject result;
-    debugger.GetCommandInterpreter().ResolveCommand(text.data(), result);
-
-    // If this command is a simple expression like `var + 1` check if there is
-    // a local variable name that is in the current expression. If so, ensure
-    // the expression runs in the variable context.
-    lldb::SBValueList variables = frame.GetVariables(true, true, true, true);
-    llvm::StringRef input = text;
-    for (uint32_t i = 0; i < variables.GetSize(); i++) {
-      llvm::StringRef name = variables.GetValueAtIndex(i).GetName();
-      // Check both directions in case the input is a partial of a variable
-      // (e.g. input = `va` and local variable = `var1`).
-      if (input.contains(name) || name.contains(input)) {
-        if (!auto_repl_mode_collision_warning) {
-          llvm::errs() << "Variable expression '" << text
-                       << "' is hiding an lldb command, prefix an expression "
-                          "with '"
-                       << g_dap.command_escape_prefix
-                       << "' to ensure it runs as a lldb command.\n";
-          auto_repl_mode_collision_warning = true;
-        }
-        return ExpressionContext::Variable;
+    ci.ResolveCommand(term.c_str(), result);
+    bool term_is_command = result.Succeeded();
+    bool term_is_variable = frame.FindVariable(term.c_str()).IsValid();
+
+    // If we have both a variable and command, warn the user about the conflict.
+    if (term_is_command && term_is_variable) {
+      llvm::errs()
+          << "warning: Expression '" << term
+          << "' is both an lldb command and variable, evaluating the "
+             "expression as a variable. To evaluate the lldb command, use '"
+          << g_dap.command_escape_prefix << "' as a prefix.\n";
+      if (!repl_mode_behavior_description_shown) {
+        repl_mode_behavior_description_shown = true;
+        llvm::errs()
+            << "\nnote: This error message is only displayed once. Use '"
+            << g_dap.command_escape_prefix
+            << "' as a prefix ensure expressions are evaluated as lldb "
+               "commands.\n\nTo change the repl behavior use:\n'"
+            << g_dap.command_escape_prefix
+            << "lldb-dap repl-mode variable' to evaluate all expressions "
+               "as variables.\n'"
+            << g_dap.command_escape_prefix
+            << "lldb-dap repl-mode command' to evaluate all expressions as "
+               "commands.\n'"
+            << g_dap.command_escape_prefix
+            << "lldb-dap repl-mode auto' to use a heuristic to detect the "
+               "mode based on the input and local variables.\n";
----------------
walter-erquinigo wrote:

I'd rather not show this, because:

- if the user configured lldb-dap on their own, they'll know what the auto mode does and they can troubleshoot themselves
- if the user doesn't configure lldb-dap and they get it from someone else, then they can't simply troubleshoot themselves, so this message might be confusing.

https://github.com/llvm/llvm-project/pull/78005