search

Martinsos / edlib

Lightweight, super fast C/C++ (& Python) library for sequence alignment using edit (Levenshtein) distance.
http://martinsos.github.io/edlib
MIT License
493 stars 162 forks source link

Fails to build on architectures where char is unsigned #126

Closed tillea closed 5 years ago

tillea commented 5 years ago

Hi, there is a bug report against the Debian packaged version about the fact that the code fails to build on architectures where char is unsigned.

./test/runTests.cpp: In function 'bool test11()':
./test/runTests.cpp:415:62: error: narrowing conversion of '-127' from 'int' to 'char' inside { } [-Wnarrowing]
     char query[8] =  {-127, 127, -55, 0, 42, 0,      127, -55};
                                                              ^
./test/runTests.cpp:415:62: error: narrowing conversion of '-55' from 'int' to 'char' inside { } [-Wnarrowing]
./test/runTests.cpp:415:62: error: narrowing conversion of '-55' from 'int' to 'char' inside { } [-Wnarrowing]
./test/runTests.cpp:416:62: error: narrowing conversion of '-127' from 'int' to 'char' inside { } [-Wnarrowing]
     char target[8] = {-127, 127,      0, 42, 0, -55, 127,  42};
                                                              ^
./test/runTests.cpp:416:62: error: narrowing conversion of '-55' from 'int' to 'char' inside { } [-Wnarrowing]

You can find a full build log at arm64 in case you need more information (please see end of the log). Kind regards, Andreas.

tillea commented 5 years ago

Hi again, for the moment I used this patch inside the Debian package which solves the issue but I'm not sure in how far you really need signed characters.

Martinsos commented 5 years ago

Hi @tillea , thank you for letting me know!

No, actually there is no requirement on char's being signed, unsigned chars would also work well.

I went with just char since that is the format I was expecting most people to already have their inputs in, so that they don't have to do conversions.

I assumed that char is unsigned, but I did a quick reading thanks to you and see that that is not guaranteed. Therefore, I modified the tests so that we are not assuming char to be signed and now you should not have that problem any more! I pushed the changes to master.

Martinsos commented 5 years ago

I am closing this issue for now, but please let me know if there is any more problems and I will reopen it.