From c7b8c353801f818a1f1e2df8762917a51e4d4620 Mon Sep 17 00:00:00 2001
From: Samuel Nicholas <nicholas.samuel@gmail.com>
Date: Sun, 29 Dec 2024 11:38:31 +1030
Subject: [PATCH] Refactor common_compiler_flags.cmake - remove incorrectly
 propagated target_compile_features - remove GNU_ prefix on generic compiler
 version genex's - remove line breaks for simple genex's - make most flags
 private - comments

---
 cmake/common_compiler_flags.cmake | 83 ++++++++++++++++---------------
 1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/cmake/common_compiler_flags.cmake b/cmake/common_compiler_flags.cmake
index e6954216..a12d5406 100644
--- a/cmake/common_compiler_flags.cmake
+++ b/cmake/common_compiler_flags.cmake
@@ -7,6 +7,16 @@ configuration. It includes flags like optimization levels, warnings, and
 features. For target platform specific flags look to each of the
 ``cmake/<platform>.cmake`` files.
 
+The default compile and link options CMake adds can be found in the
+platform modules_. When a project is created it initialises its variables from
+the ``CMAKE_*`` values. The cleanest way I have found to alter these defaults
+is the use of the ``CMAKE_PROJECT_<PROJECT-NAME>_INCLUDE`` as demonstrated by
+the emsdkHack.cmake to overcome the limitation on shared library creation.
+
+So far the emsdkHack is the only modification to the defaults we have made.
+
+.. _modules: https://github.com/Kitware/CMake/blob/master/Modules/Platform/
+
 ]=======================================================================]
 
 #[[ Compiler Configuration, not to be confused with build targets ]]
@@ -19,11 +29,11 @@ set( IS_GNU "$<CXX_COMPILER_ID:GNU>" )
 set( IS_MSVC "$<CXX_COMPILER_ID:MSVC>" )
 set( NOT_MSVC "$<NOT:$<CXX_COMPILER_ID:MSVC>>" )
 
-set( GNU_LT_V8 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,8>" )
-set( GNU_GE_V9 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,9>" )
-set( GNU_GT_V11 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,11>" )
-set( GNU_LT_V11 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,11>" )
-set( GNU_GE_V12 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,12>" )
+set( LT_V8  "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,8>" )
+set( GE_V9  "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,9>" )
+set( GT_V11 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,11>" )
+set( LT_V11 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,11>" )
+set( GE_V12 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,12>" )
 
 #[[ Check for clang-cl with MSVC frontend
 The compiler is tested and set when the project command is called.
@@ -46,19 +56,14 @@ endfunction(  )
 
 function( common_compiler_flags )
 
-    target_compile_features(${TARGET_NAME}
-            PUBLIC
-            cxx_std_17
-    )
-
     # These compiler options reflect what is in godot/SConstruct.
     target_compile_options( ${TARGET_NAME}
+        # The public flag tells CMake that the following options are transient,
+        #and will propagate to consumers.
         PUBLIC
             # Disable exception handling. Godot doesn't use exceptions anywhere, and this
             # saves around 20% of binary size and very significant build time.
-            $<${DISABLE_EXCEPTIONS}:
-                $<${NOT_MSVC}:-fno-exceptions>
-            >
+            $<${DISABLE_EXCEPTIONS}:$<${NOT_MSVC}:-fno-exceptions>>
 
             # Enabling Debug Symbols
             $<${DEBUG_SYMBOLS}:
@@ -70,20 +75,19 @@ function( common_compiler_flags )
                 >
             >
 
-            $<${IS_DEV_BUILD}:
-                $<${NOT_MSVC}:-fno-omit-frame-pointer -O0>
-            >
+            $<${IS_DEV_BUILD}:$<${NOT_MSVC}:-fno-omit-frame-pointer -O0>>
 
-            $<${HOT_RELOAD}:
-                $<${IS_GNU}:-fno-gnu-unique>
-            >
+            $<${HOT_RELOAD}:$<${IS_GNU}:-fno-gnu-unique>>
+
+        # Warnings below, these do not need to propagate to consumers.
+        PRIVATE
 
         # MSVC only
         $<${IS_MSVC}:
             # /MP isn't valid for clang-cl with msvc frontend
             $<$<CXX_COMPILER_ID:MSVC>:/MP${PROC_N}>
-            /W4
 
+            /W4      # Warning level 4 (informational) warnings that aren't off by default.
             # Disable warnings which we don't plan to fix.
             /wd4100  # C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism.
             /wd4127  # C4127 (conditional expression is constant)
@@ -124,21 +128,22 @@ function( common_compiler_flags )
             -Wplacement-new=1
             -Wshadow-local
             -Wstringop-overflow=4
-
-            # Bogus warning fixed in 8+.
-            $<${GNU_LT_V8}:-Wno-strict-overflow>
-
-            $<${GNU_GE_V9}:-Wattribute-alias=2>
-
-            # Broke on MethodBind templates before GCC 11.
-            $<${GNU_GT_V11}:-Wlogical-op>
-
-            # Regression in GCC 9/10, spams so much in our variadic templates that we need to outright disable it.
-            $<${GNU_LT_V11}:-Wno-type-limits>
-
-            # False positives in our error macros, see GH-58747.
-            $<${GNU_GE_V12}:-Wno-return-type>
         >
+
+        # Bogus warning fixed in 8+.
+        $<${IS_GNU}:$<${LT_V8}:-Wno-strict-overflow>>
+
+        $<${IS_GNU}:$<${GE_V9}:-Wattribute-alias=2>>
+
+        # Broke on MethodBind templates before GCC 11.
+        $<${IS_GNU}:$<${GT_V11}:-Wlogical-op>>
+
+        # Regression in GCC 9/10, spams so much in our variadic templates that we need to outright disable it.
+        $<${IS_GNU}:$<${LT_V11}:-Wno-type-limits>>
+
+        # False positives in our error macros, see GH-58747.
+        $<${IS_GNU}:$<${GE_V12}:-Wno-return-type>>
+
     )
 
     target_compile_definitions(${TARGET_NAME}
@@ -158,13 +163,11 @@ function( common_compiler_flags )
     )
 
     target_link_options( ${TARGET_NAME}
-        PUBLIC
-            $<${IS_MSVC}:
-                /WX             # treat link warnings as errors.
-                /MANIFEST:NO    # We dont need a manifest
-            >
+        PRIVATE
+            $<${IS_MSVC}:/WX /MANIFEST:NO>
+            # /WX             # treat link warnings as errors.
+            # /MANIFEST:NO    # We dont need a manifest
 
-            $<${DEBUG_SYMBOLS}:$<${IS_MSVC}:/DEBUG:FULL>>
             $<$<NOT:${DEBUG_SYMBOLS}>:
                 $<${IS_GNU}:-s>
                 $<${IS_CLANG}:-s>