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

giac fails to compile with clang-3.8 on OpenSuSE #24696

Closed
rwst opened this issue Feb 9, 2018 · 26 comments
Closed

giac fails to compile with clang-3.8 on OpenSuSE #24696

rwst opened this issue Feb 9, 2018 · 26 comments

Comments

@rwst
Copy link

rwst commented Feb 9, 2018

hash_map is not part of the C++ standard library. Some compilers like GCC implement it anyway. However clang does not have it, leading to compile failure with CC=clang on OpenSuSE Leap 42.3:

In file included from input_lexer.ll:48:
In file included from ./giacPCH.h:8:
./index.h:571:11: error: no template named 'hash_map' in namespace 'std'; did yo
u mean '__gnu_cxx::hash_map'?
  typedef std::hash_map< index_t,index_m,hash_function_object > hash_index ;
          ^~~~~~~~~~~~~
          __gnu_cxx::hash_map
/usr/bin/../lib64/gcc/x86_64-suse-linux/4.8/../../../../include/c++/4.8/backward
/hash_map:83:11: note: '__gnu_cxx::hash_map' declared here
    class hash_map
          ^

Instead unordered_map should be used.

See also https://stackoverflow.com/questions/5908581/is-hash-map-part-of-the-stl

CC: @frederichan-IMJPRG

Component: packages: standard

Author: Ralf Stephan

Branch/Commit: 0c38a83

Reviewer: Jeroen Demeyer

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

@rwst rwst added this to the sage-8.2 milestone Feb 9, 2018
@jdemeyer

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Feb 9, 2018

Attachment: giac-1.4.9.45.p1.log

@rwst
Copy link
Author

rwst commented Feb 9, 2018

comment:2

Actually giac has involved configure detection of hash_map/unordered_map presence which just fails here. Part of the log (attached):

checking for clock_gettime in -lrt... yes
checking unordered_map usability... no
checking unordered_map presence... no
checking for unordered_map... no
checking ext/hash_map usability... yes
checking ext/hash_map presence... yes
checking for ext/hash_map... yes
checking tr1/unordered_map usability... yes
checking tr1/unordered_map presence... yes
checking for tr1/unordered_map... yes
checking hash_map usability... yes
checking hash_map presence... yes
checking for hash_map... yes
checking pwd.h usability... yes

@rwst
Copy link
Author

rwst commented Feb 9, 2018

comment:3

This snippet in index.h:

#if defined UNORDERED_MAP  && !defined(__clang__) && !defined(VISUALC) // && !defined(__APPLE__)
#include <tr1/unordered_map>
#define HASH_MAP_NAMESPACE std::tr1
#define hash_map unordered_map
#else // UNORDERED_MAP

and the code in the same file giving the error:

  typedef HASH_MAP_NAMESPACE::hash_map< index_t,index_m,hash_function_object > hash_index ;  

@jdemeyer
Copy link

jdemeyer commented Feb 9, 2018

comment:4

These days, it would be better to just assume C++11 and use unordered_map.

@rwst
Copy link
Author

rwst commented Feb 9, 2018

@rwst
Copy link
Author

rwst commented Feb 9, 2018

Author: Ralf Stephan

@rwst
Copy link
Author

rwst commented Feb 9, 2018

comment:6

Should be tested on other clang systems.


New commits:

0c38a8324696: fix hash_map logic

@rwst
Copy link
Author

rwst commented Feb 9, 2018

Commit: 0c38a83

@jdemeyer
Copy link

jdemeyer commented Feb 9, 2018

comment:7

If this works for you, I'm willing to set this to positive_review.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2018

comment:8

I guess it wasn't seen because we haven't tested a clang that old for some time. Also I think your clang uses libstdc++ from gcc-4.8 while all we made a point of testing with clang's own libstdc++.
The fix looks OK but it is really stuff that should be tested in configure, with c++11 you don't need tr1 for unordered map, I battled that in brial.

@rwst
Copy link
Author

rwst commented Feb 10, 2018

comment:9

Jeroen, it works fine, I assume I can set positive?

Replying to @kiwifb:

I guess it wasn't seen because we haven't tested a clang that old for some time. Also I think your clang uses libstdc++ from gcc-4.8 while all we made a point of testing with clang's own libstdc++.

Right, they have no official clang libstdc++ package on OpenSuSE.

The fix looks OK but it is really stuff that should be tested in configure, with c++11 you don't need tr1 for unordered map, I battled that in brial.

Well, I think it's clearly a giac problem and the patch should be applied upstream. Could you please, @frederichan-IMJPRG?

@kiwifb
Copy link
Member

kiwifb commented Feb 10, 2018

comment:10

To be honest this patch is a band aid. A proper fix is to detect hash_map properly from configure rather applying pragma trying to find compilers you think will support such and such form of the feature. Something a bit like this BRiAl/BRiAl@510694f#diff-67e997bcfdac55191033d57a16d1408a

@dimpase
Copy link
Member

dimpase commented Feb 10, 2018

comment:11

Replying to @rwst:

Jeroen, it works fine, I assume I can set positive?

Replying to @kiwifb:

I guess it wasn't seen because we haven't tested a clang that old for some time. Also I think your clang uses libstdc++ from gcc-4.8 while all we made a point of testing with clang's own libstdc++.

Right, they have no official clang libstdc++ package on OpenSuSE.

they do have it: https://software.opensuse.org/package/libc++1

(libstdc++ is the name for the GNU c++ library)

The fix looks OK but it is really stuff that should be tested in configure, with c++11 you don't need tr1 for unordered map, I battled that in brial.

Well, I think it's clearly a giac problem and the patch should be applied upstream. Could you please, @frederichan-IMJPRG?

@rwst
Copy link
Author

rwst commented Feb 10, 2018

comment:12

Replying to @dimpase:

Replying to @rwst:

Right, they have no official clang libstdc++ package on OpenSuSE.

they do have it: https://software.opensuse.org/package/libc++1

(libstdc++ is the name for the GNU c++ library)

Maybe but the ones for Leap 42.3 are all unstable (not official in my reading).

@rwst
Copy link
Author

rwst commented Feb 10, 2018

comment:13

I installed clang-5 and libc++-5 but couldn't deinstall libstdc++ because even clang depends on it. With both libs installed sage -f giac still fails, so libc++ needs to be specified on build.

@kiwifb
Copy link
Member

kiwifb commented Feb 10, 2018

comment:14

Removing libstdc++ would destroy your g++ and everything in your system built with it. So it is kind of important. Try CXX="clang++ -stdlib=libc++". Provided that clang++ is the right name to use, would you have clang-5 and clang++-5 from those rpms?

@rwst
Copy link
Author

rwst commented Feb 10, 2018

comment:15

Replying to @kiwifb:

Removing libstdc++ would destroy your g++ and everything in your system built with it. So it is kind of important. Try CXX="clang++ -stdlib=libc++". Provided that clang++ is the right name to use, would you have clang-5 and clang++-5 from those rpms?

Yes everything up to clang++-5.0.0. Now I get

[giac-1.4.9.45.p1] /home/ralf/sage/local/var/tmp/sage/build/giac-1.4.9.45.p1/src/src/.libs/libgiac.so: undefined reference to `NTL::operator<<(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, NTL::GF2X const&)'
[giac-1.4.9.45.p1] /home/ralf/sage/local/var/tmp/sage/build/giac-1.4.9.45.p1/src/src/.libs/libgiac.so: undefined reference to `NTL::operator<<(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, NTL::ZZ_pX const&)'
[giac-1.4.9.45.p1] clang-5.0.0: error: linker command failed with exit code 1 (use -v to see invocation)
[giac-1.4.9.45.p1] Makefile:582: recipe for target 'xcas' failed
[giac-1.4.9.45.p1] make[4]: *** [xcas] Error 1

@rwst
Copy link
Author

rwst commented Feb 10, 2018

comment:16

Ah I should build NTL with that as well...

@kiwifb
Copy link
Member

kiwifb commented Feb 10, 2018

comment:17

You may want to make distclean libc++ and libstdc++ are not compatible.

@frederichan-IMJPRG
Copy link

comment:18

Hello, I am reporting this here:
http://xcas.e.ujf-grenoble.fr/XCAS/viewtopic.php?f=4&t=2010

@vbraun
Copy link
Member

vbraun commented Feb 11, 2018

comment:19

Reviewer name...

@kiwifb
Copy link
Member

kiwifb commented Feb 11, 2018

Reviewer: Jeroen Demeyer

@rwst
Copy link
Author

rwst commented Feb 12, 2018

comment:21

It looks like this is also necessary with clang-4.0.1 if no CXXFLAGS and LDFLAGS are set for libc++ but CLANG_DEFAULT_CXX_STDLIB=libc++ is set.

@frederichan-IMJPRG
Copy link

comment:22

NB: upstream is testing if there are no side effects on some other configurations:

http://xcas.e.ujf-grenoble.fr/XCAS/viewtopic.php?f=4&t=2010

@vbraun
Copy link
Member

vbraun commented Feb 13, 2018

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