Archived issue #0026783

Coding rules - eliminate GCC warning -Wunused-but-set-parameter

Open CASCADEOCCT:Codingclosed24 public notes

Search issues

Description

gcc 4.9.2 generates the following warnings with -Wextra option which should be fixed:
/occt/src/TopOpeBRep/TopOpeBRep_vprdeg.cxx|970|warning: parameter ‘hasOOEi’ set but not used [-Wunused-but-set-parameter]|
/occt/src/TestTopOpeDraw/TestTopOpeDraw_Displayer.hxx|54|warning: parameter ‘Tol’ set but not used [-Wunused-but-set-parameter]|
/occt/src/TestTopOpeDraw/TestTopOpeDraw_Displayer.hxx|58|warning: parameter ‘Par’ set but not used [-Wunused-but-set-parameter]|

Steps to reproduce

N/A

Public activity

24 archived notes

Participants are labeled by their role within this record.

01Commenter 1
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: rkv
Date: Tue Oct 27 10:44:12 2015 +0300

    26783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    Pass parameters by reference to be able to change/return their values.
02Commenter 1
Branch [archived branch] has been updated forcibly by Author.

[revision removed]
03Author
Please test the patch.
04Commenter 4
Dear BugMaster,
Branch CR26783 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

Number of compiler warnings:

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

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

Regressions/Differences/Improvements:
http://occt-tests/CR26783-master-occt-64/Debian70-64/bugs/fclasses/bug23237.html
http://occt-tests/CR26783-master-occt-64/Windows-64-VC10/bugs/fclasses/bug23237.html
bugs fclasses bug23237: FAILED

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 92604178 / 92333587 [+0.29%]
Total CPU difference: 19741.309999999703 / 19729.38999999969 [+0.06%]
products component :
Total MEMORY difference: 26215978 / 26272952 [-0.22%]
Total CPU difference: 7541.049999999976 / 7371.560000000013 [+2.30%]

Testing on Windows:
occt component :
Total MEMORY difference: 58410402 / 58044644 [+0.63%]
Total CPU difference: 18440.176205498705 / 18131.590627399022 [+1.70%]
products component :
Total MEMORY difference: 17260148 / 17167510 [+0.54%]
Total CPU difference: 5674.005971599975 / 5658.655473199974 [+0.27%]

There are no differences in images found by testdiff.
05Commenter 5
Dear rkv,
Branch [archived branch] has been rejected due to:
- regressions/differences/improvements
06Commenter 1
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: rkv
Date: Thu Oct 29 10:41:22 2015 +0300

    Merge branch 'CR26783' of git.dev.opencascade.org:occt into CR26783

Author: rkv
Date: Thu Oct 29 10:40:38 2015 +0300

    0026783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    Unreachable code is commented.

Author: rkv
Date: Tue Oct 27 10:44:12 2015 +0300

    26783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    Pass parameters by reference to be able to change/return their values.

07Commenter 1
Branch [archived branch] has been deleted by Participant.

[revision removed]
08Commenter 1
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: rkv
Date: Thu Oct 29 10:52:52 2015 +0300

    0026783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    Parameters are turned to prarmeters by reference. Unreachable code is commented.
09Author
Dear Mikhail,

-    if (hasOOEi) {
-      Handle(TopOpeBRepDS_Interference) EVI1i = ::MakeEPVInterference(T1ondg,iOOEi,PVIndex,par1ondg,
-                                         TopOpeBRepDS_VERTEX,TopOpeBRepDS_EDGE,isvertex1);
-      myHDS->StoreInterference(EVI1i,dgEd);
-    }
+    //if (hasOOEi) {
+    //  Handle(TopOpeBRepDS_Interference) EVI1i = ::MakeEPVInterference(T1ondg,iOOEi,PVIndex,par1ondg,
+                               //        TopOpeBRepDS_VERTEX,TopOpeBRepDS_EDGE,isvertex1);
+    //  myHDS->StoreInterference(EVI1i,dgEd);
+    //}

