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

Fixes clang++9 error in cram_io.h #1190

Closed
wants to merge 1 commit into from

Conversation

pjotrp
Copy link

@pjotrp pjotrp commented Dec 10, 2020

Even though it is sitting in an extern "C" block the clang++ compiler won't accept this void * assignment.

@pjotrp
Copy link
Author

pjotrp commented Dec 10, 2020

The full error was:

[9/104] Compiling C++ object 'freebayes@exe/src_DataLikelihood.cpp.o'.
FAILED: freebayes@exe/src_DataLikelihood.cpp.o
clang++ -Ifreebayes@exe -I. -I.. -I../src -I../bamtools/src/api -I../bamtools/src -I../ttmath -I../contrib -I../contrib/SeqLib -I../contrib/htslib_config -I../contrib/htslib/cram -I../contrib/htslib/htslib -I../contrib/htslib -I../vcflib/src -I../vcflib/tabixpp -I../vcflib/smithwaterman -I../vcflib/multichoose -I../vcflib/filevercmp -I/gnu/store/rykm237xkmq7rl1p0nwass01p090p88x-zlib-1.2.11/include -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Wpedantic -std=c++14 -g -pthread -fpermissive -w -MD -MQ 'freebayes@exe/src_DataLikelihood.cpp.o' -MF 'freebayes@exe/src_DataLikelihood.cpp.o.d' -o 'freebayes@exe/src_DataLikelihood.cpp.o' -c ../src/DataLikelihood.cpp
In file included from ../src/DataLikelihood.cpp:1:
In file included from ../src/DataLikelihood.h:20:
In file included from ../src/AlleleParser.h:30:
In file included from ../src/LeftAlign.h:99:
In file included from ../contrib/SeqLib/SeqLib/BamReader.h:6:
In file included from ../contrib/SeqLib/SeqLib/BamWalker.h:15:
In file included from ../contrib/htslib/cram/cram.h:48:
../contrib/htslib/cram/cram_io.h:513:20: error: cannot initialize a variable of type 'unsigned char *' with an rvalue of type 'void *'
    unsigned char *tmp = realloc(b->data, len);
                   ^     ~~~~~~~~~~~~~~~~~~~~~
1 error generated.

@jmarshall
Copy link
Member

That is indeed the only error from compiling the cram/*.h headers with a C++ compiler so it's probably worth applying this, but the real problem is that freebayes shouldn't be including these private headers. Freebayes should apply walaj/SeqLib#53 to its bundled SeqLib.

@pjotrp
Copy link
Author

pjotrp commented Dec 10, 2020

Thanks. I'll apply the patch to the bundled SeqLib files. I'll remove the implicit cram includes.

@pjotrp
Copy link
Author

pjotrp commented Dec 10, 2020

freebayes now compiles a stock htslib - no more bundled source. Above error may act like a canary in the coal mine ;)

@daviesrob
Copy link
Member

Thanks, closing as it appears to be no longer be a problem.

Please raise an issue if you find you still need something like this.

@daviesrob daviesrob closed this Dec 15, 2020
@niemasd
Copy link

niemasd commented Sep 29, 2023

I think I'm getting this issue on the following line of code when trying to compile htslib with clang++ in a Mac OS X environment:

https://github.com/samtools/htslib/blob/develop/cram/cram_io.h#L217

Here's the error:

https://dev.azure.com/bioconda/bioconda-recipes/_build/results?buildId=42286&view=logs&j=1b052f1d-4456-52f0-9d43-71c4c5bd734d&t=edc48dcd-1fc2-5e3b-7036-7be9cea00123&l=401

EDIT: Quick follow-up, the same fix seemed to have worked:

https://dev.azure.com/bioconda/bioconda-recipes/_build/results?buildId=42290&view=logs&j=1b052f1d-4456-52f0-9d43-71c4c5bd734d&t=edc48dcd-1fc2-5e3b-7036-7be9cea00123&l=286

Specifically, I just did the following before running make:

sed -i.bak 's/unsigned char \*tmp = realloc(b->data, len);/unsigned char \*tmp = (unsigned char \*)realloc(b->data, len);/g' htslib/cram/cram_io.h

which just replaces this:

unsigned char *tmp = realloc(b->data, len);

with this:

unsigned char *tmp = (unsigned char *)realloc(b->data, len);

@jkbonfield
Copy link
Contributor

The public header files should be capable of being included by C++ files, but the actual htslib library (both C and non-public header files) are in C, not C++. We don't make our C code swallowable by C++. That includes the internal cram/cram_io.h file.

Is there a reason you can't build htslib using its own supplied build system?

@jmarshall
Copy link
Member

This is bioconda/bioconda-recipes#43322, so the best answer is that @niemasd's recipe should compile against bioconda's packaged htslib instead of building htslib at all.

@daviesrob
Copy link
Member

I've made niemasd/ViralConsensus#9, which removes the use of CRAM internals from ViralConsensus.

@niemasd
Copy link

niemasd commented Sep 29, 2023

Ah, thank you so much, @daviesrob! I couldn't figure out how to use the local reference properly, and this was what I found when I dug through the source code of other tools that use htslib. I appreciate it!

I wonder if it makes sense to have a "How to properly include htslib in your code" guide? I tried looking through the htslib documentation, and I couldn't find anything. Even now that I know the solution, when I Google "hts_set_opt" "cram", I don't find any results that would have suggested this solution, whereas "Load a SAM/BAM/CRAM file and iterate over it" is a very common use-case

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

Successfully merging this pull request may close these issues.

5 participants