Re: [I] [C++][Emscripten] Suppress shorten-64-to-32 warnings [arrow]

["kou (via GitHub)" <gi...@apache.org>]

kou commented on issue #38090:
URL: https://github.com/apache/arrow/issues/38090#issuecomment-1754099894

   Other discussions on ursalabs.zulipchat.com:
   
   https://ursalabs.zulipchat.com/#narrow/stream/180245-dev/topic/C.2B.2B.20and.20Emscripten.20warnings
   
   ```text
   Antoine
   23:28
   
   @kou You've opened lots of PRs to explicitly cast int64_t to size_t for Emscripten (where I assume pointers and sizes are 32 bits?). This will generally make the code less readable and more annoying to maintain. Can we simply disable that specific warning when the target is Emscripten?
     23:28
   
   cc @Benjamin Kietzman @Felipe Carvalho for opinions.
   ```
   
   ```text
   Benjamin Kietzman
   23:44
   
   I'd be in favor of disabling most of the integer conversion warnings. I'd prefer to use -fsanitize=implicit-signed-integer-truncation and similar checks, which would give us a chance to catch actual bugs instead of littering the code with boilerplate to explicitly silence such
   ```
   
   ```text
   Antoine
   23:48
   
   On the opposite, I think that integer conversion warnings are normally useful. But if you're on a 32-bit build, casting a size to 32 bits should be obviously safe (how would you have got a 64-bit size in the first place? except if reading the size from untrusted data, that is).
     23:50
   
   Sanitize checks would only be useful if the tests actually exercise sizes larger than 2GB, right? That's not the case usually, for good reason.
   ```
   
   ```text
   kou
   05:44
   
       where I assume pointers and sizes are 32 bits?
   
   Yes.
   
       Can we simply disable that specific warning when the target is Emscripten?
   
   Yes, we can.
   
   I don't have a strong opinion how to suppress this warning (adding explicit casts or disabling this warning).
   I share most warning reported cases for information:
   
       Use int64_t for std::vector::operator[]
       Use int64_t for std::vector::resize()
       Use int64_t for memset()
       Use int64_t for memcpy()
       Use int64_t for memcmp()
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org