OCCT won't work with the latest Xcode

Just to clarify: if we can agree in a solution, I am absolutely willing to fix this problem, I just want to get some pointers on how to fix this issue. In my opinion probably the best solution would be to pass those arguments as pointers rather then references.

From my perspective, such NULL-checks should be either removed or converted to pointer arguments if there is real use cases for that.
The proper solution would depend on the context.

if (&Weights == NULL) { //Weights is passed by reference to a function

Concerning the quoted fragment - it is clear that this case is actually used, thus it would be better to change syntax to pass pointer.

Unfortunately there are lots of real use cases for those null checks.

I have linked issue with other one #0025429 which seems to be related to the same problem.

Yes it is. This is a classic example where optionals could be a great solution, but I think OCCT probably don't want to introduce such a concept. Maybe later it should be considered, many algorithms could benefit from optionals.

Is it possible to define handle arguments or pointer arguments in CDL? I could not find a way.

Handles are automatically used within CDL for transient classes (e.g. there no way to pass class not through the handle in this case).

Pointers should be declared as dedicated type in Package cdl file and used further with specified alias (sample from "TCollection.cdl"):

    pointer MapNodePtr to MapNode from TCollection;

I have tried to fix this issue by replacing the references with pointers, unfortunately I stuck after a day, since it would require hundreds or maybe thousands of modifications all around OCCT.

Have you tried to assign address of Weights to some local pointer variable, and then using it in if()? This possibly could be work around.

As for comprehensive correction, I think that replacing references by pointers is best (safest) choice, as it will break existing interface and thus we will have to update (and check) all affected code. However it requires considerable effort and even more will conflict with changes made for #0024682. Thus I suggest this issue shall be fixed after #0024682 is integrated.

Yes, this is going to be my next effort, to create a CLANG_NULL_REFERENCE_WORKARAOUND_IF macro, that can be used until a proper fix will be implemented. (I am open for any suggestions for a better name :))

I have attached a workaround that seems to be working.

> diff file icon patch.diff [^] (6,698 bytes) 2015-04-16 17:15
>
> From [revision removed] Mon Sep 17 00:00:00 2001
> From: msv <[email removed]>
> Date: Fri, 23 Jan 2015 18:07:15 +0300
> Subject: [PATCH] 0025719: Boolean operations can crash
are you sure you have attached correct commit?

Sorry, the previous patch was another one, I have attached the correct one.

Well, that's awkward: it seems, that Clang is able even with this workaround to optimize out those ifs in some cases. The only thing that worked everywhere for me was this:

#define CLANG_WORKAROUND_REFERENCE_IS_NULL(ref) ((reinterpret_cast<size_t>(&ref) & 0xFFFFFF) == 0)

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Apr 30 10:33:21 2015 +0300

    Use Standard_IS_NULL_REFERENCE in BSplCLib and LDOM

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Apr 30 12:03:23 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    Workaround Clang optimization issue with NULL-reference checks
    using new macros Standard_IS_NULL_REFERENCE().

Hi Istvan,

> The problem with this is that Apple only allows distributing apps
> compiled with the latest toolchain
though this issue is certainly should be fixed, but could you please point out where this requirement can be found?
I'm unable to find such restriction on Apple site, including "App Store Review Guidelines" document.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Wed May 6 09:39:43 2015 +0300

    Standard_IS_NULL_REFERENCE - use optnone attribute instead of tricky manipulations

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Wed May 6 09:41:08 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    Workaround Clang optimization issue with NULL-reference checks
    using new macros Standard_IS_NULL_REFERENCE().

It seems that this policy has changed since the last time I read it, now it is only "strongly suggested" to use the latest Xcode.

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: akz
Date: Wed Sep 23 16:31:15 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    NULL references was eliminated for PLib, BSplCLib and BSplSLib. All affected code was changed accordingly.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

All dereferenced null pointers in PLib, BSplCLib and BSplSLib was eliminated. Corresponding references replaced by pointers. I've replaced only that references which can be NULL, such as Weights and additional Multipliers.

Branch CR26042_3 is ready for review (OCCT and Products repositories).

Please consider some (minor) remarks:

- In AppParCurves_Gradient.gxx, AppParCurves_MultiCurve.cxx, Approx_MCurvesToBSpCurve.cxx, BSplCLib.cxx, lxx, Convert*.cxx, ProjLib*.cxx, ShapeConstruct*.cxx: I suggest that PLib::NoWeights() be either restored, or replaced by BSplCLib::NoWeights() instead of NULL, to preserve semantics. The same applies to other places where NULL is used: it is better to use NoWeights() methods everywhere (e.g. in Geom) for consistency.

- Inline methods defined in PLib.lxx can be merged to PLib.hxx, and LXX file removed (it was artifact of CDL)

