search

opencv / opencv

Open Source Computer Vision Library
https://opencv.org
Apache License 2.0
79.15k stars 55.83k forks source link

change condition of CV_DbgAssert from _DEBUG to NDEBUG #26151

Open Ginkgo-Biloba opened 2 months ago

Ginkgo-Biloba commented 2 months ago

--- to make it's precondition same as CV_DbgCheckXX. Or further, replace all #if defined _DEBUG with #if !defined NDEBUG.

The C++ standard only mentions NDEBUG as the condition of assert macro, no _DEBUG.

https://github.com/opencv/opencv/blob/e1fec1562714eb41ae48f97310b254618a26d248/modules/core/include/opencv2/core/base.hpp#L389-L394

https://github.com/opencv/opencv/blob/e1fec1562714eb41ae48f97310b254618a26d248/modules/core/include/opencv2/core/check.hpp#L153-L169

asmorkalov commented 2 months ago

@vpisarev @opencv-alalek @mshabunin what do you think?

mshabunin commented 2 months ago
Quick grep has shown that we have following picture: Macro opencv (3rdparty) opencv_contrib
_DEBUG 47 (8) 2
NDEBUG 288 (103) 2

_DEBUG is MSVC thing and is explicitly defined for GCC/Clang in our CMake scripts. NDEBUG is added to CMAKE_..._FLAGS_RELEASE (_RELWITHDEBINFO, _MINSIZEREL) by CMake

So, I agree, we can switch to using NDEBUG in all our code, or even invent our own CV_DEBUG/CV_NDEBUG macro for better control.

Ginkgo-Biloba commented 2 months ago

Vote for CV_DEBUG. By the way, we have also DEBUG (without prefix _) macro in some code.

mshabunin commented 1 month ago

We discussed this question a bit more and decided that it would be better to switch to NDEBUG and not introduce new macros.