On Thu, 2015-11-19 at 19:44 +0100, Bernd Schmidt wrote: > On 11/19/2015 07:08 PM, David Malcolm wrote: > > gcc_assert terminates the process and no further testing is done, > > whereas the approach the kit tries to run as much of the testsuite as > > possible, and then fail if any errors occurred. > > Yeah, but let's say someone is working on bitmaps and one of the bitmap > tests fail, it's somewhat unlikely that cfg will also fail (or if it > does, it'll be a consequence of the earlier failure). You debug the > issue, fix it, and run cc1 -fself-test again to see if that sorted it out. > > As I said, it's a matter of taste and style and I won't claim that my > way is necessarily the right one, but I do want to see if others feel > the same. > > > The patch kit does use a lot of "magic" via macros and C++. > > > > Taking registration/discovery/running in completely the other direction, > > another approach could be a completely manual approach, with something > > like this in toplev.c: > > > > bitmap_selftest (); > > et_forest_selftest (); > > /* etc */ > > vec_selftest (); > > > > This has the advantage of being explicit, and the disadvantage of > > requiring a bit more typing. > > (possibly passing in a "selftest *" param if we're doing the > > try-to-run-everything approach, so we can count failures etc without > > introducing globals) > > > > Was that what you had in mind, or something else? > > It's one option, but it doesn't seem like the best one either. I was > thinking of something not dissimilar to your approach, but with fewer > bells and whistles. My class registrator would look something like this: > > static list<void (*)()> test_callbacks; > > class registrator > { > public: > registrator (void (*)() cb) > { > test_callbacks.push_front (cb); > } > } > > (or use a vec if you can do that at global constructor time) > > and then you just walk the list and run the callbacks when you want to > run tests. The one you have implements both the registration and a > special case linked list, which just doesn't look right to me,
FWIW, the reason I special-cased the linked list was to avoid any dynamic memory allocation: the ctors run before main, so I wanted to keep them as simple as possible. Putting the linked list directly into those objects means that running the ctors is a simple case of wiring up some pointers: the memory is already statically allocated. (also, one thing I want to test is vec<> itself [1]). Anything more complicated feels to me like asking for trouble - and as noted, I already have some bugs to track down with the linker apparently optimizing away some tests. (Maybe auto-registration is more trouble than it's worth?) > and I think I'd also have avoided the runner class. The runner class was mostly about accumulating pass/fail information, which goes back to the abort-on-first-failure vs try-to-run-everything question. If we do want the latter, it could be global state if that would be simpler (though I prefer to avoid global state). Dave [1] although to be fair, I used vec<> in one place within the patch, when sorting the tests.