From df29cc696f22e6b91b09284ee9ce0779ed77a3d9 Mon Sep 17 00:00:00 2001 From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> Date: Thu, 29 Aug 2024 17:33:29 +0200 Subject: [PATCH] [Core] Optionally delete `Ref` `nullptr` comparisons Adds an optional (default false) compile option to enable comparing `Ref` to `nullptr` to ensure correct use, as well as future expandsion for more general dev checks (enabled with `dev_mode`) --- .github/workflows/android_builds.yml | 2 +- .github/workflows/ios_builds.yml | 2 +- .github/workflows/linux_builds.yml | 2 +- .github/workflows/macos_builds.yml | 2 +- .github/workflows/web_builds.yml | 2 +- .github/workflows/windows_builds.yml | 2 +- SConstruct | 11 ++++++++++- core/object/ref_counted.h | 5 +++++ 8 files changed, 21 insertions(+), 7 deletions(-) diff --git a/.github/workflows/android_builds.yml b/.github/workflows/android_builds.yml index 69ab8308293..2f49f84e90c 100644 --- a/.github/workflows/android_builds.yml +++ b/.github/workflows/android_builds.yml @@ -6,7 +6,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no module_text_server_fb_enabled=yes + SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no module_text_server_fb_enabled=yes strict_checks=yes concurrency: group: ci-${{github.actor}}-${{github.head_ref || github.run_number}}-${{github.ref}}-android diff --git a/.github/workflows/ios_builds.yml b/.github/workflows/ios_builds.yml index 8da6d1311e6..207219f9f44 100644 --- a/.github/workflows/ios_builds.yml +++ b/.github/workflows/ios_builds.yml @@ -6,7 +6,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no module_text_server_fb_enabled=yes + SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no module_text_server_fb_enabled=yes strict_checks=yes concurrency: group: ci-${{github.actor}}-${{github.head_ref || github.run_number}}-${{github.ref}}-ios diff --git a/.github/workflows/linux_builds.yml b/.github/workflows/linux_builds.yml index 10389dc9da0..010a13d9210 100644 --- a/.github/workflows/linux_builds.yml +++ b/.github/workflows/linux_builds.yml @@ -6,7 +6,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes + SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes strict_checks=yes DOTNET_NOLOGO: true DOTNET_CLI_TELEMETRY_OPTOUT: true TSAN_OPTIONS: suppressions=misc/error_suppressions/tsan.txt diff --git a/.github/workflows/macos_builds.yml b/.github/workflows/macos_builds.yml index 4db7462b3a8..938b25f134f 100644 --- a/.github/workflows/macos_builds.yml +++ b/.github/workflows/macos_builds.yml @@ -6,7 +6,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes + SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes strict_checks=yes concurrency: group: ci-${{github.actor}}-${{github.head_ref || github.run_number}}-${{github.ref}}-macos diff --git a/.github/workflows/web_builds.yml b/.github/workflows/web_builds.yml index d3a6b5b8b82..11ef7f01c97 100644 --- a/.github/workflows/web_builds.yml +++ b/.github/workflows/web_builds.yml @@ -6,7 +6,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no use_closure_compiler=yes + SCONSFLAGS: verbose=yes warnings=extra werror=yes debug_symbols=no use_closure_compiler=yes strict_checks=yes EM_VERSION: 3.1.64 EM_CACHE_FOLDER: "emsdk-cache" diff --git a/.github/workflows/windows_builds.yml b/.github/workflows/windows_builds.yml index 0c215765178..0d2e258308a 100644 --- a/.github/workflows/windows_builds.yml +++ b/.github/workflows/windows_builds.yml @@ -7,7 +7,7 @@ on: env: # Used for the cache key. Add version suffix to force clean build. GODOT_BASE_BRANCH: master - SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes d3d12=yes "angle_libs=${{github.workspace}}/" + SCONSFLAGS: verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes d3d12=yes strict_checks=yes "angle_libs=${{github.workspace}}/" SCONS_CACHE_MSVC_CONFIG: true concurrency: diff --git a/SConstruct b/SConstruct index b1819a1dab5..0245531b45e 100644 --- a/SConstruct +++ b/SConstruct @@ -230,7 +230,11 @@ opts.Add("custom_modules", "A list of comma-separated directory paths containing opts.Add(BoolVariable("custom_modules_recursive", "Detect custom modules recursively for each specified path.", True)) # Advanced options -opts.Add(BoolVariable("dev_mode", "Alias for dev options: verbose=yes warnings=extra werror=yes tests=yes", False)) +opts.Add( + BoolVariable( + "dev_mode", "Alias for dev options: verbose=yes warnings=extra werror=yes tests=yes strict_checks=yes", False + ) +) opts.Add(BoolVariable("tests", "Build the unit tests", False)) opts.Add(BoolVariable("fast_unsafe", "Enable unsafe options for faster rebuilds", False)) opts.Add(BoolVariable("ninja", "Use the ninja backend for faster rebuilds", False)) @@ -262,6 +266,7 @@ opts.Add( "", ) opts.Add(BoolVariable("use_precise_math_checks", "Math checks use very precise epsilon (debug option)", False)) +opts.Add(BoolVariable("strict_checks", "Enforce stricter checks (debug option)", False)) opts.Add(BoolVariable("scu_build", "Use single compilation unit build", False)) opts.Add("scu_limit", "Max includes per SCU file when using scu_build (determines RAM use)", "0") opts.Add(BoolVariable("engine_update_check", "Enable engine update checks in the Project Manager", True)) @@ -602,12 +607,16 @@ if env["dev_mode"]: env["warnings"] = ARGUMENTS.get("warnings", "extra") env["werror"] = methods.get_cmdline_bool("werror", True) env["tests"] = methods.get_cmdline_bool("tests", True) + env["strict_checks"] = methods.get_cmdline_bool("strict_checks", True) if env["production"]: env["use_static_cpp"] = methods.get_cmdline_bool("use_static_cpp", True) env["debug_symbols"] = methods.get_cmdline_bool("debug_symbols", False) # LTO "auto" means we handle the preferred option in each platform detect.py. env["lto"] = ARGUMENTS.get("lto", "auto") +if env["strict_checks"]: + env.Append(CPPDEFINES=["STRICT_CHECKS"]) + # Run SCU file generation script if in a SCU build. if env["scu_build"]: max_includes_per_scu = 8 diff --git a/core/object/ref_counted.h b/core/object/ref_counted.h index 5b358135c44..f0706b4d08f 100644 --- a/core/object/ref_counted.h +++ b/core/object/ref_counted.h @@ -86,6 +86,11 @@ public: _FORCE_INLINE_ bool operator!=(const T *p_ptr) const { return reference != p_ptr; } +#ifdef STRICT_CHECKS + // Delete these to prevent raw comparisons with `nullptr`. + bool operator==(std::nullptr_t) const = delete; + bool operator!=(std::nullptr_t) const = delete; +#endif // STRICT_CHECKS _FORCE_INLINE_ bool operator<(const Ref &p_r) const { return reference < p_r.reference;