Fix msvc compiler error C4146 (minus operator applied to unsigned type) #9916

pull kobake wants to merge 2 commits into bitcoin:master from kobake:fix-minus-operator-target-for-msvc-c4146 changing 2 files +3 −3
  1. kobake commented at 8:07 AM on March 4, 2017: contributor

    On msvc14, the error C4146 (unary minus operator applied to unsigned type, result still unsigned) had been occured. Minus operator should be applied to signed type.

  2. fanquake added the label Windows on Mar 4, 2017
  3. in src/bloom.cpp:None in 170887a0ec outdated 
     253 | @@ -254,8 +254,8 @@ void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey)
 254 |          if (nGeneration == 4) {
 255 |              nGeneration = 1;
 256 |          }
 257 | -        uint64_t nGenerationMask1 = -(uint64_t)(nGeneration & 1);
 258 | -        uint64_t nGenerationMask2 = -(uint64_t)(nGeneration >> 1);
 259 | +        uint64_t nGenerationMask1 = (uint64_t)(-(int64_t)(nGeneration & 1));
 260 | +        uint64_t nGenerationMask2 = (uint64_t)(-(int64_t)(nGeneration >> 1));
    laanwj commented at 8:54 AM on March 4, 2017:

    This works around the error, but I don't think it's very elegant. What about just formulating it explicitly:

            uint64_t nGenerationMask1 = ~(uint64_t)(nGeneration & 1) + 1;
        uint64_t nGenerationMask2 = ~(uint64_t)(nGeneration >> 1) + 1;
    laanwj commented at 8:56 AM on March 4, 2017:

    Or even just:

            uint64_t nGenerationMask1 = 0 - (uint64_t)(nGeneration & 1);
        uint64_t nGenerationMask2 = 0 - (uint64_t)(nGeneration >> 1);

    Not sure this is any more readable than the above though :)

    kobake commented at 9:57 AM on March 4, 2017:

    Thanks. I confirmed three ways work well as same in my local msvc14. The latter you show is simple but not feel as explicit bit calculation. I like the former that is readable clearly as two's complement formula, then I'll adopt it.

    kobake commented at 10:07 AM on March 4, 2017:

    Applied. (commit --amend && push -f)

  4. kobake force-pushed on Mar 4, 2017
  5. gmaxwell commented at 7:35 PM on March 4, 2017: contributor

    ugh. MSVC produces the most absurd errors for perfectly valid, strictly conforming code. I don't have a problem with silencing it in this way that this PR currently does it (though IMO it makes it less clear).

    The prior fix of casting to a signed type can actually result in undefined behavior (at least generally if not actually in our case due to the sizes we use). a=INT_MIN; a = -a; is undefined. (highlighting the foolishness of MSFT's war on bit manipulation. :P -- as someone trying to address this warning in an 'obvious way' would potentially introduce undefined behavior as a result!)

  6. laanwj commented at 9:39 PM on March 4, 2017: member

    Agreed, that this is necessary at all is silly. There's no ambiguity here. Unary minus -x equivalent to 0-x which is well-defined also for unsigned values which are defined to wrap around.

  7. kobake commented at 3:07 AM on March 5, 2017: contributor

    Yes, we should take care not to meet undefined behavior. Now in this case, even if nGeneration has INT_MIN value, it's no problem because the target values to be applied two's complement formula are "(nGeneration & 1)" and "(nGeneration >> 1)"; they are 32bit values and their most significant bit is always zero, of course it's true even after the cast.

  8. gmaxwell commented at 7:19 PM on March 5, 2017: contributor

    https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4146

    Seems to indicate that 0-(uint64_t)(nGeneration & 1); will silence it-- if so I think that would be preferable.

  9. kobake commented at 3:07 AM on March 6, 2017: contributor

    OK, I just understood. I'll adopt 0 - x style. FYI, msvc14 (vs2015) treats C4146 not as warning but as error by default because of /sdl option.

  10. kobake force-pushed on Mar 6, 2017
  12. laanwj force-pushed on Mar 6, 2017
  13. laanwj commented at 5:39 PM on March 6, 2017: member

    Amended the commit message to put a newline between the title and body, otherwise git log in short formats doesn't work as expected.

  14. Fix msvc compiler error C4146 (minus operator applied to unsigned type)
    On msvc14, the compiler error C4146 (unary minus operator applied to unsigned type, result still unsigned) had been occured.
Use '0 - x' styled formula instead of '-x' so as to fix the error.
    292112f87e
  15. kobake force-pushed on Mar 6, 2017
  16. kobake commented at 5:46 PM on March 6, 2017: contributor

    Oh, I see. I've inserted a new line as you say. Thanks for helpful tips.

  17. kobake commented at 10:15 PM on March 7, 2017: contributor

    I also fixed another C4146 error in the commit below that is about unit test. https://github.com/kobake/bitcoin-msvc/commit/3c73d820e2355b36ea6bd6c4b78e4afa82637b8f

    Should I contain the commit in this PR #9916, or create an another PR for the commit?

  18. laanwj commented at 6:29 AM on March 8, 2017: member

    It's another minor thing, I'd say add it here.

  19. Fix msvc compiler error C4146 (unary minus operator applied to unsigned type)
    On msvc14, int literal '-2147483648' is invalid, because '2147483648' is unsigned type and cant't apply minus operator to unsigned type.
To define the int literal correctly, use '-2147483647 - 1' formula that is also used to define INT_MIN in limits.h.
    8e0720bdb9
  20. kobake commented at 6:44 AM on March 8, 2017: contributor

    Thanks, I added.

  23. laanwj merged this on Mar 9, 2017
  24. laanwj closed this on Mar 9, 2017
  25. laanwj referenced this in commit b403ec5c0f on Mar 9, 2017
  26. PastaPastaPasta referenced this in commit ce2d51d28c on Jan 2, 2019
  27. PastaPastaPasta referenced this in commit d83a27c5da on Jan 2, 2019
  28. PastaPastaPasta referenced this in commit 291ee387b4 on Jan 2, 2019
  29. PastaPastaPasta referenced this in commit 34c4113fd7 on Jan 3, 2019
  30. PastaPastaPasta referenced this in commit 0d5966b9dd on Jan 21, 2019
  31. PastaPastaPasta referenced this in commit 8c7a2c6876 on Jan 27, 2019
  32. PastaPastaPasta referenced this in commit 4ff6560f4b on Jan 29, 2019
  33. PastaPastaPasta referenced this in commit e5c4a67a20 on Feb 26, 2019
  34. UdjinM6 referenced this in commit 89f283f34d on Mar 9, 2019
  35. PastaPastaPasta referenced this in commit 88db8d6bce on Mar 10, 2019
  36. kobake deleted the branch on Sep 28, 2020
  37. zkbot referenced this in commit be459619a8 on Mar 5, 2021
  38. zkbot referenced this in commit 78de0cdf46 on Apr 15, 2021
  39. DrahtBot locked this on Feb 15, 2022
Contributors
kobakelaanwjgmaxwellpaveljanik
Labels
Windows
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me