Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang compilation warnings #165

Closed
yousry opened this issue Oct 6, 2015 · 6 comments
Closed

Clang compilation warnings #165

yousry opened this issue Oct 6, 2015 · 6 comments
Assignees

Comments

@yousry
Copy link

yousry commented Oct 6, 2015

libtool: compile:  clang++ -DHAVE_CONFIG_H -I. -I../config -I/Users/yousry/Xcode/IOS/root_armv7/include/OpenEXR -I.. -I../config -pipe -g -O2 -MT ImfDwaCompressor.lo -MD -MP -MF .deps/ImfDwaCompressor.Tpo -c ImfDwaCompressor.cpp -o ImfDwaCompressor.o
ImfDwaCompressor.cpp:2386:33: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
    if (unknownUncompressedSize < 0  || 
        ~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2387:31: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        unknownCompressedSize < 0    ||
        ~~~~~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2388:26: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        acCompressedSize < 0         || 
        ~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2389:26: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        dcCompressedSize < 0         ||
        ~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2390:27: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        rleCompressedSize < 0        || 
        ~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2391:29: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        rleUncompressedSize < 0      ||
        ~~~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2392:20: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        rleRawSize < 0               ||  
        ~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2393:34: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        totalAcUncompressedCount < 0 || 
        ~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2394:34: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        totalDcUncompressedCount < 0) 
        ~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2485:18: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
    if ((version < 0) || (version > 2))
         ~~~~~~~ ^ ~
ImfDwaCompressor.cpp:2499:37: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
        if (unknownUncompressedSize < 0 || 
            ~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
11 warnings generated.

and

libtool: compile:  clang++ -DHAVE_CONFIG_H -I. -I../config -I/Users/yousry/Xcode/IOS/root_armv7/include/OpenEXR -I.. -I../config -pipe -g -O2 -MT ImfScanLineInputFile.lo -MD -MP -MF .deps/ImfScanLineInputFile.Tpo -c ImfScanLineInputFile.cpp -o ImfScanLineInputFile.o
ImfScanLineInputFile.cpp:1408:22: warning: enumeration value 'NUM_PIXELTYPES' not handled in switch [-Wswitch]
              switch(i.channel().type)
                     ^
ImfScanLineInputFile.cpp:1478:22: warning: enumeration value 'NUM_PIXELTYPES' not handled in switch [-Wswitch]
              switch(i.channel().type)
                     ^
2 warnings generated.
@meshula
Copy link
Contributor

meshula commented Oct 10, 2015

The "not handled" warnings are in the annoying nanny category.

The signed comparison warnings should be addressed, the compressedSize variables are intended to be negative as an error condition, their type should be ssize_t.

@CAHEK7
Copy link
Contributor

CAHEK7 commented Feb 5, 2016

Signed comparison warnings happened due to confusing Int64 typedef declared in IlmBase/Imath/ImathInt64.h. Int64 declared as unsigned.

I would suggest to make it signed and define UInt64 as unsigned. If there are no strong objections I can do the changes. Also I can check another signed-unsigned issues, there are bunch of it if you turn on -Wsign-compare in gcc.

@meshula
Copy link
Contributor

meshula commented Feb 5, 2016

Good catch! ImathInt64 uses tricks that are no longer necessary; there is no longer a need to specialize per compiler.. If you make the change, please use <cstdint> and define the types as

typedef int64_t Int64;

@CAHEK7
Copy link
Contributor

CAHEK7 commented Feb 14, 2016

Have dug deeper and found out that a lot of code relies on Int64 unsigned nature and only some cases treat it as signed. And there are a lot of code which mixing signed and unsigned values. Proper fixing requires deep analysis and massive changes, but this particular case can be covered.
Also <cstdint> officially is C++11 feature, it can break portability (though it's supported by most compilers).

@meshula
Copy link
Contributor

meshula commented Feb 15, 2016

Should probably open another issue with your findings if you wouldn't mind. The ad hoc treatment of signed/unsigned is undesirable.

meshula added a commit that referenced this issue Apr 28, 2018
gatgui added a commit to gatgui/openexr that referenced this issue May 13, 2018
* commit '0cf11d74c27bd191f134ada90ac773a38860fbf9': (24 commits)
  initial port of PyIlmBase to python 3
  replicate configure / cmake changes from ilmbase
  add move constructor and assignment operator
  Fix Windows Python binding builds. Does not address PyImath runtime issues, but does allow build to succeed
  Fix c++11 detection issue on windows. Fix ilmbase DLL export warnings
  enable different c++ standards to be selected instead of just c++14
  Incorporate review feedback
  add compatibility std::condition_variable semaphore when posix semaphores not available
  fix error overwriting beginning of config file
  remove the dynamic exception for all versions of c++ unless FORCE_CXX03 is on
  ThreadPool improvements
  switch mutex to be based on std::mutex when available
  switch IlmThread to use c++11 threads when enabled
  use dynamic exception macro to avoid warnings in c++14 mode
  add #define to manage dynamic exception deprecation in c++11/14
  configuration changes to enable c++14
  Cmake now building OpenEXR successfully for Windows
  Delete build.log
  fix defect in semaphore implementation which caused application hang at exit time, because not all worker threads get woken up when task semaphore is repeatedly posted (to wake them up) after setting the stopping flag in the thread pool
  fix comparison of unsigned expression < 0 (Issue AcademySoftwareFoundation#165)
  ...
@cary-ilm cary-ilm added the C++ label Jun 13, 2019
@cary-ilm cary-ilm modified the milestones: Next Release, Needs Attention Jun 29, 2019
@kdt3rd
Copy link
Contributor

kdt3rd commented Jul 23, 2019

The code base in master now compiles cleanly with gcc 9 and clang 8 using -Wall

@kdt3rd kdt3rd closed this as completed Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants