jts / sga

de novo sequence assembler using string graphs
http://genome.cshlp.org/content/22/3/549
237 stars 82 forks source link

Does not build using gcc 6 #106

Open tillea opened 8 years ago

tillea commented 8 years ago

Hi,

as you can read in the Debian bug report sga does not build when using gcc 6 due to more strict type checking:

https://bugs.debian.org/811664

It results in

g++ -DHAVE_CONFIG_H -I. -I.. -I../Bigraph -I../Thirdparty -Wdate-time -D_FORTIFY_SOURCE=2 -fopenmp -I/usr//include -I/usr//include/bamtools -Wall -Wextra -Wno-unknown-pragmas -O3 -c -o libutil_a-VariantIndex.o test -f 'VariantIndex.cpp' || echo './'VariantIndex.cpp ClusterReader.cpp: In member function 'bool ClusterReader::readCluster(ClusterRecord&)': ClusterReader.cpp:70:41: error: cannot convert 'std::basic_istream' to 'bool' in initialization bool good = getline(*m_pReader, line); ^

Makefile:722: recipe for target 'libutil_a-ClusterReader.o' failed make[3]: * [libutil_a-ClusterReader.o] Error 1 make[3]: * Waiting for unfinished jobs.... StdAlnTools.cpp: In function 'std::__cxx11::string StdAlnTools::expandCigar(const string&)': StdAlnTools.cpp:122:34: error: cannot convert 'std::basic_istream' to 'bool' in initialization bool success = parser >> code; ^~~~

Makefile:750: recipe for target 'libutil_a-StdAlnTools.o' failed

Please note that there might be more issues later in the build sequence.

Kind regards

      Andreas.
jts commented 8 years ago

Thanks for the report, I will look into this.

jts commented 8 years ago

I don't have easy access to GCC 6 right now, is this reproducible with stricter flags on GCC 4.9.x?

mateidavid commented 8 years ago

Wow- so gcc6 disallows implicit conversion of std::basic_istream to bool, meaning that this becomes illegal:

std::string s;
while (std::getline(std::cin, s)) process_line(s);

I haven't read about gcc6, but this seems a bit extreme. Can this be one of those things that might get tweaked before an official release?

tillea commented 8 years ago

jts: is this reproducible with stricter flags on GCC 4.9.x?

I do not think so. Otherwise the bug reporter would not have given explicit hints how to use gcc-6 from Debian experimental.

jts commented 8 years ago

Does this error occur if you pass the -std=c++03 flag to GCC 6?

SHuang-Broad commented 8 years ago

This is the same error as in issue #108 . Compiling with clang has the same issue.

mateidavid commented 8 years ago

As explained here: https://github.com/jts/sga/issues/108#issuecomment-178152772, an easy way to add -std=c++03 is to replace make with make CXXFLAGS="-O3 -std=c++03". Please let us know if it solves the problems.

Alternatively, where can we find a docker image with gcc-6.0?

mr-c commented 8 years ago

@mateidavid Here's a Debian unstable + gcc 6 docker image: https://hub.docker.com/r/biocrusoe/debian-gcc6/

docker pull biocrusoe/debian-gcc6
mateidavid commented 8 years ago

@mr-c: Thanks for stepping in. But it doesn't work yet, I'm getting:

$ docker pull biocrusoe/docker-gcc6
Using default tag: latest
Pulling repository docker.io/biocrusoe/docker-gcc6
Tag latest not found in repository docker.io/biocrusoe/docker-gcc6
mr-c commented 8 years ago

@mateidavid Whoops; I forgot to tag and push. I don't do this often :-) I also renamed the image (and updated my instructions above).

Does docker pull biocrusoe/debian-gcc6 work for you?

mateidavid commented 8 years ago

I got it now. I needed g++ not just gcc, and the other build requirements (zlib1g-dev, etc). But I got the Dockerfile and put them in myself, thanks! I can now see the OP error.

gerddie commented 8 years ago

Actually, you should also be able to reproduce the error by setting -std=c++11 with any compiler that is advanced enough to support it (AFAIK that includes g++-4.9).

@mateidavid The while loop should still work, but assignment to a bool variable has to be make explicit.

mateidavid commented 8 years ago

PR #110 fixes this (at least it did back when I wrote it).

gerddie commented 8 years ago

Great, I'll try to use it to fix the Debian package.

As a side note of personal taste I'd suggest using .good() or ! ... .fail() instead of the static_cast on the stream, because it would probably look more readable ...

tillea commented 8 years ago

Hi,

thanks for the hint. I'll integrate the patch into the Debian package.

Kind regards

   Andreas.

On Tue, Jul 05, 2016 at 06:32:30AM -0700, Matei David wrote:

PR #110 fixes this (at least it did back when I wrote it).


You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/jts/sga/issues/106#issuecomment-230478968

http://fam-tille.de

jts commented 8 years ago

I've merged @mateidavid's PR (thanks)