could you please confirm or reject removing these code paths?
10Commenter 10
I agree with these changes. This code is a part of old Booleans, we should not spend much time to make it better, so just elimination of warning is enough.
11Author
OK, then commented code should be removed within the patch.
12Commenter 1
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: rkv
Date: Thu Nov 5 16:07:37 2015 +0300

    0026783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    
    TestTopOpeDraw_Displayer::GetTol(), GetPar() - pass assigned parameters by reference.
    TopOpeBRep_FacesFiller::ProcessVPondgE() - remove unreachable code.
13Author
Please test updated patch in branch CR26783_1.
14Commenter 14
I would not remove the old code, because in such way we make distortion in the initial idea of the code. All this code should be removed, and it will be removed some time. I don't see any sense of removing the commented out code. May be it is better to insert there an explanation of making it commented.
15Author
> May be it is better to insert there an explanation of making it commented.
this is explained by commit description and kept for history within git.
So usually there no need to keep bloated code in file itself.
16Commenter 1
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
17Author
> Branch [archived branch] has been updated forcibly by Participant.
what for this branch has been rebased?
18Commenter 18
I agree with Mikhail: it is better to keep inactive code commented, as it will make it easier to see how offending variable (hasOOEi) was intended to be used, while looking at the code (without digging in Git history). Alternatively, we shall remove hasOOEi completely, also from FUN_putInterfonDegenEd(), but we seem to have no real interest in tracing all this now...

On the other side, I find it reasonable to remove ~650 commented lines of code at the beginning of that file -- this code seems to contain alternative implementation of the same stuff, never used.
19Commenter 1
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: rkv
Date: Thu Nov 5 16:07:37 2015 +0300

    0026783: Coding rules - eliminate GCC warning -Wunused-but-set-parameter
    
    TestTopOpeDraw_Displayer::GetTol(), GetPar() - pass assigned parameters by reference.
    TopOpeBRep_vprdeg.cxx, TopOpeBRep_FacesFiller::ProcessVPondgE() - remove commented code, comment inactive code.
20Commenter 20
Dear BugMaster,
Branch CR26783_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms on Release mode.
[revision removed]

There are following compilation errors:

Linux:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26783-2-master/job/CR26783-2-master_build_occt_linux/1/parsed_console/
../../../../src/RWStepDimTol/RWStepDimTol_RWNonUniformZoneDefinition.cxx:21:45: fatal error: StepRepr_HArray1OfShapeAspect.hxx: No such file or directory

Windows:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26783-2-master/job/CR26783-2-master_build_occt_windows_64/1/parsed_console/
         d:\builds\vc10\CR26783-2-master-occt-64\inc\StepDimTol_ToleranceZoneDefinition.hxx(24): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         d:\builds\vc10\CR26783-2-master-occt-64\inc\StepDimTol_ToleranceZoneDefinition.hxx(24): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         d:\builds\vc10\CR26783-2-master-occt-64\inc\StepDimTol_ToleranceZoneDefinition.hxx(24): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         d:\builds\vc10\CR26783-2-master-occt-64\inc\StepDimTol_ToleranceZoneDefinition.hxx(24): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         ..\..\..\src\RWStepDimTol\RWStepDimTol_RWNonUniformZoneDefinition.cxx(21): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         ..\..\..\src\RWStepDimTol\RWStepDimTol_RWProjectedZoneDefinition.cxx(22): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         ..\..\..\src\RWStepDimTol\RWStepDimTol_RWRunoutZoneDefinition.cxx(21): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
         ..\..\..\src\RWStepDimTol\RWStepDimTol_RWToleranceZoneDefinition.cxx(21): fatal error C1083: Cannot open include file: 'StepRepr_HArray1OfShapeAspect.hxx': No such file or directory
21Commenter 21
The problem was due to lost hxx files in StepRepr/FILES in fix for #0026850 on which branch CR26783 is based; now corrected
22Commenter 1
Branch [archived branch] has been deleted by Author.

[revision removed]
23Commenter 1
Branch [archived branch] has been deleted by Author.

[revision removed]
24Commenter 1
Branch [archived branch] has been deleted by Author.

[revision removed]

Related records