fix Issue 18734 - bitnum parameter of core.bitop.bt should be signed

[WalterBright]

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18734	normal	bitnum parameter of core.bitop.bt should be signed

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11306"

[aG0aep6G]

This doesn't seem to fix much. It just changes what kind of expression gets miscompiled.

Adapted test case (also made it -betterC, because getting a 32-bit Phobos is too much trouble):

import core.stdc.stdio;
extern (C) void main()
{
    static assert(size_t.sizeof == 4); /* We're in 32-bit code. */
    
    enum len = 2 ^^ 27;
    assert(len * size_t.sizeof * 8L == 2L ^^ 32);
        /* Exactly enough space for size_t.max bits. */
    
    import core.stdc.stdlib: malloc;
    auto a = (cast(size_t*) malloc((len + 1) * size_t.sizeof))[0 .. len + 1];
        /* Adding one because we're going to start from a[1]. */
    
    a[0] = 0x8000_0000; /* Set the last bit of the first size_t. */
    printf("%d\n", bt(&a[1], -1)); /* Try to read it. */
        /* Without -O -inline: "1", correct. With -O -inline: "0", wrong. */
    
    a[0] = 0; /* Unset the last bit of the first size_t. */
    a[$ - 1] = 0x8000_0000; /* Set the very last bit. */
    printf("%d\n", bt(&a[1], -1)); /* Try reading the last bit of the first size_t again. */
        /* Without -O -inline: "0", correct. With -O -inline: "1", wrong. */
}

/* Copied from core.bitop. */
int bt(in size_t* p, ptrdiff_t bitnum) pure @system
{
    static if (size_t.sizeof == 8)
        return ((p[bitnum >> 6] & (1L << (bitnum & 63)))) != 0;
    else static if (size_t.sizeof == 4)
        return ((p[bitnum >> 5] & (1  << (bitnum & 31)))) != 0;
    else
        static assert(0);
}

There is a noteworthy difference, though: -inline is needed to get bad output. With the original test case, adding -inline made the output good again. So maybe there's something going in the inliner, too.

[andralex]

There should be tests...

[RazvanN7]

@WalterBright Please add some tests.

[RazvanN7]

@WalterBright could you maybe add a test for this PR? Ideally you can use the one provided by @aG0aep6G

[ibuclaw]

Does the backend inliner make this ultimately redundant?

[RazvanN7]

This still needs to add tests.