PAQ8 C++ precedence bug (or "-Wparentheses is annoying")

[Rugxulo]

So I decided, despite my almost 100% lack of C or C++ skills, to try and remove all the warnings in paq8o8z from -Wall (which includes -Wparentheses by default since G++ 4.3.0). I've also noticed that paq8px v61 still has such things, so I thought it would benefit the PAQ8 community too. Of course, when I thought I was done, I forgot to test the other submodels (.EXE, .JPG, .BMP), so I had to restart again today.

First, let me say that there are two sites that have proved helpful (esp. the latter):

http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B
http://howaboutanorange.com/precedence/

Secondly, I was going to e-mail Matt Mahoney directly (Hi, Matt!). But he's often busy, doesn't "maintain" the PAQ8 series anymore, plus he reads here, and some of you (Jan) maintain your own hacks to PAQ8, so this will obviously interest you as well.

#include <stdio.h>

// shows a problem in PAQ8L.CPP and similar (e.g. line 1888)

int main() {

unsigned int c4 = 1;

  printf("\nc4 == %lu\n",c4);

  if ((c4&0xff) < 16)
      printf("(c4&0xff < 16) is '%lu', (c4&0xff == 0xff) "
             "is '%lu'\n", (c4&0xff < 16), (c4&0xff == 0xff));

  else if (c4&0xff < 16)
      printf("you will never see this");

  return 0;
}

And the results (GCC 4.4.1):

c4 == 1
(c4&0xff < 16) is '0', (c4&0xff == 0xff) is '1'

Now I'm the worst C/C++ coder in the world, but even I think that this can't be right!!

[nanoflooder]

Just a thought: how about calculating

((c4&0xff) < 16), ((c4&0xff) == 0xff)

?

[Jan Ondrus]

Secondly, I was going to e-mail Matt Mahoney directly (Hi, Matt!). But he's often busy, doesn't "maintain" the PAQ8 series anymore, plus he reads here, and some of you (Jan) maintain your own hacks to PAQ8, so this will obviously interest you as well.

paq8px_v62:

a) some double brackets removed

b) removed this warnings:

g++ -Wall paq8px_v61.cpp paq7asm.obj 2>warnings

paq8px_v61.cpp: In function 'int jpegModel(Mixer&)':
paq8px_v61.cpp:2510: warning: unused variable 'height'
paq8px_v61.cpp: In function 'Filetype detect(FILE*, int, Filetype, int&)':
paq8px_v61.cpp:3692: warning: unused variable 'ok'
paq8px_v61.cpp:3802: warning: comparison between signed and unsigned integer expressions
paq8px_v61.cpp:3849: warning: comparison between signed and unsigned integer expressions
paq8px_v61.cpp: In function 'void compressRecursive(FILE*, long int, Encoder&, char*, int, int, int)':
paq8px_v61.cpp:4106: warning: int format, long int arg (arg 5)
paq8px_v61.cpp:4106: warning: int format, long int arg (arg 6)
paq8px_v61.cpp:4129: warning: long int format, int arg (arg 2)

c) fixed brackets:

if (c4&0xff != 0) {
      if (isalpha(c4&0xff)) fl = 1;
      else if (ispunct(c4&0xff)) fl = 2;
      else if (isspace(c4&0xff)) fl = 3;
      else if (c4&0xff == 0xff) fl = 4;
      else if (c4&0xff < 16) fl = 5;
      else if (c4&0xff < 64) fl = 6;
      else fl = 7;
    }

->

if ((c4&0xff) != 0) {
      if (isalpha(c4&0xff)) fl = 1;
      else if (ispunct(c4&0xff)) fl = 2;
      else if (isspace(c4&0xff)) fl = 3;
      else if ((c4&0xff) == 0xff) fl = 4;
      else if ((c4&0xff) < 16) fl = 5;
      else if ((c4&0xff) < 64) fl = 6;
      else fl = 7;
    }

d) fixed brackets:

 const int chn=((buf0>>24)==0x36?6:(((buf0>>24)==0x38 || buf0&0xff==0x38)?8:4));

->

 const int chn=((buf0>>24)==0x36?6:(((buf0>>24)==0x38 || (buf0&0xff)==0x38)?8:4));

Note: Only c) or d) may affect compression ratio.

