Archived issue #0029170
GCC 7.1 warnings -Wstrict-aliasing in Graphic3d_ArrayOfPrimitives.hxx
Description
When OCCT is built with GCC 7.1 (with MinGw-64), it reports multiple warnings "dereferencing type-punned pointer will break strict aliasing rules [-Wstrict-aliasing]" in two methods Graphic3d_ArrayOfPrimitives::SetVertexColor().
The reason is that reinterpret_cast<> is used to cast pointers of types where both initial and target types are different from char*.
The reason is that reinterpret_cast<> is used to cast pointers of types where both initial and target types are different from char*.
Steps to reproduce
Compile with GCC 7.1 (MinGW-64)
Public activity
8 archived notes
Participants are labeled by their role within this record.
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Sep 30 21:35:05 2017 +0300
0029170: GCC 7.1 warnings -Wstrict-aliasing in Graphic3d_ArrayOfPrimitives.hxx
Methods Graphic3d_ArrayOfPrimitives::SetVertexColor() accepting color as three double rgb values and Graphic3d_Vec4ub object are refactored to avoid using reinterpret_cast between pointers to complex types.
Similar correction is made in ViewerTest_ObjectCommands.cxx (static function VDrawSphere).
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Sep 30 21:35:05 2017 +0300
0029170: GCC 7.1 warnings -Wstrict-aliasing in Graphic3d_ArrayOfPrimitives.hxx
Methods Graphic3d_ArrayOfPrimitives::SetVertexColor() accepting color as three double rgb values and Graphic3d_Vec4ub object are refactored to avoid using reinterpret_cast between pointers to complex types.
Similar correction is made in ViewerTest_ObjectCommands.cxx (static function VDrawSphere).
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
Fix is is pushed to CR29170, please review.
Please note that method SetVertexColor (const Standard_Integer theIndex, const Standard_Integer theColor32), unlike other similar methods around, does not update myAttribs->NbElements at the end. Is that correct?
In general, the whole design of this class looks quite dangerous, as it operates directly with array of data which elements can have different content and size, without proper consistency checks. It is quite strange that methods like SetVertexColor behave differently in the case if index is out of range (exception is raised) and when either myAttribs data array is Null or data being set is not available (they do nothing).
It seems that this class should be redesigned as template, to make a choice of format of the stored data known at compile time, and enable static type checks. It conversion to template is not feasible, then it is not clear why so many large methods are implemented as inline -- they should rather be moved to CXX.
Please note that method SetVertexColor (const Standard_Integer theIndex, const Standard_Integer theColor32), unlike other similar methods around, does not update myAttribs->NbElements at the end. Is that correct?
In general, the whole design of this class looks quite dangerous, as it operates directly with array of data which elements can have different content and size, without proper consistency checks. It is quite strange that methods like SetVertexColor behave differently in the case if index is out of range (exception is raised) and when either myAttribs data array is Null or data being set is not available (they do nothing).
It seems that this class should be redesigned as template, to make a choice of format of the stored data known at compile time, and enable static type checks. It conversion to template is not feasible, then it is not clear why so many large methods are implemented as inline -- they should rather be moved to CXX.
Please test the patch.
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
Combination -
OCCT branch : [archived branch] [revision removed]
Products branch : [archived branch]
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.
http://jenkins-test-10.nnov.opencascade.com/view/CR29152-master-abv/
http://jenkins-test-08.nnov.opencascade.com/view/CR29170_master/view/OCCT%20compile/
Number of compiler warnings:
Mingw 5 (151 master)
Regressions/Differences/Improvements:
No regressions/differences
CPU differences:
No differences that require special attention
Image differences :
No differences that require special attention
Memory differences :
No differences that require special attention
OCCT branch : [archived branch] [revision removed]
Products branch : [archived branch]
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.
http://jenkins-test-10.nnov.opencascade.com/view/CR29152-master-abv/
http://jenkins-test-08.nnov.opencascade.com/view/CR29170_master/view/OCCT%20compile/
Number of compiler warnings:
Mingw 5 (151 master)
Regressions/Differences/Improvements:
No regressions/differences
CPU differences:
No differences that require special attention
Image differences :
No differences that require special attention
Memory differences :
No differences that require special attention
Branch [archived branch] has been deleted by Commenter 3.
[revision removed]
[revision removed]