[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

[Dan Liew via Phabricator via lldb-commits]

delcypher added inline comments.


================
Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:66
+        if variable_watchpoint:
+            # FIXME: There should probably be an API to create a variable watchpoint
+            self.runCmd('watchpoint set variable -w read_write -- global')
----------------
mib wrote:
> +1, please file an enhancement request :) 
Will do. TBH I'm actually surprised that calling `SBValue::Watch()` doesn't create a variable watchpoint.


================
Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:68
+            self.runCmd('watchpoint set variable -w read_write -- global')
+            watchpoint = target.GetWatchpointAtIndex(0)
+            self.assertTrue(watchpoint.IsWatchVariable())
----------------
mib wrote:
> `runCmd` doesn't check if the command succeeded, so you probably want to add an asset to check the number of watchpoint on the target is not zero.
@mib Did I misunderstand something? `runCmd` has this in the end of its implementation.

```lang=python
        if check:
            output = ""
            if self.res.GetOutput():
                output += "\nCommand output:\n" + self.res.GetOutput()
            if self.res.GetError():
                output += "\nError output:\n" + self.res.GetError()
            if msg:
                msg += output
            if cmd:
                cmd += output
            self.assertTrue(self.res.Succeeded(),
                            msg if (msg) else CMD_MSG(cmd))
```

and `check` is set to `True` by default.

So it looks like `runCmd` does check that the command succeeded.


================
Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:83
+            self.assertFalse(watchpoint.IsWatchVariable())
+            self.assertEqual(watchpoint.GetWatchSpec(), '')
+
----------------
mib wrote:
> mib wrote:
> > unrelated to this patch, there is seems we don't initialize the watch spec for expression watchpoint, we should probably file a bug report for that.
> it seems that *
Yes. I plan to file a report about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144937