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: sage-7.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:

Status badges

Description

I am hitting the following error in compiling sagelib with clang

[sagelib-7.4.rc1] [1/9] clang -fno-strict-aliasing -I/Users/fbissey/build/sage/local/var/tmp/sage/build/python2-2.7.10.p3/include -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/fbissey/build/sage/local/lib/python2.7/site-packages/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/site-packages/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.macosx-10.9-x86_64-2.7/Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.o -std=c++11 -fno-strict-aliasing
[sagelib-7.4.rc1] In file included from /Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.cpp:336:
[sagelib-7.4.rc1] In file included from /Users/fbissey/build/sage/src/sage/symbolic/ginac_wrap.h:11:
[sagelib-7.4.rc1] In file included from /Users/fbissey/build/sage/local/include/pynac/ginac.h:38:
[sagelib-7.4.rc1] /Users/fbissey/build/sage/local/include/pynac/integral.h:44:5: warning: 'GiNaC::integral::evalf' hides overloaded virtual function [-Woverloaded-virtual]
[sagelib-7.4.rc1]         ex evalf(int level=0) const;
[sagelib-7.4.rc1]            ^
[sagelib-7.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)
[sagelib-7.4.rc1]         virtual ex evalf(int level = 0, PyObject* parent=nullptr) const;
[sagelib-7.4.rc1]                    ^
[sagelib-7.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 *>'
[sagelib-7.4.rc1]     __pyx_t_2 = (__pyx_v_itr.operator!=(__pyx_v_lstend) != 0);
[sagelib-7.4.rc1]                  ~~~~~~~~~~~ ^
[sagelib-7.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 *>'
[sagelib-7.4.rc1]     __pyx_t_3 = (__pyx_v_itr.operator!=(__pyx_v_found.end()) != 0);
[sagelib-7.4.rc1]                  ~~~~~~~~~~~ ^
[sagelib-7.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>'
[sagelib-7.4.rc1]     __pyx_t_3 = (__pyx_v_itr.operator!=(__pyx_v_sym_set.end()) != 0);
[sagelib-7.4.rc1]                  ~~~~~~~~~~~ ^
[sagelib-7.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 fbissey

Do you have a clue Ralph? I have a feeling this will require some work pynac side rather than sage side, but I don't mix very well with c++.

comment:2 Changed 6 years ago by rws

Can you try with clang -stdlib=libstdc++ ...?

comment:3 Changed 6 years ago by fbissey

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 fbissey

No luck. Difficult to force clang++ I had to go manual

Mirage:src fbissey$ clang++ -fno-strict-aliasing -I/Users/fbissey/build/sage/local/var/tmp/sage/build/python2-2.7.10.p3/include -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/fbissey/build/sage/local/lib/python2.7/site-packages/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/site-packages/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.macosx-10.9-x86_64-2.7/Users/fbissey/build/sage/src/build/cythonized/sage/symbolic/expression.o -std=c++11 -fno-strict-aliasing -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 non-static 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 non-static 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 non-static 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 [-ferror-limit=]
20 errors generated.

comment:5 Changed 6 years ago by fbissey

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 rws

Is this Mac or Linux? Please state in the description.

comment:7 Changed 6 years ago by fbissey

  • 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 follow-up: Changed 6 years ago by rws

  • 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/gcc-4.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 jdemeyer

Replying to rws:

As you can see from https://gcc.gnu.org/onlinedocs/gcc-4.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.

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 rws

Actually, using a command adapted from yours (clang-3.8.0):

clang -fno-strict-aliasing -I/home/ralf/sage/local/include/python2.7 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/home/ralf/sage/local/lib/python2.7/site-packages/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/site-packages/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 -fno-strict-aliasing

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
      [-Woverloaded-virtual]
        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 fbissey

Cannot dismiss that I guess. That would be a big blow on apple's part.

comment:12 Changed 6 years ago by fbissey

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 non-standard header or rely on some indirect inclusion. Digging...

comment:13 Changed 6 years ago by fbissey

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 fbissey

My understanding of some C++ matters is shaky at best. Does it make a difference that operator!= is a "non-member" function? http://en.cppreference.com/w/cpp/container/list

comment:15 Changed 6 years ago by rws

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/gcc-4.6.3/libstdc++/api/a01055_source.html

Does the exact equivalent exist in your stl_list.h?

comment:16 Changed 6 years ago by fbissey

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 rws

Yes.

comment:18 Changed 6 years ago by fbissey

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++ -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -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/site-packages/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.macosx-10.9-x86_64-2.7/Users/fbissey/build/sage/src/build/cythonized/sage/libs/pynac/pynac.o -std=c++11 -fno-strict-aliasing
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 [-Woverloaded-virtual]
        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 rws

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 rws

Ah no sorry, different warning, not flint.

comment:21 Changed 6 years ago by fbissey

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:22 Changed 6 years ago by dimpase

  • Cc mkoeppe added

C++ guru needed...

comment:23 Changed 6 years ago by dimpase

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 non-equality 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 fbissey

That appears to be effective, let's see how far I can go with it.

comment:25 Changed 6 years ago by fbissey

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 fbissey

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 fbissey

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 fbissey

Had to apply Dima's suggestion 3 times. We have compile!

comment:29 follow-up: Changed 6 years ago by rws

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 dimpase

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 jdemeyer

Ideally, all those cdef struct in pynac.pxd should be replaced by cdef cppclass.

comment:32 Changed 6 years ago by fbissey

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 cross-checked with a new build.

comment:33 follow-up: Changed 6 years ago by fbissey

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 dimpase

Replying to fbissey:

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?

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 follow-up: Changed 6 years ago by 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.

<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 dimpase

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 follow-up: Changed 6 years ago by 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.

comment:38 Changed 6 years ago by fbissey

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 dimpase

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 follow-up: Changed 6 years ago by 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 of is_not_equal. is_not_equal was only used in the places that caused clang to fail.

comment:41 in reply to: ↑ 40 ; follow-up: Changed 6 years ago by dimpase

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 of is_not_equal. is_not_equal was only used in the places that caused clang 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 follow-up: Changed 6 years ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/pynac-clang
  • 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:

86b2c2dConvert is_not_equal to != for clang

comment:43 follow-up: Changed 6 years ago by jhpalmieri

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 fbissey

Replying to jhpalmieri:

If I want to test this on OS X, do I just do export CC=clang and then make?

No. You also need this

diff --git a/src/bin/sage-env b/src/bin/sage-env
index 54c0783..e25081d 100644
--- a/src/bin/sage-env
+++ b/src/bin/sage-env
@@ -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 fbissey

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 ; follow-up: Changed 6 years ago by 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. Rather, the Pynac interface in pynac.pxd could be cleaned up more rigourously (in another ticket).

comment:47 Changed 6 years ago by rws

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 dimpase

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 ; follow-up: Changed 6 years ago by jdemeyer

Replying to dimpase:

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

...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 ; follow-up: Changed 6 years ago by dimpase

Replying to jdemeyer:

Replying to dimpase:

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

...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.

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 jdemeyer

  • 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 of operator!= 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:52 Changed 6 years ago by dimpase

  • Authors changed from François Bissey to François Bissey, Dima Pasechnik

claiming co-authorship ;-)

comment:53 Changed 6 years ago by fbissey

Honestly, I only typed, so you are very welcome.

comment:54 follow-up: Changed 6 years ago by jhpalmieri

On OS X, I get a failure building the Sage library:

[sagelib-7.5.beta3] 1 warning generated.
[sagelib-7.5.beta3] clang++ -bundle -undefined dynamic_lookup -L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib -Wl,-rpath,/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib -L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib -Wl,-rpath,/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib build/temp.macosx-10.9-x86_64-2.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/libs/ppl.o build/temp.macosx-10.9-x86_64-2.7/sage/libs/ppl_shim.o -L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib -L/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib -lppl -lgmp -lgmpxx -lm -lstdc++ -o build/lib.macosx-10.9-x86_64-2.7/sage/libs/ppl.so -lpari -Ddummy
[sagelib-7.5.beta3] [  6/308] clang -fno-strict-aliasing -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/var/tmp/sage/build/python2-2.7.10.p3/include -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include/python2.7 -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib/python2.7/site-packages/numpy/core/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/sage/ext -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/ext -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include/python2.7 -c /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/libs/pynac/constant.c -o build/temp.macosx-10.9-x86_64-2.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/libs/pynac/constant.o -std=c++11 -fno-strict-aliasing
[sagelib-7.5.beta3] error: invalid argument '-std=c++11' not allowed with 'C/ObjC'
[sagelib-7.5.beta3] error: command 'clang' failed with exit status 1
[sagelib-7.5.beta3] [  7/308] clang -fno-strict-aliasing -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/var/tmp/sage/build/python2-2.7.10.p3/include -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include/python2.7 -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/lib/python2.7/site-packages/numpy/core/include -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/sage/ext -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/ext -I/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/local/include/python2.7 -c /Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/libs/pynac/pynac.c -o build/temp.macosx-10.9-x86_64-2.7/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage-7.5.beta4/src/build/cythonized/sage/libs/pynac/pynac.o -std=c++11 -fno-strict-aliasing
[sagelib-7.5.beta3] error: invalid argument '-std=c++11' not allowed with 'C/ObjC'
[sagelib-7.5.beta3] make[3]: *** [sage] Error 1
[sagelib-7.5.beta3] 
[sagelib-7.5.beta3] real	0m5.450s
[sagelib-7.5.beta3] user	0m4.112s
[sagelib-7.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 jhpalmieri

This is after making the changes

  • src/bin/sage-env

    diff --git a/src/bin/sage-env b/src/bin/sage-env
    index 54c0783..fa8c557 100644
    a b if [ "$UNAME" = "Darwin" ]; then 
    528528fi
    529529
    530530# Override CC, CPP, CXX, FC if the GCC spkg was installed.
    531 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
     531# 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
    540540if [ -x "$SAGE_LOCAL/bin/gfortran" ]; then
    541541    FC=gfortran
    542542fi
  • 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++
    21# distutils: libraries = pynac gmp
    32# distutils: extra_compile_args = -std=c++11
    43"""

comment:56 in reply to: ↑ 54 Changed 6 years ago by dimpase

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 follow-up: Changed 6 years ago by jhpalmieri

This is on OS X 10.12, Xcode 8.1.

comment:58 in reply to: ↑ 57 ; follow-up: Changed 6 years ago by dimpase

Replying to jhpalmieri:

This is on OS X 10.12, Xcode 8.1.

I was confused by macosx-10.9-x86_64-2.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 fbissey

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 dimpase

Replying to dimpase:

Replying to jhpalmieri:

This is on OS X 10.12, Xcode 8.1.

I was confused by macosx-10.9-x86_64-2.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 follow-up: Changed 6 years ago by jhpalmieri

The macos-10.9... may come from these lines in sage-env:

# Mac OS X-specific 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_VERSION-4]
    fi
    export MACOSX_DEPLOYMENT_TARGET MACOSX_VERSION
fi

comment:62 in reply to: ↑ 61 Changed 6 years ago by dimpase

Replying to jhpalmieri: But this sounds alarming, to have such a version mismatch, as you do set it incorrectly...

# Mac OS X-specific 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 jhpalmieri

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 fbissey

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 vbraun

  • Branch changed from u/fbissey/pynac-clang to 86b2c2d09f0b907e10a2557b68a61d14305bb52a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.