Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiling sagelib with clang on OS X (Sierra): failure in cythonized sage/symbolic/expression.pyx #21701

Closed
kiwifb opened this issue Oct 14, 2016 · 69 comments

Comments

@kiwifb
Copy link
Member

kiwifb commented Oct 14, 2016

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.

CC: @rwst @mkoeppe

Component: porting

Author: François Bissey, Dima Pasechnik

Branch/Commit: 86b2c2d

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/21701

@kiwifb kiwifb added this to the sage-7.5 milestone Oct 14, 2016
@kiwifb
Copy link
Member Author

kiwifb commented Oct 14, 2016

comment:1

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

@rwst
Copy link

rwst commented Oct 14, 2016

comment:2

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

@kiwifb
Copy link
Member Author

kiwifb commented Oct 14, 2016

comment:3

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.

@kiwifb
Copy link
Member Author

kiwifb commented Oct 14, 2016

comment:4

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.

@kiwifb
Copy link
Member Author

kiwifb commented Oct 14, 2016

comment:5

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.

@rwst
Copy link

rwst commented Oct 14, 2016

comment:6

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

@kiwifb kiwifb changed the title Compiling sagelib with clang: failure in sage/symbolic/expression.pyx - pynac problem Compiling sagelib with clang on OS X (Sierra): failure in sage/symbolic/expression.pyx - pynac problem Oct 14, 2016
@rwst
Copy link

rwst commented Oct 14, 2016

comment:8

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.

@rwst rwst changed the title Compiling sagelib with clang on OS X (Sierra): failure in sage/symbolic/expression.pyx - pynac problem Compiling sagelib with clang on OS X (Sierra): failure in cythonized sage/symbolic/expression.pyx Oct 14, 2016
@jdemeyer
Copy link

comment:9

Replying to @rwst:

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?

@rwst
Copy link

rwst commented Oct 14, 2016

comment:10

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.

@kiwifb
Copy link
Member Author

kiwifb commented Oct 14, 2016

comment:11

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

@kiwifb
Copy link
Member Author

kiwifb commented Oct 15, 2016

comment:12

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

@kiwifb
Copy link
Member Author

kiwifb commented Oct 15, 2016

comment:13

Even manually adding #include <list> in the cpp file doesn't solve the problem. There must something subtle at work.

@kiwifb
Copy link
Member Author

kiwifb commented Oct 15, 2016

comment:14

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

@rwst
Copy link

rwst commented Oct 15, 2016

comment:15

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?

@kiwifb
Copy link
Member Author

kiwifb commented Oct 15, 2016

comment:16

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

@rwst
Copy link

rwst commented Oct 15, 2016

comment:17

Yes.

@kiwifb
Copy link
Member Author

kiwifb commented Nov 11, 2016

comment:18

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.

@rwst
Copy link

rwst commented Nov 11, 2016

comment:19

It's different. Before it was list, now it's tree. BTW the warning is fixed in flint git master.

@rwst
Copy link

rwst commented Nov 11, 2016

comment:20

Ah no sorry, different warning, not flint.

@kiwifb
Copy link
Member Author

kiwifb commented Nov 11, 2016

comment:21

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.

@dimpase
Copy link
Member

dimpase commented Nov 22, 2016

comment:22

C++ guru needed...

@dimpase
Copy link
Member

dimpase commented Nov 23, 2016

comment:23

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?

@kiwifb
Copy link
Member Author

kiwifb commented Nov 23, 2016

comment:24

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

@kiwifb
Copy link
Member Author

kiwifb commented Nov 24, 2016

comment:25

Had a new C/C++ conflict but I was still on 7.5.beta2. Switched to beta3 and rebuilding now.

@jhpalmieri
Copy link
Member

comment:43

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

@kiwifb
Copy link
Member Author

kiwifb commented Nov 25, 2016

comment:44

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

@kiwifb
Copy link
Member Author

kiwifb commented Nov 25, 2016

comment:45

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.

@rwst
Copy link

rwst commented Nov 25, 2016

comment:46

Replying to @kiwifb:

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

@rwst
Copy link

rwst commented Nov 25, 2016

comment:47

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.

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:48

Replying to @rwst:

Replying to @kiwifb:

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

@jdemeyer
Copy link

comment:49

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.

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:50

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.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

comment:51

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.

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:52

claiming co-authorship ;-)

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

Changed author from François Bissey to François Bissey, Dima Pasechnik

@kiwifb
Copy link
Member Author

kiwifb commented Nov 25, 2016

comment:53

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

@jhpalmieri
Copy link
Member

comment:54

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

@jhpalmieri
Copy link
Member

comment:55

This is after making the changes

diff --git a/src/bin/sage-env b/src/bin/sage-env
index 54c0783..fa8c557 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
diff --git a/src/sage/libs/pynac/pynac.pxd b/src/sage/libs/pynac/pynac.pxd
index 9b3ecfb..da5d447 100644
--- a/src/sage/libs/pynac/pynac.pxd
+++ b/src/sage/libs/pynac/pynac.pxd
@@ -1,4 +1,3 @@
-# distutils: language = c++
 # distutils: libraries = pynac gmp
 # distutils: extra_compile_args = -std=c++11
 """

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:56

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.

@jhpalmieri
Copy link
Member

comment:57

This is on OS X 10.12, Xcode 8.1.

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:58

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

@kiwifb
Copy link
Member Author

kiwifb commented Nov 25, 2016

comment:59

My fault I said to edit pynac.pxd instead of m4ri.pxd I was busy with something else at the time.

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:60

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

@jhpalmieri
Copy link
Member

comment:61

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

@dimpase
Copy link
Member

dimpase commented Nov 25, 2016

comment:62

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.

@jhpalmieri
Copy link
Member

comment:63

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.

@kiwifb
Copy link
Member Author

kiwifb commented Nov 25, 2016

comment:64

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.

@vbraun
Copy link
Member

vbraun commented Nov 27, 2016

Changed branch from u/fbissey/pynac-clang to 86b2c2d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants