View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0022921 | Open CASCADE | OCCT:Foundation Classes | public | 2012-01-23 12:52 | 2013-10-09 17:53 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | integration request | ||
Status | closed | Resolution | fixed | ||
Product Version | 6.5.2 | ||||
Target Version | 6.5.3 | Fixed in Version | 6.5.3 | ||
Summary | 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 | ||||
Tags | No tags attached. | ||||
Test case number | Test case is not required | ||||
|
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! |
|
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. |
|
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 Bugmaster, 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 : ----------------------------------------------------------------- |
|
Fixed, please test |
|
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 |
|
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 |
occt: master 91a16bc7 2012-02-22 12:40:11 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 |
Date Modified | Username | Field | Change |
---|---|---|---|
2012-01-23 12:52 |
|
New Issue | |
2012-01-23 12:52 |
|
Assigned To | => abv |
2012-01-23 17:32 |
|
Note Added: 0019242 | |
2012-01-23 17:32 |
|
Assigned To | abv => kgv |
2012-01-23 17:32 |
|
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 |
|
Note Added: 0019660 | |
2012-02-20 12:49 |
|
Assigned To | abv => kgv |
2012-02-20 12:49 |
|
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 |
|
Assigned To | bugmaster => aan |
2012-02-20 17:13 |
|
Note Added: 0019681 | |
2012-02-20 17:14 |
|
Assigned To | aan => kgv |
2012-02-20 17:14 |
|
Status | reviewed => assigned |
2012-02-21 08:32 |
|
Note Added: 0019687 | |
2012-02-21 08:32 |
|
Status | assigned => resolved |
2012-02-21 08:32 |
|
Status | resolved => reviewed |
2012-02-21 12:47 |
|
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 |