View Issue Details

IDProjectCategoryView StatusLast Update
0027065Open CASCADEOCCT:Modeling Algorithmspublic2018-10-09 14:21
ReporterabvAssigned Toabv 
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Target Version7.0.0Fixed in Version7.0.0 
Summary0027065: BRepOffsetAPI_MakePipe misses definition of virtual method Generated()
DescriptionAs indicated by CLang compiler warning (see 0025076), class BRepOffsetAPI_MakePipe misses definition of virtual method Generated() inherited from BRepBuilderAPI_MakeShape. Instead, it defines its own method Generated() with two arguments.

This seems to be indication of flawed design of these classes (and perhaps even whole hierarchy of BRepBuilderAPI_MakeShape): virtual methods Generated() and Modified() are defined in some descendants and not defined in others, this probably means that these methods are not used from the base level. In that case there is no need to have these methods virtual.

If these methods are still useful as virtual, they must be defined correctly in all descendants.
Steps To ReproduceCompile with CLang:

/home/abv/occt/src/BRepOffsetAPI/BRepOffsetAPI_MakePipe.hxx:82:32: warning:
      'BRepOffsetAPI_MakePipe::Generated' hides overloaded virtual function
      [-Woverloaded-virtual]
  Standard_EXPORT TopoDS_Shape Generated (const TopoDS_Shape& SSpine, co...
                               ^
/home/abv/occt/src/BRepBuilderAPI/BRepBuilderAPI_MakeShape.hxx:54:55: note:
      hidden overloaded virtual function 'BRepBuilderAPI_MakeShape::Generated'
      declared here: different number of parameters (1 vs 2)
  Standard_EXPORT virtual const TopTools_ListOfShape& Generated (const T...
TagsNo tags attached.
Test case numberbugs modalg_6 bug27065_1, bug27065_2

Relationships

child of 0025076 closedabv Community Hidden overloaded virtual functions 

Activities

git

2016-02-04 13:36

administrator   ~0050417

Branch CR27065-1 has been created by jgv.

SHA-1: 1e89b28aef72927f871f340431c4eedef0c78da1


Detailed log of new commits:

Author: jgv
Date: Thu Feb 4 13:35:52 2016 +0300

    0027065: BRepOffsetAPI_MakePipe misses definition of virtual method Generated()

jgv

2016-02-04 13:38

developer   ~0050418

Please review the branch CR27065-1.

msv

2016-02-04 19:22

developer   ~0050430

Remarks:

src\BRepFill\BRepFill_Pipe.cxx

1) In the method Generated, when shape type is edge or vertex, it is constructed a new shell or wire to return as the result. I think this method should return the actual shapes from the result without new constructions. So, instead of adding shapes into a container shape, it is needed to add them directly in the output list.

2) Again in Generated, you pass the input shape in the method Face or Edge as a part of profile. So, if the user wants to get generated shapes from a part of spine this method will not work. It is needed to develop additional code in this method to take into account this consideration.

src\BRepOffsetAPI\BRepOffsetAPI_MakePipe.cxx

3) You have added comment above old Generated method, where you give it another name GeneratedElement. Please correct the comment.

abv

2016-02-05 08:56

manager   ~0050432

In BRepFill_Pipe.cxx, please declare local function UpdateMap() as static. As well, please put "{" on a new line -- this is the common style we shall follow in OCCT.

git

2016-02-08 18:50

administrator   ~0050498

Branch CR27065-1 has been updated by jgv.

SHA-1: e9ba2232f836ade43aa5288343918dd1107237cf


Detailed log of new commits:

Author: jgv
Date: Mon Feb 8 18:49:52 2016 +0300

    Minor corrections

jgv

2016-02-08 18:51

developer   ~0050499

Please review updated branch CR27065-1.

msv

2016-02-08 19:16

developer   ~0050501

My remark 2 was not followed. Again, if on input there is an edge from spine then the method will raise exception instead of returning a face.

abv

2016-02-09 09:22

manager   ~0050505

Mikhail, I believe that it is not really necessary to support Generated() for spine -- the logic of the operation is that you are extruding the profile, while spine is only defining a path. Possibility to get part of the result corresponding to some segment of a spine could be interesting in theory, however I propose that we keep it pending until we see real need for that.

msv

2016-02-09 13:09

