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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits