Opened 3 years ago

Closed 3 years ago

#24696 closed defect (fixed)

giac fails to compile with clang-3.8 on OpenSuSE

Reported by: rws Owned by:
Priority: blocker Milestone: sage-8.2
Component: packages: standard Keywords:
Cc: frederichan Merged in:
Authors: Ralf Stephan Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 0c38a83 (Commits) Commit: 0c38a837152312c3b2c79f287812805dd91673df
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

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

Attachments (1)

giac-1.4.9.45.p1.log (29.8 KB) - added by rws 3 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Priority changed from major to blocker

Changed 3 years ago by rws

comment:2 Changed 3 years ago by rws

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

comment:3 Changed 3 years ago by rws

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 ;  

comment:4 Changed 3 years ago by jdemeyer

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

comment:5 Changed 3 years ago by rws

  • Branch set to u/rws/giac_fails_to_compile_with_clang_3_8_on_opensuse

comment:6 Changed 3 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to 0c38a837152312c3b2c79f287812805dd91673df
  • Status changed from new to needs_review

Should be tested on other clang systems.


New commits:

0c38a8324696: fix hash_map logic

comment:7 Changed 3 years ago by jdemeyer

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

comment:8 follow-up: Changed 3 years ago by fbissey

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.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 3 years ago by rws

  • Status changed from needs_review to positive_review

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

Replying to fbissey:

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?

comment:10 Changed 3 years ago by fbissey

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 https://github.com/BRiAl/BRiAl/commit/510694f702ae074288e2c3fda16fbf49dabb9c36#diff-67e997bcfdac55191033d57a16d1408a

comment:11 in reply to: ↑ 9 ; follow-up: Changed 3 years ago by dimpase

Replying to rws:

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

Replying to fbissey:

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?

comment:12 in reply to: ↑ 11 Changed 3 years ago by rws

Replying to dimpase:

Replying to rws:

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

comment:13 Changed 3 years ago by rws

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.

comment:14 follow-up: Changed 3 years ago by fbissey

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?

comment:15 in reply to: ↑ 14 Changed 3 years ago by rws

Replying to fbissey:

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

comment:16 Changed 3 years ago by rws

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

comment:17 Changed 3 years ago by fbissey

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

comment:18 Changed 3 years ago by frederichan

comment:19 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name...

comment:20 Changed 3 years ago by fbissey

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_work to positive_review

comment:21 Changed 3 years ago by rws

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.

comment:22 Changed 3 years ago by frederichan

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

comment:23 Changed 3 years ago by vbraun

  • Branch changed from u/rws/giac_fails_to_compile_with_clang_3_8_on_opensuse to 0c38a837152312c3b2c79f287812805dd91673df
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.