[Debian-med-packaging] Bug#667214: Help needed to fix gcc 4.7 bug in jellyfish package

[Andreas Tille]

Hi Guillaume,

in the Debian bug tracking system

   http://bugs.debian.org/667214

a build problem when using gcc-4.7 was reported.  Below you can read
some discussion about a possible fix.

Michael, I can confirm that I also tried to s/uint_t/int/ in
parse_dna.hpp with the same result (same error message) after I did my
initial posting (that's why I did not felt a real need to send another
mail).

And yes, I agree that assigning negative values to unsigned variables
smells like done with some purpose which might stay hidden from the
first look and I would have definitely asked upstream about any problem
my suggested patch might have caused.  I just felt like doing some
investigation into the problem might make sense.  However, it seems
that this does not lead to any progres.

Guillaume, could you enlighten us a bit about this trick?

Kind regards

        Andreas.

On Thu, May 03, 2012 at 09:52:09AM +0200, Michael Wild wrote:
> 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
> 
> 
> -- 
> To UNSUBSCRIBE, email to debian-mentors-REQUEST at lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact listmaster at lists.debian.org
> Archive: http://lists.debian.org/4FA23929.3000509@gmail.com
> 
> 

-- 
http://fam-tille.de