Resolve GCC/Clang Warnings

[Minty-Meeo]

I could not get the three -Wignored-attributes warnings across AES.cpp and SHA1.cpp to go away. I even briefly tried editing libstdc++, but it did not work. For GCC, the -Wunknown-pragmas warning is still happening in CPUCull.cpp despite the pragmas trying to ignore that diagnostic. I tried moving them out of the #if #else #endif block to see if that would fix it, but it did not. There is room for debate whether the changes for -Wunused-const-variable are best. It's either that or changing to an unscoped enum.

I am not situated to test MSVC quickly, so I have not bothered with it.

[Minty-Meeo]

Seems like the deprecation for Qt6 is just something on my end, those buildbots hated that change.

[Pokechu22]

For GCC, the -Wunknown-pragmas warning is still happening in CPUCull.cpp despite the pragmas trying to ignore that diagnostic.

See #11534 - this is a GCC bug (89038 → 53431) which will be fixed in GCC 13, but for now there's nothing that can be done about it.

I couldn't resolve the ignored attributes one either.

[Minty-Meeo]

I was hesitant to touch the bignum arithmetic code, but all signs pointed towards size_t making sense. I could change it further to only use size_t instead of mixed int usage, but then I either have to add a (n == 0) condition at the start, or assume (perhaps with a __builtin_assume) that it can never be zero.

[Pokechu22]

Note that for (size_t i = n - 1; i >= 0; --i) would break, as i >= 0 is always true for unsigned i (and this would also not trigger undefined behavior as over/underflow for unsigned values is defined to work modulo 2^n).

[Minty-Meeo]

Yeah I stupidly did that the first time I went over that file and was told off by the compiler warnings. I think this other solution with post-increment will work, but I don't yet know what relevant code uses this function to see if it breaks.
Edit: OH the unit tests. Splendid.

[AdmiralCurtiss]

I appreciate the attempt to improve Dolphin's code, but please realize that the point of a warning is to look at the code in question to verify if the current behavior is correct and, if yes, acknowledge the behavior by adding a signifier for this (eg. a cast) and if no, rewrite the code in order to be correct. The point of a warning is not, as I'm often getting the impression with your changes, to change the code to get the warning to go away without thinking about what that code is doing. For example...

[AdmiralCurtiss]

I would also recommend, despite the low line count, to split this into a few separate PRs so the good changes can be merged without getting blocked by changes that might be more... controversial.

[Minty-Meeo]

The efficiency of the modified bignum functions was eating at me, so I threw it into Godbolt and chipped away until I found solutions even faster than the originals. For example, bn_sub_modulus: https://godbolt.org/z/3n5M637Ms

Edit: It could be two instructions faster if we always assume n > 0 and move the condition to the end of the loop body, but that would make the function unsafe.

[leoetlino]

Please revert this and the callback change. This causes curl to leak through and defeats the entire purpose of having an abstraction.

[Pokechu22]

Probably instead of using curl_off_t, ProgressCallback could be changed to use s64 (keeping the change in HttpRequest.cpp).

[Minty-Meeo]

I hope the new solution I came up with is satisfactory.

[Pokechu22]

Looks good to me. Still needs @leoetlino to review the new implementation where ProgressCallback uses s64.