Fix Clang warnings, review additional warning options, add a way to use Clang if available

[EllipticEllipsis]

Before opening this PR, ensure the following:

• ./format.sh was run to apply standard formatting.
• make successfully builds a matching ROM.
• No new compiler warnings were introduced during the build process.
  • Can be verified locally by running tools/warnings_count/check_new_warnings.sh
• New variables & functions should follow standard naming conventions.
• Comments and variables have correct spelling.

---

This PR is an attempt to allow using Clang as CC_CHECK: the warnings tend to be more descriptive than GCC and it has a few more options, quite apart from being generally a better compiler than GCC and the default on Mac.

I used -Weverything and fixed or disabled warnings depending on if it was possible to fix them while maintaining matching. There follows a summary of every warning type that -Weverything revealed in the repo, divided by what I think we should do with it.

Warnings we cannot use

• -Wreserved-id-macro: easily the worst offender. I have corrected the header guards to reduce these, but a lot are from gbi.h and would require renaming a lot of the macros to not start with __ or _X
• -Wlong-long: long long is not C89, but we have it anyway.
• -Wpadded: Our structs have to have plenty of dodgy padding.
• -Wc11-extensions: we use anonymous structs/unions.
• -Wunused-parameter: plenty of functions don't use one or more of their arguments.
• -Wunused-variable: pads.
• -Wsign-conversion,
• -Wshift-sign-overflow: used for a lot of the bitwise operations we need to do to match.
• -Wimplicit-int-conversion,
• -Wfloat-conversion,
• -Wimplicit-int-float-conversion,
• -Wimplicit-float-conversion,
• -Wbad-function-cast: all required for casting for matching quite frequently, and some are not fixable by casting explicitly.
• -Wfloat-equal: not really a mistake, just unlikely. But common and unfixable.
• -Wcast-align: similar to most of the other conversion ones.

Warnings indicating genuine mistakes that we cannot fix

• -Wtautological-overlap-compare Several files in the codebase have this. It is a default Clang warning.
• -Wunreachable-code Usually a consequence of the code the previous warning refers to.
• -Wclass-varargs There is a printf in audiomgr.c that presumably was meant to point to the type of the OSTask, but it is not possible to write it in a reasonable way and still match.

Arguably these should be on.

Warnings we can (and IMO should) use

• -Wshadow: tells you if locally-scoped variable names are the same as one from an enclosing scope. Useful most of the time.
• -Wpedantic: strictly enforces the C standard. I have eliminated all the warnings apart from 2 types that have to be suppressed, and the ones we cannot fix.
• -Wstrict-prototypes: all declarations should be actual prototypes.

Warnings we would ideally use, but can't for now

• -Wunused-macros: this mostly refers to FLAGS and THIS in untouched actor files. Hopefully eventually we can use it.
• -Wunused-label: Required for fake-matching usually.
• -Wmissing-braces: most of the graphics macros do this. I have fixed the others.
• -Wint-conversion: Usually conversion to pointers. This is actually useful for finding all the pointers being passed as u32 and vice versa. But it will not distinguish uintptr_t, since that seems to always be typedef as unsigned int or similar anyway.
• -Wdouble-promotion: this is leaving fs off your floats. The problem is that several places in libultra do this deliberately to match, and certain devs genuinely do forget. It would be useful to have for the rest, though.
• -Wextra-semi-stmt: tells you about useless semicolons. I have removed all the ones that are not from gbi.h. In theory these could be fixed by editing gbi.h, or including gbi.h in some fancy way so that its warnings are ignored.

Warnings not worth bothering with

• -Wc99-extensions: this only currently matters for commas at the end of enum lists. We could remove them, but we already disable the IDO warning for them.
• -Wmissing-prototypes,
• -Wmissing-variable-declarations: these test for a function/variable having a forward declaration at all, whether or not it is required. It would force us to forward-declare everything.
• -Wimplicit-fallthrough: to encourage you to write your switches more readably, useless to us.
• -Wcomma: basically always wrong when trying to guess if commas are used correctly, particularly in for loops inits.
• -Wshorten-64-to-32: happens all the time when working with OSTime. We could cast all these, but it seems pointless.
• -Wmissing-noreturn: more a suggestion, and a GCC/C11 one at that.
• -Wcast-qual: tells you if you lost a const through casting. Debatable, but it's not like const is much use to us anyway, given IDO's eccentricities.
• -Wfour-char-constants,
• -Wmultichar: is useful for portability, but we don't care about whether our magic values like 'VIEW' are identical on little-endian.

Actually using Clang

I thought long and hard about how to actually implement using Clang, and eventually I came up with this:

ifeq ($(shell command -v clang >/dev/null 2>&1; echo $$?),0)
  CC_CHECK_COMP := clang
else
  CC_CHECK_COMP := gcc
endif

CHECK_WARNINGS_ON  := -Wall -Wextra -Wstrict-prototypes -Wshadow
CHECK_WARNINGS_OFF := -Wno-format-security -Wno-unknown-pragmas -Wno-unused-parameter -Wno-unused-variable -Wno-missing-braces -Wno-int-conversion -Wno-unused-label

ifeq ($(CC_CHECK_COMP),clang)
CHECK_WARNINGS_ON  += -Wpedantic 
CHECK_WARNINGS_OFF += -Wno-long-long -Wno-c11-extensions -Wno-c99-extensions -Wno-tautological-overlap-compare -Wno-four-char-constants
else
CHECK_WARNINGS_OFF += -Wno-unused-but-set-variable -Wmultichar
endif

CHECK_WARNINGS := $(CHECK_WARNINGS_ON) $(CHECK_WARNINGS_OFF)
CC_CHECK := $(CC_CHECK_COMP) -fno-builtin -fsyntax-only -funsigned-char -std=gnu90 -D _LANGUAGE_C -D NON_MATCHING -Iinclude -Isrc -Iassets -Ibuild -include stdarg.h $(CHECK_WARNINGS)

This will check if Clang is installed, and use it if it is. If it isn't, it will use GCC. This allows us to benefit from Clang without requiring everyone (including Jenkins) to install it right away. It also automatically mostly fixes a significant problem with trying to use two compilers, namely that Clang and GCC have different support for warnings. But since we are supposed to be supporting Mac anyway, we actually had this problem all along.

This PR is not intended to be the final word on anything; it is meant to open up a discussion about what is best for the project relating to this topic.

[EllipticEllipsis]

I'm going to draft this until libultra's finished so I can fix it all in one go.

[hensldm]

Since this is waiting on libvoice, and I imagine it will be better to another branch, rather than try and bring this up to date, perhaps we close this for the time being.

[AngheloAlf]

Closing due to inactivity and many of the changes being applied on other PRs