MantisBT - Community
View Issue Details
0032402Community[OCCT] OCCT:Codingpublic2021-05-30 13:222021-06-05 11:46
mr baley 
bugmaster 
normaltrivial 
verifiedfixed 
WindowsVC++ 201564 bit
[OCCT] 7.5.0 
[OCCT] 7.6.0* 
Not required
0032402: Coding Rules - eliminate msvc warning C4668 (symbol is not defined as a preprocessor macro, replacing with '0' for directive)
Some headers, like Standard_Macro.hxx, contain preprocessor macros for platform dependent code.
Some of this code like f.e.

#elif (defined(__GNUC__) && __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) || defined(__clang__)

is not consistent with respect to guard expressions.

Although both, the C and the C++ standard, explicitly allow using undefined identifiers and replace them with '0', see f.e. N3797, 16.1.4 (p. 402)

"After all replacements due to macro expansion and the defined unary operator
have been performed, all remaining identifiers and keywords149, except for true and false, are replaced with the pp-number 0, and then each preprocessing token is converted into a token"

a expression which consistently uses these rules would, using the example of the #elif above, be either

#elif (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))) || defined(__clang__)

or

#elif (defined(__GNUC__) && __GNUC__ > 4 || (defined(__GNUC__) && __GNUC__ == 4 && __GNUC_MINOR__ >= 6)) || defined(__clang__)

or

#elif (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) || defined(__clang__)

so that either all binary expressions are guarded by a unary one or none of them.

Although this is trivial, it reduces the noise during compilation for compilers where the value use of undefined preprocessor identifiers can be enabled as a warning; f.e. GCC provides the "-Wundef" flag and newer versions of Visual Studio seem to have this enabled per default when using /Wall.
Compile with and without the changes; the latter should result in more warnings for certain compilers and/or compiler options on the changed lines; f.e. GCCs "-Wundef"
No tags attached.
patch 0001-genproj-temporarily-enable-C4668.patch (4,499) 2021-06-01 11:51
https://tracker.dev.opencascade.org/
patch 0001-Configuration-genproj-add-EnableAllWarnings-to-gener.patch (36,109) 2021-06-01 11:52
https://tracker.dev.opencascade.org/
Issue History
2021-05-30 13:22mr baleyNew Issue
2021-05-30 13:22mr baleyAssigned To => kgv
2021-05-30 13:26gitNote Added: 0101510
2021-05-30 20:23kgvProduct Version7.5.1 => 7.5.0
2021-05-30 20:23kgvTarget Version => 7.6.0*
2021-05-30 20:23kgvSummaryMake platform definition preprocessor macros more consistent to not rely on defaulting to pp-number 0 => Coding Rules - Make platform definition preprocessor macros more consistent to not rely on defaulting to pp-number 0
2021-05-30 20:30kgvNote Added: 0101512
2021-05-31 10:02mr baleyNote Added: 0101515
2021-05-31 23:05kgvNote Added: 0101531
2021-05-31 23:06kgvNote Edited: 0101531bug_revision_view_page.php?bugnote_id=101531#r25319
2021-06-01 01:21mr baleyNote Added: 0101532
2021-06-01 01:23mr baleyNote Edited: 0101532bug_revision_view_page.php?bugnote_id=101532#r25321
2021-06-01 10:27gitNote Added: 0101534
2021-06-01 10:29mr baleyStatusnew => resolved
2021-06-01 10:29mr baleySteps to Reproduce Updatedbug_revision_view_page.php?rev_id=25323#r25323
2021-06-01 10:56kgvSummaryCoding Rules - Make platform definition preprocessor macros more consistent to not rely on defaulting to pp-number 0 => Coding Rules - eliminate msvc warning C4668 (symbol is not defined as a preprocessor macro, replacing with '0' for directive)
2021-06-01 10:58gitNote Added: 0101536
2021-06-01 11:01gitNote Added: 0101537
2021-06-01 11:36gitNote Added: 0101538
2021-06-01 11:48gitNote Added: 0101539
2021-06-01 11:51kgvFile Added: 0001-genproj-temporarily-enable-C4668.patch
2021-06-01 11:52kgvFile Added: 0001-Configuration-genproj-add-EnableAllWarnings-to-gener.patch
2021-06-01 13:39kgvNote Added: 0101541
2021-06-01 13:39kgvAssigned Tokgv => bugmaster
2021-06-01 13:39kgvStatusresolved => reviewed
2021-06-05 11:16bugmasterNote Added: 0101626
2021-06-05 11:16bugmasterStatusreviewed => tested
2021-06-05 11:19bugmasterTest case number => Not required
2021-06-05 11:21bugmasterChangeset attached => occt master ff1f0c9a
2021-06-05 11:21bugmasterStatustested => verified
2021-06-05 11:21bugmasterResolutionopen => fixed
2021-06-05 11:46gitNote Added: 0101637
2021-06-05 11:46gitNote Added: 0101638
2021-06-05 11:46gitNote Added: 0101639
2021-06-05 11:46gitNote Added: 0101640

Notes
(0101510)
git   
2021-05-30 13:26   
Branch CR32402 has been created by mr baley.

SHA-1: eefebb3e876ff059c1424506f913194d7cba9f01


Detailed log of new commits:

Author: zaphod
Date: Sun May 30 12:27:19 2021 +0200

    0032402: make preprocessor expressions consistent with respect to guard expressions
(0101512)
kgv   
2021-05-30 20:30   
-#if __QNX__
+#if defined(__QNX__) && __QNX__

There is no point checking __QNX__ for value - it was supposed to be just "#if defined(__QNX__)" with "defined" being lost by mistake.
(0101515)
mr baley   
2021-05-31 10:02   
That's true.
I was trying to preserve the exact same semantics and if __QNX__ could be defined to #define __QNX__ 0 they would change. But i don't know much about qnx and likely such a definition is defined to be invalid in that context.
(0101531)
kgv   
2021-05-31 23:05   
(edited on: 2021-05-31 23:06)
If you are considering patch for integration and it is ready - please switch bug into RESOLVED state.

(0101532)
mr baley   
2021-06-01 01:21   
(edited on: 2021-06-01 01:23)
If that comment was directed to me, i have to admit that i either don't know how to do it or am not allowed to do it; there's no option shown; i'm only allowed to reassign.
About the __QNX__ define, i don't know where to check all possible values for __QNX__. I'll happily make another commit which changes the check to your suggestion if you like.

Btw, i also did the same for "Standard_JmpBuf.hxx"
#elif IRIX
to
#elif defined(IRIX) && IRIX

(0101534)
git   
2021-06-01 10:27   
Branch CR32402_msvc_wall has been created by kgv.

SHA-1: 672b534974743a8479c82377e06de6c9d36113b7


Detailed log of new commits:

Author: kgv
Date: Tue Jun 1 10:28:36 2021 +0300

    Configuration, genproj - add EnableAllWarnings to generated msvc projects
(0101536)
git   
2021-06-01 10:58   
Branch CR32402_1 has been created by kgv.

SHA-1: 01bfd5221b8c06b94123c67552281ee2abf6af68


Detailed log of new commits:

Author: kgv
Date: Tue Jun 1 10:58:40 2021 +0300

    # genproj, temporarily enable C4668
    
    #- <AdditionalOptions>%(AdditionalOptions)</AdditionalOptions>
    #+ <AdditionalOptions>/w34668 %(AdditionalOptions)</AdditionalOptions>
(0101537)
git   
2021-06-01 11:01   
Branch CR32402_1 has been updated by kgv.

SHA-1: 1b7a65b5150578cb6b39524d41475b76c24634eb


Detailed log of new commits:

Author: zaphod
Date: Sun May 30 12:27:19 2021 +0200

    0032402: Coding Rules - eliminate msvc warning C4668 (symbol is not defined as a preprocessor macro, replacing with '0' for directive)
    
    Make preprocessor expressions consistent with respect to guard expressions.

(0101538)
git   
2021-06-01 11:36   
Branch CR32402_1 has been updated by kgv.

SHA-1: 1d42b76c533002125d6f98089ebc8f691415f9ea


Detailed log of new commits:

Author: kgv
Date: Tue Jun 1 11:37:25 2021 +0300

    Fixed usage of macros OCCT_DEBUG, DO_INVERSE, DRAW, CHFI3D_DEB by value.
    Removed obsolete hack for Sun Workshop 5.0 compiler.

(0101539)
git   
2021-06-01 11:48   
Branch CR32402_2 has been created by kgv.

SHA-1: 49175a4a355a4da63509ad7abef2319432a24054


Detailed log of new commits:

Author: zaphod
Date: Sun May 30 12:27:19 2021 +0200

    0032402: Coding Rules - eliminate msvc warning C4668 (symbol is not defined as a preprocessor macro, replacing with '0' for directive)
    
    Make preprocessor expressions consistent with respect to guard expressions.
    
    Fixed usage of macros __QNX__, IRIX, OCCT_DEBUG, DO_INVERSE, DRAW, CHFI3D_DEB by value.
    Removed obsolete hack for Sun Workshop 5.0 compiler.
(0101541)
kgv   
2021-06-01 13:39   
Please raise the patch
- OCCT: branch CR32402_2;
- OCC Products: branch CR32402_2.

http://jenkins-test-occt/view/CR32402_2-CR32402_2-KGV/ [^]
(0101626)
bugmaster   
2021-06-05 11:16   
Combination -
OCCT branch : IR-2021-06-04
master SHA - 6a920e02431a1bdfb01d6ff16e1e6a99204d9524
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-06-04 SHA - e72e4fdcde12770fe054f97d3a4a0c82e3c08f11
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17503.060000000427 / 17574.5700000004 [-0.41%]
Products
Total CPU difference: 11532.860000000106 / 11533.700000000124 [-0.01%]
Windows-64-VC14:
OCCT
Total CPU difference: 19219.09375 / 19393.5625 [-0.90%]
Products
Total CPU difference: 12831.25 / 12891.8125 [-0.47%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0101637)
git   
2021-06-05 11:46   
Branch CR32402 has been deleted by mnt.

SHA-1: eefebb3e876ff059c1424506f913194d7cba9f01
(0101638)
git   
2021-06-05 11:46   
Branch CR32402_1 has been deleted by mnt.

SHA-1: 1d42b76c533002125d6f98089ebc8f691415f9ea
(0101639)
git   
2021-06-05 11:46   
Branch CR32402_2 has been deleted by mnt.

SHA-1: 49175a4a355a4da63509ad7abef2319432a24054
(0101640)
git   
2021-06-05 11:46   
Branch CR32402_msvc_wall has been deleted by mnt.

SHA-1: 672b534974743a8479c82377e06de6c9d36113b7