I do prefer a test. 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. 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. 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. 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. Thanks, David C. On Tue, Oct 27, 2015 at 8:11 AM, Luc Hermitte <[email protected]> wrote: > Hi, > > Le 26/10/2015 16:41, David Cole via Insight-developers a écrit : >> I would really really really like to see some evidence from you that >> adding nullptr checks is materially harmful to performance in some >> real world fill/copy scenario before agreeing to any patch which does >> not include nullptr checks. > > Here are some micro-benchmarks (done with google-benchmark framework): > > https://gist.github.com/LucHermitte/7f1b8d5dd61c861e6ef9 > > Adding the test around std::copy in the assignment operator doesn't > change anything. Nor the manual implementation. > (These are not real world scenarios, however the result would be the > same : no observable difference). > > I still find that adding a redundant test, just to workaround the > implementation of a function in Debug mode and with a single family of > compiler, is a clumsy solution. > > What the manual implementation does is as good as the defensive one. > However, we could loose potential optimizations (like memcpy being used > with POD types) when the size of the vector is big enough if the STL > implementation tries to do it. > > BTW, memcpy(0,0,x) is officially an UB. And the more I read, the less > I'm convinced that func(nullptr, null_size) is officially defined > behaviour. > > So, if you prefer a test, let's go for a test -- which should be > required only in the assignment operator. > > Regards, > > -- > 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
