Message ID | 1447952699-40820-3-git-send-email-dmalcolm@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Nov 19, 2015 at 6:04 PM, David Malcolm <dmalcolm@redhat.com> wrote: > Jeff pre-approved the plugin version of this (as a new > file unittests/test-bitmap.c): > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03284.html > with: >> OK if/when prereqs are approved. Minor twiddling if we end up moving it >> elsewhere or standardizing/reducing header files is pre-approved. > > This version moves them to bitmap.c > > One issue: how to express: > TEST (bitmap_test, gc_alloc) > in a ChangeLog entry. > > I've chosen to write it as (bitmap_test, gc_alloc) since that > has the greatest chance of being found via grep. I think overloading CHECKING_P for this is bogus. Adding a new UNIT_TEST_FILE (like GENERATOR_FILE) would be better. Also selftest.h should be only conditionally included (before the tests themselves?) Richard. > gcc/ChangeLog: > * bitmap.c: Include "selftest.h". > (bitmap_test, gc_alloc): New selftest. > (bitmap_test, set_range): New selftest. > (bitmap_test, clear_bit_in_middle): New selftest. > (bitmap_test, copying): New selftest. > (bitmap_test, bitmap_single_bit_set_p): New selftest. > --- > gcc/bitmap.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/gcc/bitmap.c b/gcc/bitmap.c > index f04b8f9..e6f772e 100644 > --- a/gcc/bitmap.c > +++ b/gcc/bitmap.c > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see > #include "system.h" > #include "coretypes.h" > #include "bitmap.h" > +#include "selftest.h" > > /* Memory allocation statistics purpose instance. */ > mem_alloc_description<bitmap_usage> bitmap_mem_desc; > @@ -2094,5 +2095,96 @@ debug (const bitmap_head *ptr) > fprintf (stderr, "<nil>\n"); > } > > +#if CHECKING_P > +namespace { > + > +/* Freshly-created bitmaps ought to be empty. */ > + > +TEST (bitmap_test, gc_alloc) > +{ > + bitmap b = bitmap_gc_alloc (); > + EXPECT_TRUE (bitmap_empty_p (b)); > +} > + > +/* Verify bitmap_set_range. */ > + > +TEST (bitmap_test, set_range) > +{ > + bitmap b = bitmap_gc_alloc (); > + EXPECT_TRUE (bitmap_empty_p (b)); > + > + bitmap_set_range (b, 7, 5); > + EXPECT_FALSE (bitmap_empty_p (b)); > + EXPECT_EQ (5, bitmap_count_bits (b)); > + > + /* Verify bitmap_bit_p at the boundaries. */ > + EXPECT_FALSE (bitmap_bit_p (b, 6)); > + EXPECT_TRUE (bitmap_bit_p (b, 7)); > + EXPECT_TRUE (bitmap_bit_p (b, 11)); > + EXPECT_FALSE (bitmap_bit_p (b, 12)); > +} > + > +/* Verify splitting a range into two pieces using bitmap_clear_bit. */ > + > +TEST (bitmap_test, clear_bit_in_middle) > +{ > + bitmap b = bitmap_gc_alloc (); > + > + /* Set b to [100..200]. */ > + bitmap_set_range (b, 100, 100); > + EXPECT_EQ (100, bitmap_count_bits (b)); > + > + /* Clear a bit in the middle. */ > + bool changed = bitmap_clear_bit (b, 150); > + EXPECT_TRUE (changed); > + EXPECT_EQ (99, bitmap_count_bits (b)); > + EXPECT_TRUE (bitmap_bit_p (b, 149)); > + EXPECT_FALSE (bitmap_bit_p (b, 150)); > + EXPECT_TRUE (bitmap_bit_p (b, 151)); > +} > + > +/* Verify bitmap_copy. */ > + > +TEST (bitmap_test, copying) > +{ > + bitmap src = bitmap_gc_alloc (); > + bitmap_set_range (src, 40, 10); > + > + bitmap dst = bitmap_gc_alloc (); > + EXPECT_FALSE (bitmap_equal_p (src, dst)); > + bitmap_copy (dst, src); > + EXPECT_TRUE (bitmap_equal_p (src, dst)); > + > + /* Verify that we can make them unequal again... */ > + bitmap_set_range (src, 70, 5); > + EXPECT_FALSE (bitmap_equal_p (src, dst)); > + > + /* ...and that changing src after the copy didn't affect > + the other: */ > + EXPECT_FALSE (bitmap_bit_p (dst, 70)); > +} > + > +/* Verify bitmap_single_bit_set_p. */ > +TEST (bitmap_test, bitmap_single_bit_set_p) > +{ > + bitmap b = bitmap_gc_alloc (); > + > + EXPECT_FALSE (bitmap_single_bit_set_p (b)); > + > + bitmap_set_range (b, 42, 1); > + EXPECT_TRUE (bitmap_single_bit_set_p (b)); > + EXPECT_EQ (42, bitmap_first_set_bit (b)); > + > + bitmap_set_range (b, 1066, 1); > + EXPECT_FALSE (bitmap_single_bit_set_p (b)); > + EXPECT_EQ (42, bitmap_first_set_bit (b)); > + > + bitmap_clear_range (b, 0, 100); > + EXPECT_TRUE (bitmap_single_bit_set_p (b)); > + EXPECT_EQ (1066, bitmap_first_set_bit (b)); > +} > + > +} // anon namespace > +#endif /* CHECKING_P */ > > #include "gt-bitmap.h" > -- > 1.8.5.3 >
On 11/19/2015 10:04 AM, David Malcolm wrote: > Jeff pre-approved the plugin version of this (as a new > file unittests/test-bitmap.c): > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03284.html > with: >> OK if/when prereqs are approved. Minor twiddling if we end up moving it >> elsewhere or standardizing/reducing header files is pre-approved. > > This version moves them to bitmap.c > > One issue: how to express: > TEST (bitmap_test, gc_alloc) > in a ChangeLog entry. > > I've chosen to write it as (bitmap_test, gc_alloc) since that > has the greatest chance of being found via grep. Seems reasonable. I'm not going to go through the individual tests closely this round. I think the big question is whether or not we're getting close to an agreement on things like all tests vs stop at first failure, registration and pollution of state from one test to the next. So what I like here is we've got a single #include, which probably should, by convention, be last in the set of headers. Then at the end of the file we have the tests, guarded by a single #if CHECKING_P. Each test uses the TEST macro, which is probably fine since it gives a way to hook up registration and the like. EXPECT_XXX usage (or non-usage) will follow from the test everything vs stop at first failure decision. If we adopt test everything, then indirection of some sort is useful here too as we can log results. I'm curious what the namespace is for. It's probably fine, just curious more than anything. Jeff
diff --git a/gcc/bitmap.c b/gcc/bitmap.c index f04b8f9..e6f772e 100644 --- a/gcc/bitmap.c +++ b/gcc/bitmap.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "bitmap.h" +#include "selftest.h" /* Memory allocation statistics purpose instance. */ mem_alloc_description<bitmap_usage> bitmap_mem_desc; @@ -2094,5 +2095,96 @@ debug (const bitmap_head *ptr) fprintf (stderr, "<nil>\n"); } +#if CHECKING_P +namespace { + +/* Freshly-created bitmaps ought to be empty. */ + +TEST (bitmap_test, gc_alloc) +{ + bitmap b = bitmap_gc_alloc (); + EXPECT_TRUE (bitmap_empty_p (b)); +} + +/* Verify bitmap_set_range. */ + +TEST (bitmap_test, set_range) +{ + bitmap b = bitmap_gc_alloc (); + EXPECT_TRUE (bitmap_empty_p (b)); + + bitmap_set_range (b, 7, 5); + EXPECT_FALSE (bitmap_empty_p (b)); + EXPECT_EQ (5, bitmap_count_bits (b)); + + /* Verify bitmap_bit_p at the boundaries. */ + EXPECT_FALSE (bitmap_bit_p (b, 6)); + EXPECT_TRUE (bitmap_bit_p (b, 7)); + EXPECT_TRUE (bitmap_bit_p (b, 11)); + EXPECT_FALSE (bitmap_bit_p (b, 12)); +} + +/* Verify splitting a range into two pieces using bitmap_clear_bit. */ + +TEST (bitmap_test, clear_bit_in_middle) +{ + bitmap b = bitmap_gc_alloc (); + + /* Set b to [100..200]. */ + bitmap_set_range (b, 100, 100); + EXPECT_EQ (100, bitmap_count_bits (b)); + + /* Clear a bit in the middle. */ + bool changed = bitmap_clear_bit (b, 150); + EXPECT_TRUE (changed); + EXPECT_EQ (99, bitmap_count_bits (b)); + EXPECT_TRUE (bitmap_bit_p (b, 149)); + EXPECT_FALSE (bitmap_bit_p (b, 150)); + EXPECT_TRUE (bitmap_bit_p (b, 151)); +} + +/* Verify bitmap_copy. */ + +TEST (bitmap_test, copying) +{ + bitmap src = bitmap_gc_alloc (); + bitmap_set_range (src, 40, 10); + + bitmap dst = bitmap_gc_alloc (); + EXPECT_FALSE (bitmap_equal_p (src, dst)); + bitmap_copy (dst, src); + EXPECT_TRUE (bitmap_equal_p (src, dst)); + + /* Verify that we can make them unequal again... */ + bitmap_set_range (src, 70, 5); + EXPECT_FALSE (bitmap_equal_p (src, dst)); + + /* ...and that changing src after the copy didn't affect + the other: */ + EXPECT_FALSE (bitmap_bit_p (dst, 70)); +} + +/* Verify bitmap_single_bit_set_p. */ +TEST (bitmap_test, bitmap_single_bit_set_p) +{ + bitmap b = bitmap_gc_alloc (); + + EXPECT_FALSE (bitmap_single_bit_set_p (b)); + + bitmap_set_range (b, 42, 1); + EXPECT_TRUE (bitmap_single_bit_set_p (b)); + EXPECT_EQ (42, bitmap_first_set_bit (b)); + + bitmap_set_range (b, 1066, 1); + EXPECT_FALSE (bitmap_single_bit_set_p (b)); + EXPECT_EQ (42, bitmap_first_set_bit (b)); + + bitmap_clear_range (b, 0, 100); + EXPECT_TRUE (bitmap_single_bit_set_p (b)); + EXPECT_EQ (1066, bitmap_first_set_bit (b)); +} + +} // anon namespace +#endif /* CHECKING_P */ #include "gt-bitmap.h"