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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to