labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Thanks for adding the test. I have some nits about some details of the test. If
you agree with them, you can commit the updated version without additional
review.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:2
@@ +1,3 @@
+"""
+Test watchpoint size cases (1-byte, 2-byte, 4-byte or 8-byte).
+"""
----------------
Add: "Also make sure that the watchpoints work when the variables are not
placed at 8-byte aligned addresses." or something to that effect. Otherwise,
someone in the future will wonder why the test is so complicated.
Also, you don't seem to test 8-byte watchpoints (which is fine, but the comment
is not correct then).
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:15
@@ +14,3 @@
+
+ mydir = TestBase.compute_mydir(__file__)
+
----------------
How about adding NO_DEBUG_INFO_TESTCASE = True here? We are not testing debug
info, so it should be enough to run the test once.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:49
@@ +48,3 @@
+
+ def run_watchpoint_size_test(self, arrayName, rangelimit, wsize):
+ self.build(dictionary=self.d)
----------------
this would a lot more natural if you renamed this into `array_size` or
something and used 0-based indexes.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:66
@@ +65,3 @@
+
+ for i in range(1,rangelimit):
+ # We should be stopped again due to the breakpoint.
----------------
`range(array_size)`
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:73
@@ +72,3 @@
+ # Set a read_write type watchpoint arrayName
+ watch_loc=arrayName+"[" + str(i-1) + "]"
+ self.expect("watchpoint set variable -w read_write " + watch_loc,
----------------
`str(i)`
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py:104
@@ +103,3 @@
+ self.expect("watchpoint list -v",
+ substrs = ['hit_count = 2'])
+
----------------
I guess the intention here is to test both read and write watchpoints. If so,
could you state that somewhere (in a comment at least, if it's possible to
somehow verify the watchpoint type in code, even better).
================
Comment at:
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/main.c:46
@@ +45,3 @@
+ pad1++;
+ localWord = wordArray[i]; // Here onwards we should'nt be stopped in
loop
+ wordArray[i]++;
----------------
shouldn't (typo)
http://reviews.llvm.org/D21280
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