developer   ~0050517

OK, I agree then.

msv

2016-02-09 13:10

developer   ~0050518

Reviewed.

git

2016-02-09 15:01

administrator   ~0050537

Branch CR27065-1 has been updated forcibly by mkv.

SHA-1: 1a7f9bc40f7a96582c15056b7a3b2fd782d025f0

mkv

2016-02-09 17:28

tester   ~0050549

Dear BugMaster,
Branch CR27065-1 was rebased on current master of occt git-repository.
SHA-1: 1a7f9bc40f7a96582c15056b7a3b2fd782d025f0

mkv

2016-02-09 19:16

tester   ~0050553

Dear BugMaster,
Branch CR27065-1 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: 1a7f9bc40f7a96582c15056b7a3b2fd782d025f0

Number of compiler warnings:

occt component :
Linux: 1 (0 on master)
Windows: 2 (0 on master)
MacOS : 1 (1 on master)

products component :
Linux: 36 (36 on master)
Windows: 0 (0 on master)

There is new additional compilation warning on Linux platform:
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR27065-1-master_build_occt_linux/1/warnings17Result/
BRepFill_Pipe.cxx:418, GNU C Compiler 4 (gcc), Priority: Normal
unused variable 'BB' [-Wunused-variable]

There is new additional compilation warning on Windows platform:
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR27065-1-master_build_occt_windows_64/1/warnings34Result/
BRepFill_Pipe.cxx:418, MSBuild, Priority: Normal
'BB' : unreferenced local variable

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:

http://occt-tests/CR27065-1-master-occt-64/Debian70-64/bugs/modalg_6/bug27065_1.html
http://occt-tests/CR27065-1-master-occt-64/Windows-64-VC10/bugs/modalg_6/bug27065_1.html
bugs modalg_6 bug27065_1: OK

http://occt-tests/CR27065-1-master-occt-64/Debian70-64/bugs/modalg_6/bug27065_2.html
http://occt-tests/CR27065-1-master-occt-64/Windows-64-VC10/bugs/modalg_6/bug27065_2.html
bugs modalg_6 bug27065_2: OK

Testing on Linux:
occt component :
Total MEMORY difference: 90145472 / 89959333 [+0.21%]
Total CPU difference: 18977.439999999802 / 19072.499999999924 [-0.50%]
products component :
Total MEMORY difference: 25610607 / 25540066 [+0.28%]
Total CPU difference: 5936.1199999999935 / 8193.57999999999 [-27.55%]

Testing on Windows:
occt component :
Total MEMORY difference: 57308935 / 57306429 [+0.00%]
Total CPU difference: 17917.323253898918 / 18107.114070498843 [-1.05%]
products component :
Total MEMORY difference: 17256114 / 17265139 [-0.05%]
Total CPU difference: 5735.20516389997 / 5549.173971399976 [+3.35%]

There are no differences in images found by testdiff.

mkv

2016-02-09 19:17

tester   ~0050554

Dear jgv,
Branch CR27065-1 has been rejected due to:
- additional warnings

git

2016-02-10 12:41

administrator   ~0050563

Branch CR27065_1 has been created by abv.

SHA-1: 2d2e12c59761fbc25c1d7208d015794b114de6eb


Detailed log of new commits:

Author: abv
Date: Wed Feb 10 12:41:06 2016 +0300

    // eliminate compiler warning

abv

2016-02-10 12:42

manager   ~0050564

Compiler warning is fixed in CR27065_1; please check compilation only (no need to re-test)

mkv

2016-02-10 15:33

tester   ~0050577

Dear BugMaster,
Branch CR27065_1 from occt git-repository was compiled on Linux, MacOS and Windows platforms .
SHA-1: 2d2e12c59761fbc25c1d7208d015794b114de6eb

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (1 on master)

mkv

2016-02-10 15:33

tester   ~0050578

Dear BugMaster,
Branch CR27065_1 is TESTED.

git

2016-04-17 13:27

administrator   ~0052864

Branch CR27065-1 has been deleted by kgv.

SHA-1: 1a7f9bc40f7a96582c15056b7a3b2fd782d025f0

git

2016-04-17 13:27

administrator   ~0052865

Branch CR27065_1 has been deleted by kgv.

SHA-1: 2d2e12c59761fbc25c1d7208d015794b114de6eb

