fix implicit 64 to 32 bit conversion warnings

[antond-weta]

Description

Enabling the implicit 64 to 32 bit conversion warnings shows several hundreds of them. This is my attempt to fix them all.

Tests

I have built the code on an Apple silicon Mac (both command line and Xcode), and an x64 linux machine, and have run the tests. I have not tested on Windows, or any 32-bit hardware.

I'm not familiar with the project enough to say that my testing was adequate. Any help would be appreciated.

Checklist:

• [x] I have read the contribution guidelines.
• [x] I have updated the documentation, if applicable.
• [ ] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• [ ] If I added or modified a C++ API call, I have also amended the corresponding Python bindings (and if altering ImageBufAlgo functions, also exposed the new functionality as oiiotool options).
• [x] My code follows the prevailing code style of this project. If I haven't already run clang-format before submitting, I definitely will look at the CI test that runs clang-format and fix anything that it highlights as being nonconforming.

[lgritz]

I'm worried that this is a rathole that's just going to waste a lot of Anton's time and will touch lots of things that have been working for a long time, possibly introducing subtle bugs if we aren't very careful.

Before continuing to fix these individually, let's step back and think about a few things:

1. Why is this happening now? I feel like we've fixed this kind of warning before, any time the compilers detect them. Is xcode different? Is it doing the equivalent of -Wall by default? Is Apple silicon different because the definitions of long or long long have changed versus x86_64 or something of that nature?

2. Are the same warnings revealed on other platforms if we use -Wall? (This can be enabled from from the OIIO cmake build with -DEXTRA_WARNINGS=ON.)

3. Anton, before trying to actually fix any more of these, can you go back to the original code, build it (maybe with -DSTOP_ON_WARNING=OFF so it doesn't stop at the first one) and just paste the full warning report here or in an issue? Then let's just spend a little time looking it over and discussing what should really be fixed and how, what is a "false positive", and what should have pragmas to ignore the errors (and how local can we make those pragmas -- per file or even per line might be more targeted than turning the warnings off for the whole codebase).

[antond-weta]

Larry, doing Wall does not find anything new for me, the log is clean. The -DEXTRA_WARNINGS=ON does find a lot of stuff, like unused comments.

Xcode enables shorten-64-to-32 by default, which is, I assume, is not covered by Wall. I can reproduce the same behaviour by adding the option to the cmake config, see attached. Oddly, Xcode disables sign-compare by default. Both are worth having enabled in my opinion.

Wshorten-64-to-32.log.zip

[lgritz]

Sorry, my bad. We always enable -Wall. What I meant is -Wextra, which is turned on by the EXTRA_WARNINGS cmake variable. I guess "all" isn't really "all"!

Here's what I think we should do:

• Let's take our hands off of this jumbled PR for the moment.

• If EXTRA_WARNINGS doesn't catch all of these (i.e., if -Wextra doesn't implicitly include shorten-64-to-32, then let's add -Wshorten-64-to-32 to the clause here so that we have a way to enable it easily in the build, optionally.

• Let's then attack the warnings bit by bit, not all in one PR, but go one area or issue at a time of some related warnings (such as, just the ones in the TIFF plugin as one batch) so we can get consensus on the right way to solve it. For some of them, such as ones that touch public header files and may affect downstream projects, we should proceed very cautiously.

[antond-weta]

Sounds good.

Larry, while we are looking at warnings, could you check this one. Shouldn't there be logical and, not bitwise? Am I missing something? https://github.com/OpenImageIO/oiio/blob/56c734a6752d2a03af3455c50967e1f7a3835d92/src/libtexture/texturesys.cpp#L2349-L2358

[lgritz]

That one is intentionally bitwise, for performance reasons. (&& is short-circuited, i.e. there is an implied branch.) The pragma immediately before that is to disable the warning because we really do want this the way we wrote it.

[lgritz]

By the way, a lot of what you did on this PR is just fine. Part of asking to step back and turn them into smaller PRs is to cleave off the good ones from the ones that require some more thought. For example, all the ones of this nature:

    int numImg = m_images.size();

changing to size_t, those are fine and good to change since obviously vector::size() returns size_t and so it's weird that we ever assigned that to an int.