Opened 6 years ago
Closed 6 years ago
#21701 closed defect (fixed)
Compiling sagelib with clang on OS X (Sierra): failure in cythonized sage/symbolic/expression.pyx
Reported by:  fbissey  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  porting  Keywords:  
Cc:  rws, mkoeppe  Merged in:  
Authors:  François Bissey, Dima Pasechnik  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  86b2c2d (Commits, GitHub, GitLab)  Commit:  86b2c2d09f0b907e10a2557b68a61d14305bb52a 
Dependencies:  Stopgaps: 
Description
I am hitting the following error in compiling sagelib with clang
[sagelib7.4.rc1] [1/9] clang fnostrictaliasing I/Users/fbissey/build/sage/local/var/tmp/sage/build/python22.7.10.p3/include DNDEBUG g fwrapv O3 Wall Wnounused I/Users/fbissey/build/sage/local/lib/python2.7/sitepackages/cysignals I/Users/fbissey/build/sage/local/include I/Users/fbissey/build/sage/local/include/python2.7 I/Users/fbissey/build/sage/local/lib/python2.7/sitepackages/numpy/core/include I/Users/fbissey/build/sage/src I/Users/fbissey/build/sage/src/sage/ext I/Users/fbissey/build/sage/src/build/cythonized I/Users/fbissey/build/sage/src/build/cythonized/sage/ext I/Users/fbissey/build/sage/local/include/python2.7 c /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp o build/temp.macosx10.9x86_642.7/Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.o std=c++11 fnostrictaliasing [sagelib7.4.rc1] In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:336: [sagelib7.4.rc1] In file included from /Users/fbissey/build/sage/src/sage/symbolic/ginac_wrap.h:11: [sagelib7.4.rc1] In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:38: [sagelib7.4.rc1] /Users/fbissey/build/sage/local/include/pynac/integral.h:44:5: warning: 'GiNaC::integral::evalf' hides overloaded virtual function [Woverloadedvirtual] [sagelib7.4.rc1] ex evalf(int level=0) const; [sagelib7.4.rc1] ^ [sagelib7.4.rc1] /Users/fbissey/build/sage/local/include/pynac/basic.h:176:13: note: hidden overloaded virtual function 'GiNaC::basic::evalf' declared here: different number of parameters (2 vs 1) [sagelib7.4.rc1] virtual ex evalf(int level = 0, PyObject* parent=nullptr) const; [sagelib7.4.rc1] ^ [sagelib7.4.rc1] /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:27231:30: error: no member named 'operator!=' in 'std::__1::__list_const_iterator<GiNaC::ex, void *>' [sagelib7.4.rc1] __pyx_t_2 = (__pyx_v_itr.operator!=(__pyx_v_lstend) != 0); [sagelib7.4.rc1] ~~~~~~~~~~~ ^ [sagelib7.4.rc1] /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:27446:30: error: no member named 'operator!=' in 'std::__1::__list_const_iterator<GiNaC::ex, void *>' [sagelib7.4.rc1] __pyx_t_3 = (__pyx_v_itr.operator!=(__pyx_v_found.end()) != 0); [sagelib7.4.rc1] ~~~~~~~~~~~ ^ [sagelib7.4.rc1] /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:28649:30: error: no member named 'operator!=' in 'std::__1::__tree_const_iterator<GiNaC::ex, std::__1::__tree_node<GiNaC::ex, void *> *, long>' [sagelib7.4.rc1] __pyx_t_3 = (__pyx_v_itr.operator!=(__pyx_v_sym_set.end()) != 0); [sagelib7.4.rc1] ~~~~~~~~~~~ ^ [sagelib7.4.rc1] 1 warning and 3 errors generated.
This looks like a pynac
or pynac
related problem.
Change History (65)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
Can you try with clang stdlib=libstdc++ ...
?
comment:3 Changed 6 years ago by
I just noticed that clang is used instead of clang++, that may have an impact. I will try to rectify that and if it doesn't work try what you said.
comment:4 Changed 6 years ago by
No luck. Difficult to force clang++
I had to go manual
Mirage:src fbissey$ clang++ fnostrictaliasing I/Users/fbissey/build/sage/local/var/tmp/sage/build/python22.7.10.p3/include DNDEBUG g fwrapv O3 Wall Wnounused I/Users/fbissey/build/sage/local/lib/python2.7/sitepackages/cysignals I/Users/fbissey/build/sage/local/include I/Users/fbissey/build/sage/local/include/python2.7 I/Users/fbissey/build/sage/local/lib/python2.7/sitepackages/numpy/core/include I/Users/fbissey/build/sage/src I/Users/fbissey/build/sage/src/sage/ext I/Users/fbissey/build/sage/src/build/cythonized I/Users/fbissey/build/sage/src/build/cythonized/sage/ext I/Users/fbissey/build/sage/local/include/python2.7 c /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp o build/temp.macosx10.9x86_642.7/Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.o std=c++11 fnostrictaliasing stdlib=libstdc++ In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:336: In file included from /Users/fbissey/build/sage/src/sage/symbolic/ginac_wrap.h:11: In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:28: In file included from /Users/fbissey/build/sage/local/include/pynac/basic.h:40: In file included from /Users/fbissey/build/sage/local/include/pynac/registrar.h:30: /Users/fbissey/build/sage/local/include/pynac/class_info.h:43:31: error: no member named 'move' in namespace 'std'; did you mean 'modf'? class_info(OPT o) : options(std::move(o)), next(first), parent(nullptr) ^~~~~~~~~ modf /usr/include/math.h:407:15: note: 'modf' declared here extern double modf(double, double *); ^ In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:336: In file included from /Users/fbissey/build/sage/src/sage/symbolic/ginac_wrap.h:11: In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:28: In file included from /Users/fbissey/build/sage/local/include/pynac/basic.h:40: In file included from /Users/fbissey/build/sage/local/include/pynac/registrar.h:31: /Users/fbissey/build/sage/local/include/pynac/print.h:244:21: error: no type named 'unique_ptr' in namespace 'std' print_functor(std::unique_ptr<print_functor_impl> impl_) : impl(std::move(impl_)) {} ~~~~~^ /Users/fbissey/build/sage/local/include/pynac/print.h:244:31: error: expected ')' print_functor(std::unique_ptr<print_functor_impl> impl_) : impl(std::move(impl_)) {} ^ /Users/fbissey/build/sage/local/include/pynac/print.h:244:15: note: to match this '(' print_functor(std::unique_ptr<print_functor_impl> impl_) : impl(std::move(impl_)) {} ^ /Users/fbissey/build/sage/local/include/pynac/print.h:269:7: error: no type named 'unique_ptr' in namespace 'std' std::unique_ptr<print_functor_impl> impl; ~~~~~^ /Users/fbissey/build/sage/local/include/pynac/print.h:269:17: error: expected member name or ';' after declaration specifiers std::unique_ptr<print_functor_impl> impl; ~~~~~~~~~~~~~~~^ /Users/fbissey/build/sage/local/include/pynac/print.h:242:20: error: member initializer 'impl' does not name a nonstatic data member or base class print_functor() : impl(nullptr) {} ^~~~~~~~~~~~~ /Users/fbissey/build/sage/local/include/pynac/print.h:243:58: error: no member named 'impl' in 'GiNaC::print_functor' print_functor(const print_functor & other) : impl(other.impl.get() ? other.impl>duplicate() : nullptr) {} ~~~~~ ^ /Users/fbissey/build/sage/local/include/pynac/print.h:243:77: error: no member named 'impl' in 'GiNaC::print_functor' print_functor(const print_functor & other) : impl(other.impl.get() ? other.impl>duplicate() : nullptr) {} ~~~~~ ^ /Users/fbissey/build/sage/local/include/pynac/print.h:244:71: error: no member named 'move' in namespace 'std' print_functor(std::unique_ptr<print_functor_impl> impl_) : impl(std::move(impl_)) {} ~~~~~^ /Users/fbissey/build/sage/local/include/pynac/print.h:244:76: error: use of undeclared identifier 'impl_' print_functor(std::unique_ptr<print_functor_impl> impl_) : impl(std::move(impl_)) {} ^ /Users/fbissey/build/sage/local/include/pynac/print.h:247:58: error: member initializer 'impl' does not name a nonstatic data member or base class print_functor(void f(const T &, const C &, unsigned)) : impl(new print_ptrfun_handler<T, C>(f)) {} ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /Users/fbissey/build/sage/local/include/pynac/print.h:250:59: error: member initializer 'impl' does not name a nonstatic data member or base class print_functor(void (T::*f)(const C &, unsigned) const) : impl(new print_memfun_handler<T, C>(f)) {} ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /Users/fbissey/build/sage/local/include/pynac/print.h:255:34: error: no member named 'impl' in 'GiNaC::print_functor' print_functor_impl *p = other.impl.get(); ~~~~~ ^ /Users/fbissey/build/sage/local/include/pynac/print.h:256:4: error: use of undeclared identifier 'impl' impl.reset(p ? other.impl>duplicate() : nullptr); ^ /Users/fbissey/build/sage/local/include/pynac/print.h:256:25: error: no member named 'impl' in 'GiNaC::print_functor' impl.reset(p ? other.impl>duplicate() : nullptr); ~~~~~ ^ /Users/fbissey/build/sage/local/include/pynac/print.h:263:5: error: use of undeclared identifier 'impl' (*impl)(obj, c, level); ^ /Users/fbissey/build/sage/local/include/pynac/print.h:266:33: error: use of undeclared identifier 'impl' bool is_valid() const { return impl.get(); } ^ In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:336: In file included from /Users/fbissey/build/sage/src/sage/symbolic/ginac_wrap.h:11: In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:28: /Users/fbissey/build/sage/local/include/pynac/basic.h:364:60: error: no member named 'forward' in namespace 'std' return const_cast<B &>(static_cast<const B &>((new B(std::forward<Args>(args)...))>setflag(status_flags::dynallocated))); ~~~~~^ /Users/fbissey/build/sage/local/include/pynac/basic.h:364:68: error: 'Args' does not refer to a value return const_cast<B &>(static_cast<const B &>((new B(std::forward<Args>(args)...))>setflag(status_flags::dynallocated))); ^ /Users/fbissey/build/sage/local/include/pynac/basic.h:361:31: note: declared here template<class B, typename... Args> ^ fatal error: too many errors emitted, stopping now [ferrorlimit=] 20 errors generated.
comment:5 Changed 6 years ago by
The first error in the description comes from the following in the generated file
/* "sage/symbolic/expression.pyx":4581 * cdef GExListIter itr = mlst.begin() * cdef GExListIter lstend = mlst.end() * while itr.is_not_equal(lstend): # <<<<<<<<<<<<<< * key = new_Expression_from_GEx(self._parent, itr.obj().lhs()) * val = new_Expression_from_GEx(self._parent, itr.obj().rhs()) */ while (1) { __pyx_t_2 = (__pyx_v_itr.operator!=(__pyx_v_lstend) != 0); if (!__pyx_t_2) break;
That's how the while
from cython is transformed.
comment:6 Changed 6 years ago by
Is this Mac or Linux? Please state in the description.
comment:7 Changed 6 years ago by
 Summary changed from Compiling sagelib with clang: failure in sage/symbolic/expression.pyx  pynac problem to Compiling sagelib with clang on OS X (Sierra): failure in sage/symbolic/expression.pyx  pynac problem
comment:8 followup: ↓ 9 Changed 6 years ago by
 Summary changed from Compiling sagelib with clang on OS X (Sierra): failure in sage/symbolic/expression.pyx  pynac problem to Compiling sagelib with clang on OS X (Sierra): failure in cythonized sage/symbolic/expression.pyx
As you can see from https://gcc.gnu.org/onlinedocs/gcc4.6.3/libstdc++/api/a00344.html the libstdc++ has an operator!=
member for std::lst::const_iterator
. The definition is in stl_list.h. From the error it looks like the MacOS version does not have it, although Cython needs it. This is not a Pynac issue IMHO.
comment:9 in reply to: ↑ 8 Changed 6 years ago by
Replying to rws:
As you can see from https://gcc.gnu.org/onlinedocs/gcc4.6.3/libstdc++/api/a00344.html the libstdc++ has an
operator!=
member forstd::lst::const_iterator
. The definition is in stl_list.h. From the error it looks like the MacOS version does not have it, although Cython needs it. This is not a Pynac issue IMHO.
Then somebody should test if this error happens only on OS X. So the question is: what happens with clang
on Linux?
comment:10 Changed 6 years ago by
Actually, using a command adapted from yours (clang3.8.0):
clang fnostrictaliasing I/home/ralf/sage/local/include/python2.7 DNDEBUG g fwrapv O3 Wall Wnounused I/home/ralf/sage/local/lib/python2.7/sitepackages/cysignals I/home/ralf/sage/local/include I/home/ralf/sage/src/build/sage/local/include/python2.7 I/home/ralf/sage/local/lib/python2.7/sitepackages/numpy/core/include I/home/ralf/sage/src/build I/home/ralf/sage/src/build/sage/ext I/home/ralf/sage/src/build/cythonized I/home/ralf/sage/src/build/cythonized/sage/ext I/home/ralf/sage/src/build/sage/local/include/python2.7 c /home/ralf/sage/src/build/cythonized/sage/symbolic/expression.cpp o /home/ralf/sage/src/build/cythonized/sage/symbolic/expression.o std=c++11 fnostrictaliasing In file included from /home/ralf/sage/src/build/cythonized/sage/symbolic/expression.cpp:336: In file included from /home/ralf/sage/src/build/cythonized/sage/symbolic/ginac_wrap.h:11: In file included from /home/ralf/sage/local/include/pynac/ginac.h:38: /home/ralf/sage/local/include/pynac/integral.h:44:5: warning: 'GiNaC::integral::evalf' hides overloaded virtual function [Woverloadedvirtual] ex evalf(int level=0) const; ^ /home/ralf/sage/local/include/pynac/basic.h:177:13: note: hidden overloaded virtual function 'GiNaC::basic::evalf' declared here: different number of parameters (2 vs 1) virtual ex evalf(int level = 0, PyObject* parent=nullptr) const; ^ 1 warning generated.
So no errors. But I have a full libstdc++ on system. So it is possible that your Xcode version is missing a part of it.
comment:11 Changed 6 years ago by
Cannot dismiss that I guess. That would be a big blow on apple's part.
comment:12 Changed 6 years ago by
OK it is there: /usr/include/c++/4.2.1/bits/stl_list.h
on OS X and it has the right operator. Of course anything with bits
is an internal header and apparently the proper way to include it is with <list>
. So I am guessing some inclusion is either using a nonstandard header or rely on some indirect inclusion. Digging...
comment:13 Changed 6 years ago by
Even manually adding #include <list>
in the cpp file doesn't solve the problem. There must something subtle at work.
comment:14 Changed 6 years ago by
My understanding of some C++ matters is shaky at best. Does it make a difference that operator!=
is a "nonmember" function? http://en.cppreference.com/w/cpp/container/list
comment:15 Changed 6 years ago by
Note that the missing operator!=()
is not a member of list
but of list::const_iterator
, see line 266 in https://gcc.gnu.org/onlinedocs/gcc4.6.3/libstdc++/api/a01055_source.html
Does the exact equivalent exist in your stl_list.h?
comment:16 Changed 6 years ago by
It appears to be in the header  which bears a lot of similarities with your link. We are talking about this section?
00198 template<typename _Tp> 00199 struct _List_const_iterator
comment:17 Changed 6 years ago by
Yes.
comment:18 Changed 6 years ago by
sage 7.5.beta2 (includes all the stuff we found out so far) and xcode 8.1 on sierra and still a similar kind of error. Different place but same error
clang++ fnostrictaliasing DNDEBUG g fwrapv O3 Wall Wnounused I/Users/fbissey/build/sage/local/include I/Users/fbissey/build/sage/local/include/python2.7 I/Users/fbissey/build/sage/local/lib/python2.7/sitepackages/numpy/core/include I/Users/fbissey/build/sage/src I/Users/fbissey/build/sage/src/sage/ext I/Users/fbissey/build/sage/src/build/cythonized I/Users/fbissey/build/sage/src/build/cythonized/sage/ext I/Users/fbissey/build/sage/local/include/python2.7 c /Users/fbissey/build/sage/src/build/cythonized/sage/libs/pynac/pynac.cpp o src/build/temp.macosx10.9x86_642.7/Users/fbissey/build/sage/src/build/cythonized/sage/libs/pynac/pynac.o std=c++11 fnostrictaliasing In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/libs/pynac/pynac.cpp:486: In file included from /Users/fbissey/build/sage/src/sage/libs/pynac/wrap.h:11: In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:38: /Users/fbissey/build/sage/local/include/pynac/integral.h:44:5: warning: 'GiNaC::integral::evalf' hides overloaded virtual function [Woverloadedvirtual] ex evalf(int level=0) const; ^ /Users/fbissey/build/sage/local/include/pynac/basic.h:177:13: note: hidden overloaded virtual function 'GiNaC::basic::evalf' declared here: different number of parameters (2 vs 1) virtual ex evalf(int level = 0, PyObject* parent=nullptr) const; ^ /Users/fbissey/build/sage/src/build/cythonized/sage/libs/pynac/pynac.cpp:4479:30: error: no member named 'operator!=' in 'std::__1::__tree_const_iterator<unsigned int, std::__1::__tree_node<unsigned int, void *> *, long>' __pyx_t_2 = (__pyx_v_itr.operator!=(__pyx_v_s.end()) != 0); ~~~~~~~~~~~ ^ 1 warning and 1 error generated.
comment:19 Changed 6 years ago by
It's different. Before it was list, now it's tree. BTW the warning is fixed in flint git master.
comment:20 Changed 6 years ago by
Ah no sorry, different warning, not flint.
comment:21 Changed 6 years ago by
The original one had three errors, 2 "list" and one "tree". In any case it is extremely vexing. I need to find a C++ expert that could have a clue.
comment:23 Changed 6 years ago by
I don't claim being a tenured C++ guru, but
pynac.pxd: bint is_not_equal "operator!=" (GParamSetIter i)
looks strange. IIRC iterators can be compared for nonequality using merely !=
.
If in sage/libs/pynac/pynac.pyx
(line 194) I replace
while itr.is_not_equal(s.end()):
with
while itr != s.end():
this still compiles (with gcc), and tests in symbolic/ pass, too.
Can you try this with clang?
comment:24 Changed 6 years ago by
That appears to be effective, let's see how far I can go with it.
comment:25 Changed 6 years ago by
Had a new C/C++ conflict but I was still on 7.5.beta2. Switched to beta3 and rebuilding now.
comment:26 Changed 6 years ago by
OK that changes definitely get me past the compiling of sage/libs/pynac/pynac.so
, the new issue in beta3 means that I haven't reach the initial problem in sage/symbolyc/expression.pyx
. Since there were several errors that particular fix may be insufficient. I should put a temporary branch here for other people interested in testing at some point.
comment:27 Changed 6 years ago by
After sorting the m4ri
problems I am back at the original error in expression.pyx. I will see if I can apply Dima's suggestion again.
comment:28 Changed 6 years ago by
Had to apply Dima's suggestion 3 times. We have compile!
comment:29 followup: ↓ 30 Changed 6 years ago by
Nice. Looks like another example where gcc is more generous than clang with template function creation.
comment:30 in reply to: ↑ 29 Changed 6 years ago by
Replying to rws:
Nice. Looks like another example where gcc is more generous than clang with template function creation.
Well, I am not sure it is about templates  it seems to be about an operator !=
being replaced by gcc with "plain" !=
. Or perhaps I'm misreading the code there.
comment:31 Changed 6 years ago by
Ideally, all those cdef struct
in pynac.pxd
should be replaced by cdef cppclass
.
comment:32 Changed 6 years ago by
Yes, I have numerous warning about struct
that should class
.
I am doing a new compile from scratch, doctests of the first build has shown a few issues but it started as 7.5.beta2 switched to beta3 in flight so everything will be crosschecked with a new build.
comment:33 followup: ↓ 34 Changed 6 years ago by
I have been grepping is_not_equal
over the entire sage tree and apart from the one occurence in pynac.pyx
and 3 in symbolic/expression.pyx
I found 3 in pynac.pxd
grep rI is_not_equal src/sage/* src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GExListIter i) src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GParamSetIter i) src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GExSetIter i)
I am guessing they can go as well if I remove all the instances of is_not_equal
. Second opinion? Ralf?
comment:34 in reply to: ↑ 33 Changed 6 years ago by
Replying to fbissey:
I have been grepping
is_not_equal
over the entire sage tree and apart from the one occurence inpynac.pyx
and 3 insymbolic/expression.pyx
I found 3 inpynac.pxd
grep rI is_not_equal src/sage/* src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GExListIter i) src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GParamSetIter i) src/sage/libs/pynac/pynac.pxd: bint is_not_equal "operator!=" (GExSetIter i)I am guessing they can go as well if I remove all the instances of
is_not_equal
. Second opinion? Ralf?
Why would you remove these that compile? This seems to be asking for trouble.
The whole point of that operator!=
is that you can actually provide your own implementation. E.g. in Sage there are lots of these in local/include/[boost,NTL]
, and moreover in
local/include/pynac/ptr.h: bool operator!=(const ptr<U> & rhs) const throw() { return p != get_pointer(rhs); } local/include/pynac/ptr.h: inline friend bool operator!=(const ptr & lhs, const U * rhs) throw() { return lhs.p != rhs; } local/include/pynac/ptr.h: inline friend bool operator!=(const U * lhs, const ptr & rhs) throw() { return lhs != rhs.p; } local/include/pynac/ex.h: bool operator!=(const const_iterator &other) const throw() local/include/pynac/ex.h: bool operator!=(const _iter_rep &other) const throw() local/include/pynac/ex.h: bool operator!=(const const_preorder_iterator &other) const throw() local/include/pynac/ex.h: bool operator!=(const const_postorder_iterator &other) const throw() local/include/pynac/operators.h:const relational operator!=(const ex & lh, const ex & rh); local/include/pynac/numeric.h: bool operator!=(const numeric &other) const;
We should check whether one of the latter must have been taken by the compiler,
but somehow the compiler didn't manage it.
I guess that in particular GExListIter
and GExSetIter
are dealt with such an operator in local/include/pynac/ex.h
, and it does do some stuff...
comment:35 followup: ↓ 36 Changed 6 years ago by
Why would I remove them?
1) proven not work with clang 2) I am removing all the instances were they are used
I could understand keeping them just in case if they were working, but my experience is that they do not. If I keep them there someone will try to use them and break the clang build.
<disclaimer: I am a bit grumpy after dealing with some very ugly setup on the Pan cluster in Auckland for a local user. Right now, I am seeing them as an accident waiting to happen.>
comment:36 in reply to: ↑ 35 Changed 6 years ago by
Replying to fbissey:
Why would I remove them?
1) proven not work with clang 2) I am removing all the instances were they are used
I could understand keeping them just in case if they were working, but my experience is that they do not. If I keep them there someone will try to use them and break the clang build.
just removing them potentially introduces bugs/memory leaks. I'd think it's ok to remove the GParamSetIter
operator!=, but not the other two. Did you have to remove the latter to compile, too?
Anyhow, this all should be fixable by doing something right in C++ code.
comment:37 followup: ↓ 39 Changed 6 years ago by
Can you explain to me how something that is not used can prevent memory leaks? May be I shouldn't ask, it is C++ after all.
comment:38 Changed 6 years ago by
I should answer the question: No I didn't need to remove it to compile. I discovered them as an afterthought. I started to wonder if there were many other instances, and that's when I discovered those.
comment:39 in reply to: ↑ 37 Changed 6 years ago by
Replying to fbissey:
Can you explain to me how something that is not used can prevent memory leaks? May be I shouldn't ask, it is C++ after all.
Hmm, what do you mean by saying "not used"? Try removing definitions of
operator!=
from pynac C++ headers in local/include/, and see how far
you can go (using gcc, even). E.g. this one (in ex.h) clearly does stuff:
bool operator==(const const_iterator &other) const throw() { return are_ex_trivially_equal(e, other.e) && i == other.i; } bool operator!=(const const_iterator &other) const throw() { return !(*this == other); }
by invoking are_ex_trivially_equal()
. (This would be a proper test of "not used" claim).
comment:40 followup: ↓ 41 Changed 6 years ago by
I think we are not talking about the same thing Dima! You are talking about removing operator!=
definitions. I am only talking about removing the three instances of is_not_equal
. is_not_equal
was only used in the places that caused clang
to fail.
comment:41 in reply to: ↑ 40 ; followup: ↓ 49 Changed 6 years ago by
Replying to fbissey:
I think we are not talking about the same thing Dima! You are talking about removing
operator!=
definitions. I am only talking about removing the three instances ofis_not_equal
.is_not_equal
was only used in the places that causedclang
to fail.
Cython converts is_not_equal
into operator!=
. By replacing is_not_equal
with !=
in cython files, we in effect replacing whatever operator!=
implementation is picked by the C++ compiler with a "trivial" implementation, which might be correct, or not. In clang's OSX case, it simply fails to pick an appropriate implementation of operator!=
. This might either be a bug of the compiler, or a missing default implementation in STL.
See, there are several ifs here.
comment:42 followup: ↓ 46 Changed 6 years ago by
 Branch set to u/fbissey/pynacclang
 Commit set to 86b2c2d09f0b907e10a2557b68a61d14305bb52a
 Status changed from new to needs_review
I am not going to fight that fight. If a reviewer think they should not go, fine with me. Still needs some testing on normal setups. Not expecting problems but we never know.
New commits:
86b2c2d  Convert is_not_equal to != for clang

comment:43 followup: ↓ 44 Changed 6 years ago by
If I want to test this on OS X, do I just do export CC=clang
and then make
?
comment:44 in reply to: ↑ 43 Changed 6 years ago by
Replying to jhpalmieri:
If I want to test this on OS X, do I just do
export CC=clang
and thenmake
?
No. You also need this
diff git a/src/bin/sageenv b/src/bin/sageenv index 54c0783..e25081d 100644  a/src/bin/sageenv +++ b/src/bin/sageenv @@ 528,15 +528,15 @@ if [ "$UNAME" = "Darwin" ]; then fi # Override CC, CPP, CXX, FC if the GCC spkg was installed. if [ x "$SAGE_LOCAL/bin/gcc" ]; then  CC=gcc fi if [ x "$SAGE_LOCAL/bin/cpp" ]; then  CPP=cpp fi if [ x "$SAGE_LOCAL/bin/g++" ]; then  CXX=g++ fi +#if [ x "$SAGE_LOCAL/bin/gcc" ]; then +# CC=gcc +#fi +#if [ x "$SAGE_LOCAL/bin/cpp" ]; then +# CPP=cpp +#fi +#if [ x "$SAGE_LOCAL/bin/g++" ]; then +# CXX=g++ +#fi if [ x "$SAGE_LOCAL/bin/gfortran" ]; then FC=gfortran fi
Then CC=clang CXX=clang++ make
. I don't have a proper way to switch yet, the last thing I tried made gcc
fail to build (we still need gcc
because of gfortran
).
comment:45 Changed 6 years ago by
I forgot there is one more problem to build sage with clang that is not included in this ticket. The first line src/sage/libs/pynac/pynac.pxd
has to be edited out.
comment:46 in reply to: ↑ 42 ; followup: ↓ 48 Changed 6 years ago by
Replying to fbissey:
I am not going to fight that fight. If a reviewer think they should not go, fine with me. Still needs some testing on normal setups. Not expecting problems but we never know.
I see no problem because in ginac/fderivative.h
typedef std::multiset<unsigned> paramset;
this is just a standard container with a standard iterator. Rather, the Pynac interface in pynac.pxd
could be cleaned up more rigourously (in another ticket).
comment:47 Changed 6 years ago by
I think it's possible that the code in pynac.pxd
was written because at the time there was no std::multiset
support in Cython.
comment:48 in reply to: ↑ 46 Changed 6 years ago by
Replying to rws:
Replying to fbissey:
I am not going to fight that fight. If a reviewer think they should not go, fine with me. Still needs some testing on normal setups. Not expecting problems but we never know.
I see no problem because in
ginac/fderivative.h
typedef std::multiset<unsigned> paramset;this is just a standard container with a standard iterator.
and this is why the 1st chunk of the patch :
cdef GParamSetIter itr = s.begin() res = []  while itr.is_not_equal(s.end()): + while itr != s.end(): res.append(itr.obj()) itr.inc() return res
is perfectly fine with me,
but the other ones are about different containers, and it seems that for them
operator!=
implementations are provided, and are not totally trivial
(see my comment 39 above).
comment:49 in reply to: ↑ 41 ; followup: ↓ 50 Changed 6 years ago by
Replying to dimpase:
Cython converts
is_not_equal
intooperator!=
. By replacingis_not_equal
with!=
in cython files, we in effect replacing whateveroperator!=
implementation is picked by the C++ compiler with a "trivial" implementation
...if no custom operator !=
is provided, which is exactly what you want. If a custom operator !=
operator is provided, then C++ will use that for !=
. Writing operator!=
is more explicit than !=
but it means the same thing.
comment:50 in reply to: ↑ 49 ; followup: ↓ 51 Changed 6 years ago by
Replying to jdemeyer:
Replying to dimpase:
Cython converts
is_not_equal
intooperator!=
. By replacingis_not_equal
with!=
in cython files, we in effect replacing whateveroperator!=
implementation is picked by the C++ compiler with a "trivial" implementation...if no custom operator
!=
is provided, which is exactly what you want. If a custom operator!=
operator is provided, then C++ will use that for!=
. Writingoperator!=
is more explicit than!=
but it means the same thing.
it seems to me that for some is_not_equal
implementations of operator!=
are provided in pynac, and are picked up by gcc, but not by clang@osx.
comment:51 in reply to: ↑ 50 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
Replying to dimpase:
it seems to me that for some
is_not_equal
implementations ofoperator!=
are provided in pynac, and are picked up by gcc, but not by clang@osx.
It's in the C++ standard library, not Pynac. The fix makes sense to me. It could be that the C++ standard library on OS X is different, but that shouldn't matter here.
comment:53 Changed 6 years ago by
Honestly, I only typed, so you are very welcome.
comment:54 followup: ↓ 56 Changed 6 years ago by
On OS X, I get a failure building the Sage library:
[sagelib7.5.beta3] 1 warning generated. [sagelib7.5.beta3] clang++ bundle undefined dynamic_lookup L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib Wl,rpath,/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib Wl,rpath,/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib build/temp.macosx10.9x86_642.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/libs/ppl.o build/temp.macosx10.9x86_642.7/sage/libs/ppl_shim.o L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib lppl lgmp lgmpxx lm lstdc++ o build/lib.macosx10.9x86_642.7/sage/libs/ppl.so lpari Ddummy [sagelib7.5.beta3] [ 6/308] clang fnostrictaliasing I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/var/tmp/sage/build/python22.7.10.p3/include DNDEBUG g fwrapv O3 Wall Wnounused I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include/python2.7 I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib/python2.7/sitepackages/numpy/core/include I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/sage/ext I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/ext I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include/python2.7 c /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/libs/pynac/constant.c o build/temp.macosx10.9x86_642.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/libs/pynac/constant.o std=c++11 fnostrictaliasing [sagelib7.5.beta3] error: invalid argument 'std=c++11' not allowed with 'C/ObjC' [sagelib7.5.beta3] error: command 'clang' failed with exit status 1 [sagelib7.5.beta3] [ 7/308] clang fnostrictaliasing I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/var/tmp/sage/build/python22.7.10.p3/include DNDEBUG g fwrapv O3 Wall Wnounused I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include/python2.7 I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/lib/python2.7/sitepackages/numpy/core/include I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/sage/ext I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/ext I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/local/include/python2.7 c /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/libs/pynac/pynac.c o build/temp.macosx10.9x86_642.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage7.5.beta4/src/build/cythonized/sage/libs/pynac/pynac.o std=c++11 fnostrictaliasing [sagelib7.5.beta3] error: invalid argument 'std=c++11' not allowed with 'C/ObjC' [sagelib7.5.beta3] make[3]: *** [sage] Error 1 [sagelib7.5.beta3] [sagelib7.5.beta3] real 0m5.450s [sagelib7.5.beta3] user 0m4.112s [sagelib7.5.beta3] sys 0m0.925s make[2]: *** [sagelib] Error 2 make[1]: *** [all] Error 2 real 0m6.149s user 0m4.477s sys 0m1.067s *************************************************************** Error building Sage. The following package(s) may have failed to build (not necessarily during this run of 'make all'): The build directory may contain configuration files and other potentially helpful information. WARNING: if you now run 'make' again, the build directory will, by default, be deleted. Set the environment variable SAGE_KEEP_BUILT_SPKGS to 'yes' to prevent this. make: *** [all] Error 1
comment:55 Changed 6 years ago by
This is after making the changes

src/bin/sageenv
diff git a/src/bin/sageenv b/src/bin/sageenv index 54c0783..fa8c557 100644
a b if [ "$UNAME" = "Darwin" ]; then 528 528 fi 529 529 530 530 # Override CC, CPP, CXX, FC if the GCC spkg was installed. 531 if [ x "$SAGE_LOCAL/bin/gcc" ]; then532 CC=gcc533 fi534 if [ x "$SAGE_LOCAL/bin/cpp" ]; then535 CPP=cpp536 fi537 if [ x "$SAGE_LOCAL/bin/g++" ]; then538 CXX=g++539 fi531 # if [ x "$SAGE_LOCAL/bin/gcc" ]; then 532 # CC=gcc 533 # fi 534 # if [ x "$SAGE_LOCAL/bin/cpp" ]; then 535 # CPP=cpp 536 # fi 537 # if [ x "$SAGE_LOCAL/bin/g++" ]; then 538 # CXX=g++ 539 # fi 540 540 if [ x "$SAGE_LOCAL/bin/gfortran" ]; then 541 541 FC=gfortran 542 542 fi 
src/sage/libs/pynac/pynac.pxd
diff git a/src/sage/libs/pynac/pynac.pxd b/src/sage/libs/pynac/pynac.pxd index 9b3ecfb..da5d447 100644
a b 1 # distutils: language = c++2 1 # distutils: libraries = pynac gmp 3 2 # distutils: extra_compile_args = std=c++11 4 3 """
comment:56 in reply to: ↑ 54 Changed 6 years ago by
Replying to jhpalmieri:
On OS X, I get a failure building the Sage library:
Are you still on OSX 10.9 ? (and on Xcode 6 ?) Well, no wonder it might not work.
comment:57 followup: ↓ 58 Changed 6 years ago by
This is on OS X 10.12, Xcode 8.1.
comment:58 in reply to: ↑ 57 ; followup: ↓ 60 Changed 6 years ago by
Replying to jhpalmieri:
This is on OS X 10.12, Xcode 8.1.
I was confused by macosx10.9x86_642.7
string in your log...
Anyway, I think I know where this comes from: you probably need
diff git a/src/sage/libs/m4ri.pxd b/src/sage/libs/m4ri.pxd index a1fe594..ea07121 100644  a/src/sage/libs/m4ri.pxd +++ b/src/sage/libs/m4ri.pxd @@ 1,4 +1,3 @@ # distutils: extra_compile_args = std=c99 cdef extern from "m4ri/m4ri.h": ctypedef int rci_t
comment:59 Changed 6 years ago by
My fault I said to edit pynac.pxd
instead of m4ri.pxd
I was busy with something else at the time.
comment:60 in reply to: ↑ 58 Changed 6 years ago by
Replying to dimpase:
Replying to jhpalmieri:
This is on OS X 10.12, Xcode 8.1.
I was confused by
macosx10.9x86_642.7
string in your log...Anyway, I think I know where this comes from: you probably need
diff git a/src/sage/libs/m4ri.pxd b/src/sage/libs/m4ri.pxd index a1fe594..ea07121 100644  a/src/sage/libs/m4ri.pxd +++ b/src/sage/libs/m4ri.pxd @@ 1,4 +1,3 @@ # distutils: extra_compile_args = std=c99 cdef extern from "m4ri/m4ri.h": ctypedef int rci_t
Right, so, and, as Francois says, put back # distutils: language = c++
into src/sage/libs/pynac/pynac.pxd
comment:61 followup: ↓ 62 Changed 6 years ago by
The macos10.9...
may come from these lines in sageenv
:
# Mac OS Xspecific setup if [ "$UNAME" = "Darwin" ]; then # set MACOSX_DEPLOYMENT_TARGET  if set incorrectly, this can # cause lots of random "Abort trap" issues on OSX. see trac # #7095 for an example. MACOSX_VERSION=`uname r  awk F. '{print $1}'` if [ $MACOSX_VERSION ge 14 ]; then # various packages have still have issues with # two digit OS X versions MACOSX_DEPLOYMENT_TARGET=10.9 else MACOSX_DEPLOYMENT_TARGET=10.$[$MACOSX_VERSION4] fi export MACOSX_DEPLOYMENT_TARGET MACOSX_VERSION fi
comment:62 in reply to: ↑ 61 Changed 6 years ago by
Replying to jhpalmieri: But this sounds alarming, to have such a version mismatch, as you do set it incorrectly...
# Mac OS Xspecific setup if [ "$UNAME" = "Darwin" ]; then # set MACOSX_DEPLOYMENT_TARGET  if set incorrectly, this can # cause lots of random "Abort trap" issues on OSX.
comment:63 Changed 6 years ago by
I plan to test what happens if we remove those lines and just set MACOSX_DEPLOYMENT_TARGET
correctly. It may take me a few days to get to it, though.
comment:64 Changed 6 years ago by
MACOSX_DEPLOYMENT_TARGET
was set that way on purpose some time ago. That makes binaries backward compatible for example. There is nothing wrong with it in itself. And at the time there were difficulties for putting it at 10.10
(because two digits instead of one). By any means we can see what happens when we raise it.
comment:65 Changed 6 years ago by
 Branch changed from u/fbissey/pynacclang to 86b2c2d09f0b907e10a2557b68a61d14305bb52a
 Resolution set to fixed
 Status changed from positive_review to closed
Do you have a clue Ralph? I have a feeling this will require some work
pynac
side rather thansage
side, but I don't mix very well withc++
.