View Issue Details

IDProjectCategoryView StatusLast Update
0022921Open CASCADEOCCT:Foundation Classespublic2013-10-09 17:53
ReporterabvAssigned Tobugmaster  
PrioritynormalSeverityintegration request 
Status closedResolutionfixed 
Product Version6.5.2 
Target Version6.5.3Fixed in Version6.5.3 
Summary0022921: Adding macros for convenient inclusion of run-time asserts
DescriptionBy 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
TagsNo tags attached.
Test case numberTest case is not required

Activities

abv

2012-01-23 17:32

manager   ~0019242

The code is integrated to branch OCC22921, please review

kgv

2012-01-31 13:01

developer   ~0019320

Last edited: 2012-01-31 13:03

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!

kgv

2012-02-20 11:22

developer   ~0019657

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.

abv

2012-02-20 12:49

manager   ~0019660

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.

kgv

2012-02-20 12:54

developer   ~0019662

Patch looks fine now.
Dear Bugmaster, please perform the tests.

aan

2012-02-20 17:13

developer   ~0019681

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 : -----------------------------------------------------------------

abv

2012-02-21 08:32

manager   ~0019687

Fixed, please test

apn

2012-02-22 11:11

administrator   ~0019715

Dear BugMaster,
   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

bugmaster

2012-02-22 16:42

administrator   ~0019739

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

Related Changesets

occt: master 91a16bc7

2012-02-22 12:40:11

kgv


Committer: bugmaster Details Diff
0022921: Adding macros for convenient inclusion of run-time asserts Affected Issues
0022921
mod - src/AIS2D/AIS2D_ProjShape.cxx Diff File
mod - src/QANewDBRepNaming/QANewDBRepNaming_FeatureCommands.cxx Diff File
mod - src/QANewDBRepNaming/QANewDBRepNaming_PrimitiveCommands.cxx Diff File
mod - src/QAUsinor/QAUsinor.cxx Diff File
mod - src/Standard/FILES Diff File
add - src/Standard/Standard_Assert.hxx Diff File

Issue History

Date Modified Username Field Change
2012-01-23 12:52 abv New Issue
2012-01-23 12:52 abv Assigned To => abv
2012-01-23 17:32 abv Note Added: 0019242
2012-01-23 17:32 abv Assigned To abv => kgv
2012-01-23 17:32 abv Status new => resolved
2012-01-31 13:01 kgv Note Added: 0019320
2012-01-31 13:01 kgv Assigned To kgv => abv
2012-01-31 13:01 kgv Status resolved => assigned
2012-01-31 13:03 kgv Note Edited: 0019320
2012-01-31 13:03 kgv Note Edited: 0019320
2012-02-20 11:22 kgv Note Added: 0019657
2012-02-20 12:49 abv Note Added: 0019660
2012-02-20 12:49 abv Assigned To abv => kgv
2012-02-20 12:49 abv Status assigned => resolved
2012-02-20 12:54 kgv Note Added: 0019662
2012-02-20 12:54 kgv Assigned To kgv => bugmaster
2012-02-20 12:54 kgv Status resolved => reviewed
2012-02-20 13:10 mkv Assigned To bugmaster => aan
2012-02-20 17:13 aan Note Added: 0019681
2012-02-20 17:14 aan Assigned To aan => kgv
2012-02-20 17:14 aan Status reviewed => assigned
2012-02-21 08:32 abv Note Added: 0019687
2012-02-21 08:32 abv Status assigned => resolved
2012-02-21 08:32 abv Status resolved => reviewed
2012-02-21 12:47 mkv Assigned To kgv => apn
2012-02-22 11:11 apn Note Added: 0019715
2012-02-22 11:12 apn Test case number => Test case is not required
2012-02-22 11:12 apn Assigned To apn => bugmaster
2012-02-22 11:12 apn Status reviewed => tested
2012-02-22 16:42 bugmaster Note Added: 0019739
2012-02-22 16:42 bugmaster Status tested => verified
2012-02-22 16:42 bugmaster Resolution open => fixed
2012-02-22 16:42 bugmaster Assigned To bugmaster => abv
2012-03-29 17:26 bugmaster Changeset attached => occt master 91a16bc7