Re: Help needed to fix gcc 4.7 bug in jellyfish package

[Michael Wild <themiwi@gmail.com>]

On 05/02/2012 08:33 AM, Andreas Tille wrote:
> Hi,
> 
> I tried to fix the problem in the jellyfish package but the general
> hints given did not helped me really.  Any more precise help to fix
> this problem:
> 
> parse_dna.cc:97:3: error: narrowing conversion of '-3' from 'int' to 'const uint_t {aka const long unsigned int}' inside { } is ill-formed in C++11 [-Werror=narrowing]
> 
> My first idea was to do
> 
> --- jellyfish.orig/jellyfish/parse_dna.cc
> +++ jellyfish/jellyfish/parse_dna.cc
> @@ -57,7 +57,7 @@
>      }
>    }
> 
> -  const uint_t parse_dna::codes[256] = {
> +  const int parse_dna::codes[256] = {
>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -2, -3, -3, -3, -3, -3,
>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3,
>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -1, -3, -3,
> --- jellyfish.orig/jellyfish/parse_dna.hpp
> +++ jellyfish/jellyfish/parse_dna.hpp
> @@ -55,7 +55,7 @@
>      static uint64_t mer_string_to_binary(const char *in, uint_t klen) {
>        uint64_t res = 0;
>        for(uint_t i = 0; i < klen; i++) {
> -        const uint_t c = parse_dna::codes[(uint_t)*in++];
> +        const int c = parse_dna::codes[(int)*in++];
>          if(c & CODE_NOT_DNA)
>            return 0;
>          res = (res << 2) | c;
> 
> 
> because it makes no sense to initialise uint with negative numbers but
> this did not changed the error message which sounds totally strange to
> me.
> 
> Kind regards
> 
>         Andreas.
> 

You missed the declaration of parse_dna::codes in parse_dna.hpp.


diff --git a/jellyfish/parse_dna.cc b/jellyfish/parse_dna.cc
index ab3ec64..9ea5ae1 100644
--- a/jellyfish/parse_dna.cc
+++ b/jellyfish/parse_dna.cc
@@ -57,7 +57,7 @@ namespace jellyfish {
     }
   }

-  const uint_t parse_dna::codes[256] = {
+  const int parse_dna::codes[256] = {
     -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -2, -3, -3, -3, -3, -3,
     -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3,
     -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -1, -3, -3,
diff --git a/jellyfish/parse_dna.hpp b/jellyfish/parse_dna.hpp
index 0435ae2..7ef8afd 100644
--- a/jellyfish/parse_dna.hpp
+++ b/jellyfish/parse_dna.hpp
@@ -46,7 +46,7 @@ namespace jellyfish {
      * '\n': map to -2. ignore
      * Other ASCII: map to -3. Skip to next line
      */
-    static const uint_t codes[256];
+    static const int codes[256];
     static const uint_t CODE_RESET = -1;
     static const uint_t CODE_IGNORE = -2;
     static const uint_t CODE_COMMENT = -3;
@@ -55,7 +55,7 @@ namespace jellyfish {
     static uint64_t mer_string_to_binary(const char *in, uint_t klen) {
       uint64_t res = 0;
       for(uint_t i = 0; i < klen; i++) {
-        const uint_t c = parse_dna::codes[(uint_t)*in++];
+        const int c = parse_dna::codes[(uint_t)*in++];
         if(c & CODE_NOT_DNA)
           return 0;
         res = (res << 2) | c;

That said, assigning -3 to an unsigned int seems to be a pretty
conscious choice to me, so it might have been done on purpose to create
a wrap-around. Also, the same pattern shows up many other places (e.g.
parse_dna::CODE_RESET, parse_dna::CODE_IGNORE, ...). IMHO bad practice,
but plausible. Probably it's best to contact upstream about this and ask
what their original intention was.

Michael