Results 1 to 14 of 14

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

  1. #1
    Member
    Join Date
    Jun 2008
    Location
    USA
    Posts
    111
    Thanks
    0
    Thanked 0 Times in 0 Posts

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

    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.

    Code:
    #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!!

  2. #2
    Member
    Join Date
    Aug 2008
    Location
    Saint Petersburg, Russia
    Posts
    215
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Just a thought: how about calculating
    Code:
    ((c4&0xff) < 16), ((c4&0xff) == 0xff)
    ?

  3. #3
    Programmer Jan Ondrus's Avatar
    Join Date
    Sep 2008
    Location
    Rychnov nad Kněžnou, Czech Republic
    Posts
    278
    Thanks
    33
    Thanked 137 Times in 49 Posts
    Quote Originally Posted by Rugxulo View Post
    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:
    Code:
    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;
        }
    ->
    Code:
    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:
    Code:
     const int chn=((buf0>>24)==0x36?6:(((buf0>>24)==0x38 || buf0&0xff==0x38)?8:4));
    ->
    Code:
     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
    Last edited by Jan Ondrus; 16th August 2009 at 14:39.

  4. #4
    Programmer Jan Ondrus's Avatar
    Join Date
    Sep 2008
    Location
    Rychnov nad Kněžnou, Czech Republic
    Posts
    278
    Thanks
    33
    Thanked 137 Times in 49 Posts
    paq8px_v63
    -additionaly -Wparentheses warnings removed
    "g++ -Wall paq8px_v63.cpp paq7asm.obj 2>warning"
    produces no warning in GCC 4.4.0 now.

  5. #5
    Member
    Join Date
    Jun 2008
    Location
    USA
    Posts
    111
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by Jan Ondrus View Post
    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

  6. #6
    Programmer Jan Ondrus's Avatar
    Join Date
    Sep 2008
    Location
    Rychnov nad Kněžnou, Czech Republic
    Posts
    278
    Thanks
    33
    Thanked 137 Times in 49 Posts
    Quote Originally Posted by Rugxulo View Post
    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

  7. #7
    Expert
    Matt Mahoney's Avatar
    Join Date
    May 2008
    Location
    Melbourne, Florida, USA
    Posts
    3,255
    Thanks
    306
    Thanked 779 Times in 486 Posts
    Quote Originally Posted by Rugxulo View Post
    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 ==

  8. #8
    Member
    Join Date
    Jun 2008
    Location
    USA
    Posts
    111
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by Matt Mahoney View Post
    It's right. & has lower precedence than < and ==
    paq8l.cpp, line 1888:
    Code:
          else if( c4&0xff < 16 ) fl = 5;
    Notice the whitespace? It seems clear that somebody thought otherwise.

  9. #9
    Expert
    Matt Mahoney's Avatar
    Join Date
    May 2008
    Location
    Melbourne, Florida, USA
    Posts
    3,255
    Thanks
    306
    Thanked 779 Times in 486 Posts
    Compiler doesn't notice whitespace.

  10. #10
    Tester
    Black_Fox's Avatar
    Join Date
    May 2008
    Location
    [CZE] Czechia
    Posts
    471
    Thanks
    26
    Thanked 9 Times in 8 Posts
    parentheses > whitespace
    I am... Black_Fox... my discontinued benchmark
    "No one involved in computers would ever say that a certain amount of memory is enough for all time? I keep bumping into that silly quotation attributed to me that says 640K of memory is enough. There's never a citation; the quotation just floats like a rumor, repeated again and again." -- Bill Gates

  11. #11
    Programmer schnaader's Avatar
    Join Date
    May 2008
    Location
    Hessen, Germany
    Posts
    566
    Thanks
    217
    Thanked 200 Times in 93 Posts
    Quote Originally Posted by Matt Mahoney View Post
    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.
    http://schnaader.info
    Damn kids. They're all alike.

  12. #12
    Expert
    Matt Mahoney's Avatar
    Join Date
    May 2008
    Location
    Melbourne, Florida, USA
    Posts
    3,255
    Thanks
    306
    Thanked 779 Times in 486 Posts
    LMAO Who is going to be the first to port paq8px to Whitespace?

  13. #13
    Administrator Shelwien's Avatar
    Join Date
    May 2008
    Location
    Kharkov, Ukraine
    Posts
    3,375
    Thanks
    214
    Thanked 1,023 Times in 544 Posts

  14. #14
    Expert
    Matt Mahoney's Avatar
    Join Date
    May 2008
    Location
    Melbourne, Florida, USA
    Posts
    3,255
    Thanks
    306
    Thanked 779 Times in 486 Posts
    Nice try but it doesn't run

Similar Threads

  1. The lie of "The world is a globe"
    By Vacon in forum The Off-Topic Lounge
    Replies: 2
    Last Post: 14th December 2009, 16:58
  2. Addition of "thanks" button
    By IsName in forum The Off-Topic Lounge
    Replies: 5
    Last Post: 14th July 2008, 21:59
  3. LZ77 speed optimization, 2 mem accesses per "round"
    By Lasse Reinhold in forum Forum Archive
    Replies: 4
    Last Post: 11th June 2007, 22:53
  4. Freeware "Send To" interface for CCM and QUAD
    By LovePimple in forum Forum Archive
    Replies: 2
    Last Post: 20th March 2007, 18:22
  5. The "Nuff said" video!
    By encode in forum Forum Archive
    Replies: 5
    Last Post: 3rd January 2007, 23:11

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •