Le 29/10/2015 13:42, David Cole a écrit : > Thanks for your work so far on all this.
You're welcome. > I really appreciate the effort, > and it has already paid off: the test passed yesterday and today on my > Debug VS dashboard. Wonderful! > I will give a try to your latest patch set with the added assert calls > later today. OK. Thank you. --Luc > > Thanks, > David > > > On Tuesday, October 27, 2015, Luc Hermitte <luc.hermi...@c-s.fr > <mailto:luc.hermi...@c-s.fr>> wrote: > > Le 27/10/2015 17:04, David Cole a écrit : > > I do prefer a test. > > The alternatives are not just test+std::copy VS no-test+std::copy. But > test and std::copy VS manual loop where the test is pointless as it's > already part of for() invariant. > > > This is C++. You should always check for nullptr > > if it is possible to get nullptr, and something bad could happen if > > you don't check. > > It's the "something bad could happen if you don't check" that makes all > the difference here and that explain my reluctance. > > i.e. nothing bad should happen with usual/naive implementations of > std::copy with std::copy(p,p, nullptr) -- if memcpy is triggered, I must > admit my ignorance, and as others have said, it would be very > surprising: > > http://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-perform-memcpy0-0-0 > > [On a side note, it's up to the client code to check for nullity if > nullity is rejected by the contract. Within the service code, we could > wait for C++17, or more likely use assertions.] > > > If you could prove to me that there is never a > > nullptr in this context, I would not care. But there is. The failing > > test proves it. > > > > Rather than thinking of this as "just to workaround the implementation > > of a function in Debug mode and with a single family of compiler," > > perhaps you should instead think of it as defensive programming. > > That's my problem. I see it exactly as defensive programming. Unlike > (narrow) Design By Contract, the (wide) defensive programming approach > adds tests that make no sense (in context were the contract is > respected), and complicate the nominal code. > > > > The fact is, these debug checks from Microsoft have helped and > > assisted me over the years FAR MORE than they have hurt. They help to > > detect problems early on, as soon as incorrect code is introduced and > > run. Yes, they're painful, and yes, sometimes slightly too aggressive. > > But they're worth it. > > I completely agree they are worth it. This time, the test (on copy -- as > I've workaround the one on fill) is as you said : too aggressive. > > > > Let me know if you would like me to build/run your patch set after you > > think you have it to the point where the Debug MS build should run > > without raising these assert dialogs. > > Yes please, I'd like to. > > Here is a patched (I hope -- memset and memcpy may trigger assertions, I > don't know) version of the code. > > http://review.source.kitware.com/#/c/20313/ (I didn't know if, and how, > I could take over the previous patch set on the subject. Sorry for the > inconvenience) > > On the way, I've added missing documentation regarding invariants, > preconditions and postconditions, plus the related assertions. This is a > first draft. It's likely to contains English errors. I hope, however I > did not assert incorrect contracts in the documentation. I'll read it > again more carefully later. > > I've also added code to test operations on empty VLV. > > > Thank you. > > -- > Luc Hermitte > _______________________________________________ Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Kitware offers ITK Training Courses, for more information visit: http://kitware.com/products/protraining.php Please keep messages on-topic and check the ITK FAQ at: http://www.itk.org/Wiki/ITK_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/insight-developers
