Use int instead of char for flag variables

[jamesjer]

I attempted to build version 2.2.0 for Fedora Rawhide. All of the non-x86 architectures (aarch, ppc64le, and s390x) failed a test:

=================================== FAILURES ===================================
_____________________________ test_mpz_from_bytes ______________________________

    @settings(max_examples=1000)
>   @given(integers(), integers(min_value=0, max_value=10000),
           sampled_from(['big', 'little']), booleans())

test/test_mpz.py:108: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.13/site-packages/hypothesis/core.py:1300: in _raise_to_user
    raise the_error_hypothesis_found
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

x = -2, length = 5, byteorder = 'little', signed = True

    @settings(max_examples=1000)
    @given(integers(), integers(min_value=0, max_value=10000),
           sampled_from(['big', 'little']), booleans())
    @example(0, 0, 'big', False)
    @example(0, 0, 'little', False)
    @example(0, 1, 'big', False)
    @example(128, 1, 'big', True)
    @example(128, 1, 'little', True)
    @example(-129, 1, 'big', True)
    @example(-129, 1, 'little', True)
    @example(-1, 0, 'big', True)
    @example(-1, 1, 'big', True)
    @example(-1, 1, 'little', True)
    @example(-2, 0, 'big', True)
    @example(-2, 0, 'little', True)
    @example(-1, 3, 'big', True)
    @example(-2, 3, 'big', True)
    @example(-2, 5, 'little', True)
    def test_mpz_from_bytes(x, length, byteorder, signed):
        try:
            bytes = x.to_bytes(length, byteorder, signed=signed)
        except OverflowError:
            assume(False)
        else:
            rx = int.from_bytes(bytes, byteorder, signed=signed)
>           assert rx == mpz.from_bytes(bytes, byteorder, signed=signed)
E           AssertionError: assert -2 == mpz(-4294967297)
E            +  where mpz(-4294967297) = <built-in method from_bytes of type object at 0xffffa57105b0>(b'\xfe\xff\xff\xff\xff', 'little', signed=True)
E            +    where <built-in method from_bytes of type object at 0xffffa57105b0> = mpz.from_bytes
E           Falsifying explicit example: test_mpz_from_bytes(
E               x=-2,
E               length=5,
E               byteorder='little',
E               signed=True,
E           )

test/test_mpz.py:132: AssertionError

The problem is the use of the char type for flag variables. The C standard says that char can be equivalent to either signed char or unsigned char. The code in GMPy_MPZ_Method_From_Bytes assigns -1 to such a variable, endian. On platforms where char == unsigned char, the value stored is actually 255, a positive value.

This PR converts flag variables declared with type char to type int instead, for the following reasons:

1. Using char doesn't save any space. These are local variables, so they are either in a CPU register (which is 32 or 64 bits, even if we only use 8 bits), or on the stack where, for alignment reasons, 32 bits will be reserved even though we only use 8 bits of it.
2. Using char instead of int generates less efficient code. Modern 64-bit processors are optimized for processing 64-bit and 32-bit quantities. They can operate on 8-bit quantities, but take more clock cycles to do so.
3. The char type is only useful for unsigned 7-bit values; i.e., 0 to 127. That is the intersection of the signed char and unsigned char types.

With this change, the build succeeds on all architectures.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.50%. Comparing base (224745b) to head (4b3aee6).
Report is 30 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #490      +/-   ##
==========================================
- Coverage   85.52%   85.50%   -0.03%     
==========================================
  Files          50       50              
  Lines       11656    11646      -10     
  Branches     2202     2201       -1     
==========================================
- Hits         9969     9958      -11     
- Misses       1687     1688       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[skirpichev]

Mostly LGTM from me, thanks for the fix. Build failure seems to be unrelated, I'll try to address this in a separate PR.

Suggestion: maybe you could expand commit message with the body of your pr description? Reduce failed example a little.

[jamesjer]

I think the PR comment is unnecessarily verbose. How does this look?

[skirpichev]

Yep, fine.

[casevh]

Thanks for the fix.

[skirpichev]

Maybe this should go to the next bugfix release?

[casevh]

I agree. I can release 2.2.1 this weekend.

…

On Thu, Jul 11, 2024, 11:46 PM Sergey B Kirpichev ***@***.***> wrote: Maybe this should go to the next bugfix release? — Reply to this email directly, view it on GitHub <#490 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMR233MLC63XWDGHKDBIM3ZL53TZAVCNFSM6AAAAABKTGOBYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRUHEZDOMRXGY> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>