Although everything compiles, MSVC spits out a bunch of warnings about
the MKSUnit stuff
I looked this morning and, as far as I could tell, it was warning about
using a standard feature of C++ that I was intentionally using. MSVC has
ways to turning off specific warnings, so I may go and turn the warnings
off for those calls.
Good; that was my understanding too. It's unfortunate that MSVC sucks.
The patch is a bit hazy on what NDEBUG should do (reflecting my
haziness). In general, NDEBUG should do or not do whatever we want it to
do (and turning it on shouldn't make code slower). Other than that, I
wouldn't worry too much about user expectations, especially since we
don't allow the users to control it directly.
The only thing we can say for sure is that when NDEBUG is defined,
assert() is a noop. IMHO, IMP_assert() acts like assert(), so it should
do the same thing - that's how I put it in the patch I committed. For
the other runtime checks, it seems reasonable for it to be like CHEAP or
NONE, as you say.
From what I understand, the tests were exposing bugs in IMP. The
patches probably should have fixed the bugs and in case we should fix
the bugs in IMP rather than just disable the tests. Or am I confused.
You are probably confused because it did both. ;) The patch did fix the
bug in IMP (use of invalidated iterators). It also disabled the particle
deletion part of the tests, because IMP was touching freed particle
pointers in that case. Arguably that's also a bug in IMP, but it was my
understanding that we can't "fix" that without putting in particle
reference counting, which I didn't have the time to do.