Archived issue #0022921
Adding macros for convenient inclusion of run-time asserts
Description
By the moment OCCT code does not provide convenient means to include run-time asserts in the code. The typical method to check for validity of some condition at run-time is either (a) silent check, or (b) check with exception raising in Debug mode / no check in Release mode (implemented by macros in exception classes, *_Raise_if).
The problem with method (a) is that programmer has no direct feedback if assert fails; method (b) usually is activated in Debug mode only (thus Release mode behaves differently) and is limited to raising exception in case if assert failed.
Note that in a few places C function assert() is called; these places must be cleared since this function leads to termination of the program if assert is failed, and it is not what the library like OCCT shall do.
It is proposed to provide macros that would allow convenient insertion of asserts in the code with the following features:
1. Same behavior in Debug and Release mode
2. In Debug mode, notification to developer if assert fails (on MSVC, prompt to attach debugger)
3. Single statement to make a check and specify action if assert failed
The problem with method (a) is that programmer has no direct feedback if assert fails; method (b) usually is activated in Debug mode only (thus Release mode behaves differently) and is limited to raising exception in case if assert failed.
Note that in a few places C function assert() is called; these places must be cleared since this function leads to termination of the program if assert is failed, and it is not what the library like OCCT shall do.
It is proposed to provide macros that would allow convenient insertion of asserts in the code with the following features:
1. Same behavior in Debug and Release mode
2. In Debug mode, notification to developer if assert fails (on MSVC, prompt to attach debugger)
3. Single statement to make a check and specify action if assert failed
Public activity
9 archived notes
Participants are labeled by their role within this record.
The code is integrated to branch OCC22921, please review
Standard_Assert.hxx,
general suggestion - file will be more readable if preprocessor statements if/else/endif will be indented.
Standard_ASSERT_ALWAYS_FALSE looks ugly. Are these do/while constructions really needed?
Standard_Assert.hxx, 53:
> #ifndef _DEBUG
> #define _DEBUG
> #endif
Macros _DEBUG should NOT be overridden here because it is automatically defined by VS compiler with options /LDd, /MDd, or /MTd and indicates linkage with debug version of C/C++ runtime libraries. This is important because _CrtDbgReport() exported only in debug version of runtimes!
Standard_Assert.hxx, 57:
> #ifdef WNT
> #include <crtdbg.h>
This is incorrect test - crtdbg.h provided only with VS compiler so _MSC_VER should be checked instead.
Standard_Assert.hxx, 66:
> #define Standard_ASSERT_INVOKE_(expr,mess) \
> { fprintf ( stderr, "*** ERROR: LH3D ASSERT in file %s, line %d:\n"\
> "%s\n(Condition: \"%s\")\n", \
> (const char*)__FILE__, (int)__LINE__, mess, #expr); \
> fflush (stdout); }
I think LH3D is not proper identifier for OCCT-based application.
Also what the reason for this (const char*)__FILE__ explicit cast? This is safer to remove it.
Standard_Assert.hxx, 99:
> #define Standard_ASSERT_RAISE(expr,mess) \
> Standard_ASSERT(expr,mess,Standard_ProgramError::Raise( \
> "*** ERROR: ASSERT in file " __FILE__ ": \n" mess " (" #expr ")" ) )
This is preferred to escape __FILE__ with quotes to improve message readability (especially in case of full path with spaces). The same for Standard_ASSERT_INVOKE_ macros.
QAUsinor.cxx, 269
> Standard_ASSERT_EXCEPT(vw2.IsSame(ve1) || vw2.IsSame(ve2), "Disconnected vertices");
Here and in other places - Standard_ASSERT_EXCEPT is undefined!
general suggestion - file will be more readable if preprocessor statements if/else/endif will be indented.
Standard_ASSERT_ALWAYS_FALSE looks ugly. Are these do/while constructions really needed?
Standard_Assert.hxx, 53:
> #ifndef _DEBUG
> #define _DEBUG
> #endif
Macros _DEBUG should NOT be overridden here because it is automatically defined by VS compiler with options /LDd, /MDd, or /MTd and indicates linkage with debug version of C/C++ runtime libraries. This is important because _CrtDbgReport() exported only in debug version of runtimes!
Standard_Assert.hxx, 57:
> #ifdef WNT
> #include <crtdbg.h>
This is incorrect test - crtdbg.h provided only with VS compiler so _MSC_VER should be checked instead.
Standard_Assert.hxx, 66:
> #define Standard_ASSERT_INVOKE_(expr,mess) \
> { fprintf ( stderr, "*** ERROR: LH3D ASSERT in file %s, line %d:\n"\
> "%s\n(Condition: \"%s\")\n", \
> (const char*)__FILE__, (int)__LINE__, mess, #expr); \
> fflush (stdout); }
I think LH3D is not proper identifier for OCCT-based application.
Also what the reason for this (const char*)__FILE__ explicit cast? This is safer to remove it.
Standard_Assert.hxx, 99:
> #define Standard_ASSERT_RAISE(expr,mess) \
> Standard_ASSERT(expr,mess,Standard_ProgramError::Raise( \
> "*** ERROR: ASSERT in file " __FILE__ ": \n" mess " (" #expr ")" ) )
This is preferred to escape __FILE__ with quotes to improve message readability (especially in case of full path with spaces). The same for Standard_ASSERT_INVOKE_ macros.
QAUsinor.cxx, 269
> Standard_ASSERT_EXCEPT(vw2.IsSame(ve1) || vw2.IsSame(ve2), "Disconnected vertices");
Here and in other places - Standard_ASSERT_EXCEPT is undefined!
Most remarks were applied to bug branch (except Standard_ASSERT_ALWAYS_FALSE).
Notice that on POSIX systems raise(SIGTRAP) now called which better corresponds to original WNT code and expected assertions behaviour at all. SIGTRAP will add breakpoint if debugger is attached or abort program execution if not.
2abv, please look onto theses changes carefully.
Notice that on POSIX systems raise(SIGTRAP) now called which better corresponds to original WNT code and expected assertions behaviour at all. SIGTRAP will add breakpoint if debugger is attached or abort program execution if not.
2abv, please look onto theses changes carefully.
Great! I have fixed the last point: replaced do {} while(0) by if () {} else void_function() -- this removes limitation of impossibility to use break and continue from assert. Checked to work fine on WNT.
Patch looks fine now.
Dear Commenter 1, please perform the tests.
Dear Commenter 1, please perform the tests.
Error : Errors occured in Shell
/dn47/KAS/dev/aan-22921-occt/src/AIS2D/AIS2D_ProjShape.cxx: In member function 'void AIS2D_ProjShape::DrawCurves(const TopoDS_Shape&, Handle_GGraphic2d_SetOfCurves&)':
/dn47/KAS/dev/aan-22921-occt/src/AIS2D/AIS2D_ProjShape.cxx:298: error: no matching function for call to 'BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom_Surface&, Standard_Real&, Standard_Real&)'
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:123: note: candidates are: static Handle_Geom2d_Curve BRep_Tool::CurveOnSurface(const TopoDS_Edge&, const TopoDS_Face&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:128: note: static Handle_Geom2d_Curve BRep_Tool::CurveOnSurface(const TopoDS_Edge&, const Handle_Geom_Surface&, const TopLoc_Location&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:133: note: static void BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom2d_Curve&, Handle_Geom_Surface&, TopLoc_Location&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:138: note: static void BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom2d_Curve&, Handle_Geom_Surface&, TopLoc_Location&, Standard_Real&, Standard_Real&, Standard_Integer)
Error : Failed : AIS2D_ProjShape.cxx
Info : ----------------------- Compilation Report -----------------------
Info : Failed : :KAS:dev:aan-22921-occt:AIS2D:source:AIS2D_ProjShape.cxx
Info : -----------------------------------------------------------------
/dn47/KAS/dev/aan-22921-occt/src/AIS2D/AIS2D_ProjShape.cxx: In member function 'void AIS2D_ProjShape::DrawCurves(const TopoDS_Shape&, Handle_GGraphic2d_SetOfCurves&)':
/dn47/KAS/dev/aan-22921-occt/src/AIS2D/AIS2D_ProjShape.cxx:298: error: no matching function for call to 'BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom_Surface&, Standard_Real&, Standard_Real&)'
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:123: note: candidates are: static Handle_Geom2d_Curve BRep_Tool::CurveOnSurface(const TopoDS_Edge&, const TopoDS_Face&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:128: note: static Handle_Geom2d_Curve BRep_Tool::CurveOnSurface(const TopoDS_Edge&, const Handle_Geom_Surface&, const TopLoc_Location&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:133: note: static void BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom2d_Curve&, Handle_Geom_Surface&, TopLoc_Location&, Standard_Real&, Standard_Real&)
/dn47/KAS/dev/aan-22921-occt/inc/BRep_Tool.hxx:138: note: static void BRep_Tool::CurveOnSurface(const TopoDS_Edge&, Handle_Geom2d_Curve&, Handle_Geom_Surface&, TopLoc_Location&, Standard_Real&, Standard_Real&, Standard_Integer)
Error : Failed : AIS2D_ProjShape.cxx
Info : ----------------------- Compilation Report -----------------------
Info : Failed : :KAS:dev:aan-22921-occt:AIS2D:source:AIS2D_ProjShape.cxx
Info : -----------------------------------------------------------------
Fixed, please test
Dear Commenter 1,
Workbench KAS:dev:aan-22921-occt was created from SVN branch http://svn/svn/occt/branches/OCC22921 [^]
(and aan-22921-products from trunk) and compiled on Windows and Linux platforms.
There are not regressions in aan-22921-products regarding to KAS:dev:products-20120217-opt
See results in /QADisk/occttests/results/KAS/dev/ aan-22921-products_21022012/lin
See reference results in /QADisk/occttests/results/KAS/dev/products-20120217-opt_17022012/lin
See test cases in /QADisk/occttests/tests/ED
Workbench KAS:dev:aan-22921-occt was created from SVN branch http://svn/svn/occt/branches/OCC22921 [^]
(and aan-22921-products from trunk) and compiled on Windows and Linux platforms.
There are not regressions in aan-22921-products regarding to KAS:dev:products-20120217-opt
See results in /QADisk/occttests/results/KAS/dev/ aan-22921-products_21022012/lin
See reference results in /QADisk/occttests/results/KAS/dev/products-20120217-opt_17022012/lin
See test cases in /QADisk/occttests/tests/ED
Integrated into trunk of occt repository
Date: 2012-02-22 16:40:11 +0400 (Wed, 22 Feb 2012)
New Revision: 10596
Added:
trunk/src/Standard/Standard_Assert.hxx
Modified:
trunk/src/AIS2D/AIS2D_ProjShape.cxx
trunk/src/QANewDBRepNaming/QANewDBRepNaming_FeatureCommands.cxx
trunk/src/QANewDBRepNaming/QANewDBRepNaming_PrimitiveCommands.cxx
trunk/src/QAUsinor/QAUsinor.cxx
trunk/src/Standard/FILES
Date: 2012-02-22 16:40:11 +0400 (Wed, 22 Feb 2012)
New Revision: 10596
Added:
trunk/src/Standard/Standard_Assert.hxx
Modified:
trunk/src/AIS2D/AIS2D_ProjShape.cxx
trunk/src/QANewDBRepNaming/QANewDBRepNaming_FeatureCommands.cxx
trunk/src/QANewDBRepNaming/QANewDBRepNaming_PrimitiveCommands.cxx
trunk/src/QAUsinor/QAUsinor.cxx
trunk/src/Standard/FILES