diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ada01e93..131d83b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,7 @@ jobs: SCONS_CACHE: ${{ github.workspace }}/.scons-cache/ EM_VERSION: 3.1.39 EM_CACHE_FOLDER: "emsdk-cache" + SCONSFLAGS: warnings=extra werror=yes steps: - name: Checkout diff --git a/include/godot_cpp/core/binder_common.hpp b/include/godot_cpp/core/binder_common.hpp index ce90acda..498609a1 100644 --- a/include/godot_cpp/core/binder_common.hpp +++ b/include/godot_cpp/core/binder_common.hpp @@ -229,6 +229,13 @@ void call_with_ptr_args(T *p_instance, R (T::*p_method)(P...) const, const GDExt call_with_ptr_args_retc_helper(p_instance, p_method, p_args, r_ret, BuildIndexSequence{}); } +// GCC raises "parameter 'p_args' set but not used" when P = {}, +// it's not clever enough to treat other P values as making this branch valid. +#if defined(DEBUG_METHODS_ENABLED) && defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-but-set-parameter" +#endif + template void call_with_variant_args_helper(T *p_instance, void (T::*p_method)(P...), const Variant **p_args, GDExtensionCallError &r_error, IndexSequence) { r_error.error = GDEXTENSION_CALL_OK; @@ -470,13 +477,6 @@ void call_with_variant_args_retc_dv(T *p_instance, R (T::*p_method)(P...) const, call_with_variant_args_retc_helper(p_instance, p_method, argsp.data(), r_ret, r_error, BuildIndexSequence{}); } -// GCC raises "parameter 'p_args' set but not used" when P = {}, -// it's not clever enough to treat other P values as making this branch valid. -#if defined(DEBUG_METHODS_ENABLED) && defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-but-set-parameter" -#endif - template void call_get_argument_type_helper(int p_arg, int &index, GDExtensionVariantType &type) { if (p_arg == index) { diff --git a/include/godot_cpp/core/method_bind.hpp b/include/godot_cpp/core/method_bind.hpp index 37ae7317..382fa4ca 100644 --- a/include/godot_cpp/core/method_bind.hpp +++ b/include/godot_cpp/core/method_bind.hpp @@ -605,6 +605,7 @@ protected: #if defined(__GNUC__) && !defined(__clang__) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wlogical-op" +#pragma GCC diagnostic ignored "-Wunused-but-set-parameter" #endif virtual GDExtensionVariantType gen_argument_type(int p_arg) const { if (p_arg >= 0 && p_arg < (int)sizeof...(P)) { diff --git a/tools/targets.py b/tools/targets.py index 21611349..8cc43c80 100644 --- a/tools/targets.py +++ b/tools/targets.py @@ -1,4 +1,5 @@ import os +import re import subprocess import sys from SCons.Script import ARGUMENTS @@ -9,6 +10,37 @@ from SCons.Variables.BoolVariable import _text2bool # Helper methods +def get_compiler_version(env): + """ + Returns an array of version numbers as ints: [major, minor, patch]. + The return array should have at least two values (major, minor). + """ + if not env.get("is_msvc", False): + # Not using -dumpversion as some GCC distros only return major, and + # Clang used to return hardcoded 4.2.1: # https://reviews.llvm.org/D56803 + try: + version = subprocess.check_output([env.subst(env["CXX"]), "--version"]).strip().decode("utf-8") + except (subprocess.CalledProcessError, OSError): + print("Couldn't parse CXX environment variable to infer compiler version.") + return None + else: # TODO: Implement for MSVC + return None + match = re.search( + "(?:(?<=version )|(?<=\) )|(?<=^))" + "(?P\d+)" + "(?:\.(?P\d*))?" + "(?:\.(?P\d*))?" + "(?:-(?P[0-9a-zA-Z-]*))?" + "(?:\+(?P[0-9a-zA-Z-]*))?" + "(?: (?P[0-9]{8}|[0-9]{6})(?![0-9a-zA-Z]))?", + version, + ) + if match is not None: + return match.groupdict() + else: + return None + + def get_cmdline_bool(option, default): """We use `ARGUMENTS.get()` to check if options were manually overridden on the command line, and SCons' _text2bool helper to convert them to booleans, otherwise they're handled as strings. @@ -20,6 +52,14 @@ def get_cmdline_bool(option, default): return default +def using_gcc(env): + return "gcc" in os.path.basename(env["CC"]) + + +def using_emcc(env): + return "emcc" in os.path.basename(env["CC"]) + + def using_clang(env): return "clang" in os.path.basename(env["CC"]) @@ -49,6 +89,8 @@ def options(opts): ) opts.Add(BoolVariable("debug_symbols", "Build with debugging symbols", True)) opts.Add(BoolVariable("dev_build", "Developer build with dev-only debugging code (DEV_ENABLED)", False)) + opts.Add(EnumVariable("warnings", "Level of compilation warnings", "all", ("extra", "all", "moderate", "no"))) + opts.Add(BoolVariable("werror", "Treat compiler warnings as errors", False)) def exists(env): @@ -115,6 +157,37 @@ def generate(env): env.Append(LINKFLAGS=["/OPT:REF"]) elif env["optimize"] == "debug" or env["optimize"] == "none": env.Append(CCFLAGS=["/Od"]) + + # Warnings + if env["warnings"] == "no": + env.Append(CCFLAGS=["/w"]) + else: + if env["warnings"] == "extra": + env.Append(CCFLAGS=["/W4"]) + elif env["warnings"] == "all": + env.Append(CCFLAGS=["/W3"]) + elif env["warnings"] == "moderate": + env.Append(CCFLAGS=["/W2"]) + # Disable warnings which we don't plan to fix. + + env.Append( + CCFLAGS=[ + "/wd4100", # C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism. + "/wd4127", # C4127 (conditional expression is constant) + "/wd4201", # C4201 (non-standard nameless struct/union): Only relevant for C89. + "/wd4244", # C4244 C4245 C4267 (narrowing conversions): Unavoidable at this scale. + "/wd4245", + "/wd4267", + "/wd4305", # C4305 (truncation): double to float or real_t, too hard to avoid. + "/wd4514", # C4514 (unreferenced inline function has been removed) + "/wd4714", # C4714 (function marked as __forceinline not inlined) + "/wd4820", # C4820 (padding added after construct) + ] + ) + + if env["werror"]: + env.Append(CCFLAGS=["/WX"]) + else: if env["debug_symbols"]: # Adding dwarf-4 explicitly makes stacktraces work with clang builds, @@ -142,3 +215,64 @@ def generate(env): env.Append(CCFLAGS=["-Og"]) elif env["optimize"] == "none": env.Append(CCFLAGS=["-O0"]) + + # Warnings + cc_version = get_compiler_version(env) or { + "major": None, + "minor": None, + "patch": None, + "metadata1": None, + "metadata2": None, + "date": None, + } + cc_version_major = int(cc_version["major"] or -1) + cc_version_minor = int(cc_version["minor"] or -1) + + common_warnings = [] + + if using_gcc(env): + common_warnings += ["-Wshadow-local", "-Wno-misleading-indentation"] + if cc_version_major == 7: # Bogus warning fixed in 8+. + common_warnings += ["-Wno-strict-overflow"] + if cc_version_major < 11: + # Regression in GCC 9/10, spams so much in our variadic templates + # that we need to outright disable it. + common_warnings += ["-Wno-type-limits"] + if cc_version_major >= 12: # False positives in our error macros, see GH-58747. + common_warnings += ["-Wno-return-type"] + elif using_clang(env) or using_emcc(env): + # We often implement `operator<` for structs of pointers as a requirement + # for putting them in `Set` or `Map`. We don't mind about unreliable ordering. + common_warnings += ["-Wno-ordered-compare-function-pointers"] + + if env["warnings"] == "extra": + env.Append(CCFLAGS=["-Wall", "-Wextra", "-Wwrite-strings", "-Wno-unused-parameter"] + common_warnings) + env.Append(CXXFLAGS=["-Wctor-dtor-privacy", "-Wnon-virtual-dtor"]) + if using_gcc(env): + env.Append( + CCFLAGS=[ + "-Walloc-zero", + "-Wduplicated-branches", + "-Wduplicated-cond", + "-Wstringop-overflow=4", + ] + ) + env.Append(CXXFLAGS=["-Wplacement-new=1"]) + # Need to fix a warning with AudioServer lambdas before enabling. + # if cc_version_major != 9: # GCC 9 had a regression (GH-36325). + # env.Append(CXXFLAGS=["-Wnoexcept"]) + if cc_version_major >= 9: + env.Append(CCFLAGS=["-Wattribute-alias=2"]) + if cc_version_major >= 11: # Broke on MethodBind templates before GCC 11. + env.Append(CCFLAGS=["-Wlogical-op"]) + elif using_clang(env) or using_emcc(env): + env.Append(CCFLAGS=["-Wimplicit-fallthrough"]) + elif env["warnings"] == "all": + env.Append(CCFLAGS=["-Wall"] + common_warnings) + elif env["warnings"] == "moderate": + env.Append(CCFLAGS=["-Wall", "-Wno-unused"] + common_warnings) + else: # 'no' + env.Append(CCFLAGS=["-w"]) + + if env["werror"]: + env.Append(CCFLAGS=["-Werror"]) diff --git a/tools/windows.py b/tools/windows.py index d5a729c5..a4055459 100644 --- a/tools/windows.py +++ b/tools/windows.py @@ -32,7 +32,6 @@ def generate(env): env.Append(CPPDEFINES=["TYPED_METHOD_BIND", "NOMINMAX"]) env.Append(CCFLAGS=["/utf-8"]) - env.Append(LINKFLAGS=["/WX"]) if env["use_clang_cl"]: env["CC"] = "clang-cl"