gfan fails when compiled with XCode 9.3

[vbraun]

On certain architectures only, gfan compiled with XCode 9.3 fails

sage -t --long src/doc/de/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/doc/en/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/doc/fr/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/doc/ja/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/doc/pt/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/doc/ru/tutorial/tour_advanced.rst  # 2 doctests failed
sage -t --long src/sage/rings/polynomial/groebner_fan.py  # 70 doctests failed
sage -t --long src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed

There are also reported Cython testsuite failures, so it might be a compiler issue.

[dimpase]

See testing results for the "stand-alone" gfan-6.2 on ​https://github.com/rwst/gfan06/issues/2

Basically, one test fails on OSX Xeon with Xcode 9.3 (same as on Ubuntu with gcc 5.4), while a slew of tests fail on OSX i5/7 with Xcode 9.3.

[dimpase]

gfan-6.2 source assumes that cdd headers are in cdd/, so one can e.g.

cd $SAGE_LOCAL/include
mkdir cdd
cp cdd*.h cdd/
cp setoper.h cdd/

before hitting make. Also, the makefile needs to be modified not to use -fno-guess-branch-probability on clang.

[fbissey]

Replying to dimpase:

gfan-6.2 source assumes that cdd headers are in cdd/, so one can e.g.

cd $SAGE_LOCAL/include
mkdir cdd
cp cdd*.h cdd/
cp setoper.h cdd/

before hitting make. Also, the makefile needs to be modified not to use -fno-guess-branch-probability on clang.

For cdd's header you can pass -DNOCDDPREFIX that's one of the things I have learnt from looking at the new spkg at #23353. We probably should depend on it.

[dimpase]

Can one try building/testing gfan with -O0 on affected Xcode/CPUs, see if this provides a fix?

(i.e. with CXXFLAGS (and CFLAGS?) having -O0 instead of the default?)

[dimpase]

Travis CI can now do Xcode 9.3 for you,so it's matter of writing a suitable yaml file to test gfan there...

[dimpase]

The crashes in gfan's 0.6.2 testsuite come from line 208 of app_main.cpp

    delete ep;//<--- In next release, make the class virtual

It's a pointer to an object of type EnumerationFilePrinter handling output.

it's perhaps safe to just delete it, as it is at the end of main() member function of GCats class. (I'll figure it out exactly, anyway, it's a small thing to fix).

If I delete this line then 44 out of 45 tests pass (same as on Ubunty).

This was tried on OSX 10.13.4 on very old Core2 Duo CPU, with clang from XCode 9.3, which is Apple LLVM version 9.1.0 (clang-902.0.39.1).

Please, please, try this on i5/i7.

[jhpalmieri]

I won’t be able to try until the weekend.

[fbissey]

Nice work Dima. So I took my broken 8.2.rc1. Digged the source for gfan-0.5, found the line in those sources, created a patch and put it in the build tree. Check ed that the multi_polynomial_ideal.py test is broken - yup. Rebuild gfan with my patch, gfan do not produces libraries, so there is no need to rebuild sage. We really want to upgrade and hope stuff like these is fixed by the way

[gfan-0.5.p2] ./intsinpolytope.h:1:9: warning: 'INTSINPOLYTOPE_H_INCLUDED' is used as a header guard here, followed by #define of a different macro [-Wheader-guard]
[gfan-0.5.p2] #ifndef INTSINPOLYTOPE_H_INCLUDED
[gfan-0.5.p2]         ^~~~~~~~~~~~~~~~~~~~~~~~~
[gfan-0.5.p2] ./intsinpolytope.h:2:9: note: 'INTSINPOLYTOPE_H_INLCUDED' is defined here; did you mean 'INTSINPOLYTOPE_H_INCLUDED'?
[gfan-0.5.p2] #define INTSINPOLYTOPE_H_INLCUDED
[gfan-0.5.p2]         ^~~~~~~~~~~~~~~~~~~~~~~~~
[gfan-0.5.p2]         INTSINPOLYTOPE_H_INCLUDED
[gfan-0.5.p2] 1 warning generated.

Tried the multi_polynomial_ideal.py test again. Success! I checked the other tests (well only one of the .rst, after all they are all copies of the same tests) and they work too.

I am doing full doctesting now in case something broke in the process.

And the patch

app_main.cpp

@@ -180 +180 @@
     }
 
     ep->close();
-    delete ep;
 
     printf("\n");
 

[fbissey]

All doctests passed. We can go ahead with this.

[dimpase]

Yep, I also tried the same as in comment 9 patch on 8.2.rc1 with latest Xcode on my museum Core2 Duo, and only some timeouts from tests, no real problems.

[dimpase]

Done. Hopefully this can still make it into 8.2.

---

New commits:

​7e13822	Revert "Trac #25113: OSX is f*ed up sometimes"
​2146433	Fixes miscompilation by clang from XCode 6.3, see #25118

[fbissey]

I am going to be a pain in your ass. Please make it a revision bump of gfan.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​ab26648

gfan version bump

[dimpase]

Done, sorry for forgetting this.

[dimpase]

I wonder if something needs to be done with configure, as this changes configure.ac.

[fbissey]

Depends on the flow of the release manager, but Volker didn't do a bump for #25113 so hopefully he has a procedure for that now.