- BSplCLib.cxx: when calling a method of object by pointer, it is more natural to use -> instead of (*).:
  (*Mults).Lower() -> Mults->Lower()
  
- in PLib.hxx, BSpl*Lib.hxx, please indicate in documentation comments that weight and multiplicity arrays are passed by pointer so that NULL value is valid, meaning no weights / no multiplicities (this can be done in general comment to the package)

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

All remarks are handled. Branch CR26042_3.

No remarks, please test

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 1,
Branch CR26049_3 was rebased on branch IR-2015-09-24 of occt git-repository.
[revision removed]

Dear Commenter 1,
Branch CR26049_3 from occt git-repository (and IR-2015-09-24 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

There are following compilation errors:
Linux:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26042-3-master/job/CR26042-3-master_build_occt_products_linux/1/parsed_console/

../../../../src/DxfData/DxfData_TranslateCurve.cxx:429:19: error: no matching function for call to 'BSplCLib::RemoveKnot(Standard_Integer&, Standard_Integer&, Standard_Integer&, Standard_Boolean&, const TColgp_Array1OfPnt&, const TColStd_Array1OfReal&, const TColStd_Array1OfReal&, const TColStd_Array1OfInteger&, TColgp_Array1OfPnt&, TColStd_Array1OfReal&, TColStd_Array1OfReal&, TColStd_Array1OfInteger&, const Standard_Real&)'

../../../../src/DxfData/DxfData_TranslateCurve.cxx:438:19: error: no matching function for call to 'BSplCLib::RemoveKnot(Standard_Integer&, Standard_Integer&, Standard_Integer&, Standard_Boolean&, const TColgp_Array1OfPnt&, TColStd_Array1OfReal*, const TColStd_Array1OfReal&, const TColStd_Array1OfInteger&, TColgp_Array1OfPnt&, TColStd_Array1OfReal&, TColStd_Array1OfReal&, TColStd_Array1OfInteger&, const Standard_Real&)'

Windows:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26042-3-master/job/CR26042-3-master_build_occt_products_windows_64/1/parsed_console/

26>..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(429): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


26>..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(438): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(429): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]
..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(438): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


Number of compiler warnings:

occt component :
Linux: 17 (13 on master)
Windows: 0 (0 on master)

There are new additional compilation warnings on Linux platform:

http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-671474530/
BSplCLib_CurveComputation.gxx:1089, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'Weights' will never be NULL [-Waddress]
BSplCLib_CurveComputation.gxx:1130, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'theWeights' will never be NULL [-Waddress]
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-605032998/
BSplCLib_2.cxx:73, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'Weights' will never be NULL [-Waddress]
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-604556342/
BSplSLib.cxx:2213, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'WeightsArray' will never be NULL [-Waddress]

Regressions/Differences/Improvements:
(partially)
http://occt-tests/CR26042-3-master-occt-64/Debian70-64/summary.html
http://occt-tests/CR26042-3-master-occt-64/Windows-64-VC10/summary.html

Testing cases:
Not needed

Dear akz,
Branch [archived branch] has been rejected due to:
- compilation errors
- additional warnings
- regressions/differences/improvements

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear mkv,

I've update branch CR26042_3 on OCCT repository with minor changes that should fix warnings for GCC and probably fix regressions.

Could you please also use branch CR26042_3 on Products repository? That was pushed before testing and seems to be correct.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 1,
Branch CR26042_3 was rebased on branch IR-2015-09-24 of occt git-repository.
[revision removed]
Branch CR26042_3 was rebased on branch IR-2015-09-24 of products git-repository.
[revision removed]

Dear Commenter 1,
Branch CR26042_3 from occt git-repository (and CR26042_3 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
[revision removed]

Number of compiler warnings:

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

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

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 92116663 / 93204908 [-1.17%]
Total CPU difference: 19606.959999999224 / 19578.25999999923 [+0.15%]
products component :
Total MEMORY difference: 26311742 / 26313327 [-0.01%]
Total CPU difference: 6572.919999999949 / 7200.489999999998 [-8.72%]

Testing on Windows:
occt component :
Total MEMORY difference: 57872215 / 57877934 [-0.01%]
Total CPU difference: 18337.37154649904 / 17715.66076119908 [+3.51%]
products component :
Total MEMORY difference: 17135379 / 17140676 [-0.03%]
Total CPU difference: 5935.354446900002 / 5623.134045499962 [+5.55%]

There are no differences in images found by testdiff.

Dear Commenter 1,
Branch CR26042_3 is TESTED.

Branch [archived branch] has been deleted by Commenter 3.

[revision removed]

Branch [archived branch] has been deleted by Commenter 3.

[revision removed]

Branch [archived branch] has been deleted by Commenter 3.

[revision removed]

Branch [archived branch] has been deleted by Commenter 3.

[revision removed]