Related Changesets

occt: master 5e9548e7

2016-02-04 10:35:52

abv


Committer: abv Details Diff
0027065: BRepOffsetAPI_MakePipe misses definition of virtual method Generated()

Virtual method Generated() inherited from BRepPrimAPI_MakeSweep is overridden in class BRepOffsetAPI_MakePipe, providing information on shapes generated from the profile.
Affected Issues
0027065
mod - src/BRepFill/BRepFill_Pipe.cxx Diff File
mod - src/BRepFill/BRepFill_Pipe.hxx Diff File
mod - src/BRepOffsetAPI/BRepOffsetAPI_MakePipe.cxx Diff File
mod - src/BRepOffsetAPI/BRepOffsetAPI_MakePipe.hxx Diff File
mod - src/QABugs/QABugs_19.cxx Diff File
add - tests/bugs/modalg_6/bug27065_1 Diff File
add - tests/bugs/modalg_6/bug27065_2 Diff File

Issue History

Date Modified Username Field Change
2016-01-08 19:10 abv New Issue
2016-01-08 19:10 abv Assigned To => msv
2016-01-08 19:11 abv Relationship added child of 0025076
2016-01-08 19:16 abv Steps to Reproduce Updated
2016-01-25 17:32 msv Assigned To msv => jgv
2016-01-25 17:32 msv Status new => assigned
2016-02-04 13:36 git Note Added: 0050417
2016-02-04 13:38 jgv Note Added: 0050418
2016-02-04 13:38 jgv Assigned To jgv => msv
2016-02-04 13:38 jgv Status assigned => resolved
2016-02-04 19:22 msv Note Added: 0050430
2016-02-04 19:22 msv Assigned To msv => jgv
2016-02-04 19:22 msv Status resolved => assigned
2016-02-05 08:56 abv Note Added: 0050432
2016-02-08 16:10 abv Target Version 7.1.0 => 7.0.0
2016-02-08 18:50 git Note Added: 0050498
2016-02-08 18:51 jgv Note Added: 0050499
2016-02-08 18:51 jgv Assigned To jgv => abv
2016-02-08 18:51 jgv Status assigned => resolved
2016-02-08 19:16 msv Note Added: 0050501
2016-02-08 19:16 msv Assigned To abv => jgv
2016-02-08 19:16 msv Status resolved => assigned
2016-02-09 09:22 abv Note Added: 0050505
2016-02-09 13:09 msv Note Added: 0050517
2016-02-09 13:10 msv Assigned To jgv => msv
2016-02-09 13:10 msv Status assigned => resolved
2016-02-09 13:10 msv Note Added: 0050518
2016-02-09 13:10 msv Assigned To msv => bugmaster
2016-02-09 13:10 msv Status resolved => reviewed
2016-02-09 13:35 mkv Assigned To bugmaster => mkv
2016-02-09 15:01 git Note Added: 0050537
2016-02-09 17:28 mkv Note Added: 0050549
2016-02-09 19:16 mkv Note Added: 0050553
2016-02-09 19:17 mkv Note Added: 0050554
2016-02-09 19:17 mkv Assigned To mkv => jgv
2016-02-09 19:17 mkv Status reviewed => assigned
2016-02-09 19:17 mkv Test case number => bugs modalg_6 bug27065_1, bug27065_2
2016-02-10 12:41 git Note Added: 0050563
2016-02-10 12:42 abv Note Added: 0050564
2016-02-10 12:42 abv Assigned To jgv => mkv
2016-02-10 12:42 abv Status assigned => feedback
2016-02-10 15:33 mkv Note Added: 0050577
2016-02-10 15:33 mkv Note Added: 0050578
2016-02-10 15:33 mkv Assigned To mkv => bugmaster
2016-02-10 15:33 mkv Status feedback => tested
2016-02-12 11:37 abv Changeset attached => occt master 5e9548e7
2016-02-12 11:37 abv Assigned To bugmaster => abv
2016-02-12 11:37 abv Status tested => verified
2016-02-12 11:37 abv Resolution open => fixed
2016-04-17 13:27 git Note Added: 0052864
2016-04-17 13:27 git Note Added: 0052865
2016-04-20 15:42 aiv Fixed in Version => 7.0.0
2016-04-20 15:51 aiv Status verified => closed