Quick test: (-6 level)

facompress.dll - 361984
facompress.dll.paq8px_v61 - 99917
facompress.dll.paq8px_v62 - 99738

[Jan Ondrus]

paq8px_v63
-additionaly -Wparentheses warnings removed
"g++ -Wall paq8px_v63.cpp paq7asm.obj 2>warning"
produces no warning in GCC 4.4.0 now.

[Rugxulo]

paq8px_v63
-additionaly -Wparentheses warnings removed
"g++ -Wall paq8px_v63.cpp paq7asm.obj 2>warning"
produces no warning in GCC 4.4.0 now.

MinGW 4.4.0:

g++ -Wall -DNOASM z:\mahoney\paq8px_v63.cpp -o p.exe

z:\mahoney\paq8px_v63.cpp: In function 'void train(short int*, short int*, int, int)':
z:\mahoney\paq8px_v63.cpp:1218: warning: suggest parentheses around '+' inside '>>'

Also, if you use -O or -O2 (uh ... what, it's already too fast for ya? heh), it whines some more:

z:\mahoney\paq8px_v63.cpp: In function 'void train(short int*, short int*, int, int)':
z:\mahoney\paq8px_v63.cpp:1218: warning: suggest parentheses around '+' inside '>>'
z:\mahoney\paq8px_v63.cpp: In member function 'U8* BH<B>:perator[](U32) [with int B = 4]':
z:\mahoney\paq8px_v63.cpp:1539: instantiated from here
z:\mahoney\paq8px_v63.cpp:1479: warning: dereferencing type-punned pointer will break strict-aliasing rules
z:\mahoney\paq8px_v63.cpp: In member function 'U8* BH<B>:perator[](U32) [with int B = 9]':
z:\mahoney\paq8px_v63.cpp:2742: instantiated from here
z:\mahoney\paq8px_v63.cpp:1479: warning: dereferencing type-punned pointer will break strict-aliasing rules
z:\mahoney\paq8px_v63.cpp: In function 'int decode_cd(FILE*, int, FILE*, FMode, int&)':
z:\mahoney\paq8px_v63.cpp:3914: warning: 'a' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3914: warning: 'bsize' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp: In function 'Filetype detect(FILE*, int, Filetype, int&)':
z:\mahoney\paq8px_v63.cpp:3586: warning: 'wavsize' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3586: warning: 'wavch' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3586: warning: 'wavbps' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3586: warning: 'wavm' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3587: warning: 'aiffm' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3587: warning: 'aiffs' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3588: warning: 's3mno' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3588: warning: 's3mni' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3589: warning: 'imgbpp' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3589: warning: 'bmpx' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3589: warning: 'bmpy' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3589: warning: 'bmpof' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3590: warning: 'rgbx' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3590: warning: 'rgby' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3591: warning: 'tgax' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3591: warning: 'tgay' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3591: warning: 'tgaz' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3591: warning: 'tgat' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3594: warning: 'cda' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3594: warning: 'cdm' may be used uninitialized in this function
z:\mahoney\paq8px_v63.cpp:3595: warning: 'cdf' may be used uninitialized in this function

[Jan Ondrus]

MinGW 4.4.0:

Also, if you use -O or -O2 (uh ... what, it's already too fast for ya? heh), it whines some more:

paq8px_v64:
- no warnings with -DNOASM -O2 switches

[Matt Mahoney]

Now I'm the worst C/C++ coder in the world, but even I think that this can't be right!!

It's right. & has lower precedence than < and ==

[Rugxulo]

It's right. & has lower precedence than < and ==

paq8l.cpp, line 1888:

      else if( c4&0xff < 16 ) fl = 5;

Notice the whitespace? It seems clear that somebody thought otherwise.

[Matt Mahoney]

Compiler doesn't notice whitespace.

[Black_Fox]

parentheses > whitespace

[schnaader]

Compiler doesn't notice whitespace.

A compiler for Whitespace does

But this is why I am using an "I'm using too much of them but I know I'm on the safe side with it" paranthesis style in C.

[Matt Mahoney]

LMAO Who is going to be the first to port paq8px to Whitespace?

[Shelwien]

What about this?
http://shelwien.googlepages.com/paq_ws_v0.rar

[Matt Mahoney]

Nice try but it doesn't run