"implicit conversion loses integer precision" warnings

[hebasto]

When building with clang and the -Wshorten-64-to-32 flag, there are some warnings:

In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:27:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/field_impl.h:14:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/field_5x52_impl.h:13:
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:279:39: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  279 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:289:19: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  289 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:290:26: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  290 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:370:39: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  370 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:380:19: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  380 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:381:26: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  381 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~
In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:28:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/scalar_impl.h:20:
/home/hebasto/git/secp256k1/secp256k1/src/scalar_4x64_impl.h:119:42: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  119 |     overflow = secp256k1_u128_to_u64(&t) + secp256k1_scalar_check_overflow(r);
      |              ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/scalar_4x64_impl.h:682:34: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'unsigned int' [-Wshorten-64-to-32]
  682 |     secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r));
      |     ~~~~~~~~~~~~~~~~~~~~~~~    ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:30:
/home/hebasto/git/secp256k1/secp256k1/src/ecmult_impl.h:518:21: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  518 |     for (i = n_wnaf - 1; i >= 0; i--) {
      |            ~ ~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/ecmult_impl.h:562:52: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
  562 |         for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) {
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:32:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/ecmult_gen_impl.h:14:
/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:151:52: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  151 |     secp256k1_write_be32(&sizedesc[0], hash->bytes >> 29);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~~
/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:152:52: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  152 |     secp256k1_write_be32(&sizedesc[4], hash->bytes << 3);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~
/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:261:19: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  261 |         int now = outlen;
      |             ~~~   ^~~~~~
13 warnings generated.

[real-or-random]

When building with clang and the -Wshorten-64-to-32 flag, there are some warnings:

Well, then don't use this flag. :P Or do you think this is a problem? Is it a default flag?

[hebasto]

Or do you think this is a problem?

Each case has to be investigated to answer this question.

Is it a default flag?

macOS Xcode set it by default.

[real-or-random]

I'm a bit on the fence. All of these are fixable, and maybe some should be fixed (in particular the last one), but in other cases, this will just use up some of the precious reviewer bandwidth.

In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:27:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/field_impl.h:14:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/field_5x52_impl.h:13:
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:279:39: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  279 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:289:19: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  289 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:290:26: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  290 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:370:39: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  370 |             w = (f * g * (f * f - 2)) & m;
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:380:19: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  380 |             w = f + (((f + 1) & 4) << 1);
      |               ~ ~~^~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/modinv64_impl.h:381:26: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  381 |             w = (-w * g) & m;
      |               ~ ~~~~~~~~~^~~

These are harmless, but it may be more natural to make w a uint64_t? @sipa

In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:28:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/scalar_impl.h:20:
/home/hebasto/git/secp256k1/secp256k1/src/scalar_4x64_impl.h:119:42: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  119 |     overflow = secp256k1_u128_to_u64(&t) + secp256k1_scalar_check_overflow(r);
      |              ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hebasto/git/secp256k1/secp256k1/src/scalar_4x64_impl.h:682:34: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'unsigned int' [-Wshorten-64-to-32]
  682 |     secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r));
      |     ~~~~~~~~~~~~~~~~~~~~~~~    ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I think these warnings are a bit stupid, but okay, explicit is better than implicit. I wouldn't mind adding an explicit cast.

In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:30:
/home/hebasto/git/secp256k1/secp256k1/src/ecmult_impl.h:518:21: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  518 |     for (i = n_wnaf - 1; i >= 0; i--) {
      |            ~ ~~~~~~~^~~
/home/hebasto/git/secp256k1/secp256k1/src/ecmult_impl.h:562:52: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
  562 |         for(j = ECMULT_TABLE_SIZE(bucket_window+2) - 1; j > 0; j--) {
      |               ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
```

I don't know. We could make all of these size_t or ptrdiff_t if we care about negative values. Does this improve the code? Perhaps, but it's not entirely trivial because it needs work and review.

In file included from /home/hebasto/git/secp256k1/secp256k1/src/secp256k1.c:32:
In file included from /home/hebasto/git/secp256k1/secp256k1/src/ecmult_gen_impl.h:14:
/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:151:52: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  151 |     secp256k1_write_be32(&sizedesc[0], hash->bytes >> 29);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~~
/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:152:52: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
  152 |     secp256k1_write_be32(&sizedesc[4], hash->bytes << 3);
      |     ~~~~~~~~~~~~~~~~~~~~               ~~~~~~~~~~~~^~~~

These are obviously okay. Happy to add a cast if that makes apple clang happy...

/home/hebasto/git/secp256k1/secp256k1/src/hash_impl.h:261:19: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  261 |         int now = outlen;
      |             ~~~   ^~~~~~

Okay, now should be a size_t here. This is not a problem in the current code base because the function is always called with outlen == 32.