libgme / game-music-emu

Blargg's video game music emulation library, which allows audio applications to easily add playback support for the music of many classic video game consoles.
GNU Lesser General Public License v2.1
59 stars 12 forks source link

MSVC warning fixes #88

Closed sezero closed 4 months ago

sezero commented 4 months ago

This fixes (silences) MSVC warnings C4291, C4805 and C4804. See individual commit messages for details. Also see MSVC CI build logs from master branch.

There is one warning left, C4146, occurring at 8 places. Don't know what to do about them, don't know whether or not to do anything about them.

Ay_Apu.cpp(302,22): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Ay_Cpu.cpp(1044,11): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Hes_Apu.cpp(109,28): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Hes_Apu.cpp(112,50): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Kss_Cpu.cpp(1082,11): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Sap_Apu.cpp(33,27): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Spc_Dsp.cpp(158,19): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Sms_Apu.cpp(144,26): warning C4146: unary minus operator applied to unsigned type, result still unsigned
Wohlstand commented 4 months ago

I'll check this soon, I'm not at PC yet (I'm at kitchen right now).

sezero commented 4 months ago

Your warning reducing commit dd2b7b0841748c99c996ed4b675121216c0ea529 doesn't seem to have any effect.

I rebased this PR to latest master, anyway..

Wohlstand commented 4 months ago

Your warning reducing commit dd2b7b0 doesn't seem to have any effect.

On MY end it has, otherwise, I wouldn't add it.

Anyway, about the minus warning, I found the solution - just add more casting, and here is a demo test I did:

#include <stdio.h>

int main()
{
    unsigned long i;
    unsigned long noise_lfsr = 42;
    unsigned long res_minus = -(noise_lfsr & 1);
    unsigned long res_tilda = ~(noise_lfsr & 1);
    unsigned long res_none  =   noise_lfsr & 1;
    unsigned long res_minus2= ((-noise_lfsr) & 1);

    printf("noise_lfsr minus = (0x%08X) %lu\n", res_minus,  res_minus);
    printf("noise_lfsr tilda = (0x%08X) %lu\n", res_tilda,  res_tilda);
    printf("noise_lfsr none  = (0x%08X) %lu\n", res_none,   res_none);
    printf("noise_lfsr minus = (0x%08X) %lu\n", res_minus2, res_minus2);

    for(i = 0; i < 0xFFFFFFFF; ++i)
    {
        res_minus = -(i & 1);
        res_minus2 = (unsigned long)(-((long)(i & 1)));
        res_tilda = ~(i & 1);
        res_none  =   i & 1;

        if(res_minus != res_minus2)
        {
            printf("----------------------------------------------------\n");
            printf("DIFFERENCE (i=%lu)!!!\n", i);
            printf("noise_lfsr minus = (0x%08X) %lu\n", res_minus,  res_minus);
            printf("noise_lfsr minus2= (0x%08X) %lu\n", res_minus2, res_minus2);
            printf("noise_lfsr tilda = (0x%08X) %lu\n", res_tilda,  res_tilda);
            printf("noise_lfsr none  = (0x%08X) %lu\n", res_none,   res_none);
            fflush(stdout);
        }
    }

    return 0;
}
Wohlstand commented 4 months ago

Uno momento, I'll apply this...

sezero commented 4 months ago

I think you want to decrement in tilda calculation, no? E.g.:

-   unsigned long res_tilda = ~(noise_lfsr & 1);
+   unsigned long res_tilda = ~((noise_lfsr & 1) - 1);
Wohlstand commented 4 months ago

I think you want to decrement in tilda calculation, no? E.g.:

Sounds like a good idea, I tested out, and it works, going to apply some...

Wohlstand commented 4 months ago

Your warning reducing commit dd2b7b0 doesn't seem to have any effect.

I checked, and that was because I forgot to clear the /W3 from the C side (I cleaned C++ only), So, now should be fixed too.

sezero commented 4 months ago

Off-topic: There is an impossible mess about [unsigned] long type usage: I think most, if not all, of [unsigned] long type usage can be replaced with [u]int32_t, no? Same goes for the blarggh_ulong thing.