[Lldb-commits] [PATCH] D144688: [lldb] Fix watchpoint command function stopping behavior

[Dan Liew via Phabricator via lldb-commits]

delcypher added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectWatchpointCommand.cpp:425
           else if (!m_options.m_function_name.empty()) {
-            std::string oneliner(m_options.m_function_name);
+            std::string oneliner("return ");
+            oneliner += m_options.m_function_name;
----------------
@mib There's a reason I didn't do it this way when I tried to fix this locally.

The python stub function we generate would look something like this with your patch.

```lang=python
def lldb_autogen_python_wp_callback_func__0 (frame, wp, internal_dict):
     global_dict = globals()
     new_keys = internal_dict.keys()
     old_keys = global_dict.keys()
     global_dict.update (internal_dict)
     if True:
         return call_user_function(frame, wp, internal_dict)
     for key in new_keys:
         internal_dict[key] = global_dict[key]
         if key not in old_keys:
            del global_dict[key]
```

with your early return the logic for updating `internal_dict` and `global_dict` will **not run**.  I'm not entirely sure what the purpose of this is but if its important then we need to implement this patch another way.

Here's another way we could do it. You could take the patch I originally wrote but change `ScriptInterpreterPythonImpl::GenerateFunction(...)` to take an additional parameter `bool ReturnValue` that is false by default. This parameter when `true` would do the work my patch did to make sure we use the return value from the last user statement evaluated. For the watchpoint evaluation we can pass a `true` value. For the other cases `ReturnValue` will be false so there will be no behavior change there.


================
Comment at: lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py:159
+
+        self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stop reason = breakpoint'])
----------------
This `self.expect()` fails when the bug hasn't been fixed, correct?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144688/new/

https://reviews.llvm.org/D144688