build: C++20 migration (again^3) (#5514)

* build: `17` -> `20` * fix: c++20 complaints also 1) fix incomplete achievement_requirement 2) remove user defined copy constructor: error: definition of implicit copy assignment operator for 'point_with_value' is deprecated because it has a user-declared copy constructor [-Werror,-Wdeprecated-copy] Co-authored-by: AngelicosPhosphoros <angelicos.phosphoros@protonmail.com> * build: disable zero-as-null-pointer-constant cannot use spaceship operator until llvm/llvm-project#43670 * refactor: overhaul `units_def.h` 1. add `concepts_utility.h` for storing useful concepts related headers. 2. uses `Arithmatic` concepts to ensure they're numbers. 3. uses spaceship operator for comparison. 4. uses auto and arrow returns to make templates readable. 5. make fabs and fmod constexpr. Co-authored-by: Coolthulhu <Coolthulhu@gmail.com> * ci: bump lit version to c++20 * refactor: use default equality * ci: explicitly use C++20 for clang-tidy * build: add c++20 flag to `Android.mk` * build: ignore `modernize-use-nullptr` it works as a false positive on spaceship operators `<=>` * refactor: apply some idea suggestions * ci: bump java langauge version to 11 * test: run all clang-tidy check even if it fails * test: problemetic checks * fix: attempt to fix MSVC build error * fix: rollback to old filesystem code passing string directly borks filesystem test on windows * fix: use relative import to prevent clang-tidy complaints * test: add more captures like what's the problem * test: skip `remove_tree` and `remove_directory` test atm rationale: - spent past 6 hours trying to fix this and failed - currently not worth the effort considering it's not used anywhere in-game --------- Co-authored-by: AngelicosPhosphoros <angelicos.phosphoros@protonmail.com> Co-authored-by: Coolthulhu <Coolthulhu@gmail.com>
cataclysmbnteam · Oct 8, 2024 · fd85e05 · fd85e05
1 parent 6d55f9c
commit fd85e05
Show file tree

Hide file tree

Showing 35 changed files with 303 additions and 270 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -58,6 +58,7 @@ Checks: >
   -modernize-concat-nested-namespaces,
   -modernize-loop-convert,
   -modernize-unary-static-assert,
+  -modernize-use-nullptr,
   -performance-no-automatic-move,
   -performance-trivially-destructible,
   -performance-for-range-copy,
@@ -99,8 +100,12 @@ Checks: >
   -misc-use-anonymous-namespace,
   -clang-diagnostic-unused-macros,
   -clang-analyzer-optin.*,
+  -performance-enum-size,
+  -cert-dcl58-cpp,
 # https://github.com/llvm/llvm-project/issues/59572
 # https://github.com/llvm/llvm-project/issues/111003
+# performance-enum-size: no automatic fix available
+# cert-dcl58-cpp: too many issues with LUNA
 
 # Turn back on when all the fixes are applied
 # WarningsAsErrors: '*'

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -236,7 +236,7 @@ if (NOT MSVC)
        -Wpedantic \
        -Wsuggest-override \
        -Wunused-macros \
-       -Wzero-as-null-pointer-constant \
+       -Wno-zero-as-null-pointer-constant \
        -Wno-unknown-warning-option \
        -Wno-dangling-reference \
        -Wno-range-loop-analysis")
@@ -255,7 +255,7 @@ if (NOT MSVC)
     set(CMAKE_CXX_FLAGS_DEBUG "-Og -g2")
 endif ()
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 

diff --git a/Makefile b/Makefile
@@ -116,7 +116,7 @@ WARNINGS = \
   -Wpedantic \
   -Wsuggest-override \
   -Wunused-macros \
-  -Wzero-as-null-pointer-constant \
+  -Wno-zero-as-null-pointer-constant \
   -Wno-unknown-warning-option \
   -Wno-dangling-reference \
   -Wno-range-loop-analysis # TODO: Fix warnings instead of disabling
@@ -432,9 +432,9 @@ ifndef RELEASE
 endif
 
 ifeq ($(shell sh -c 'uname -o 2>/dev/null || echo not'),Cygwin)
-  OTHERS += -std=gnu++17
+  OTHERS += -std=gnu++20
 else
-  OTHERS += -std=c++17
+  OTHERS += -std=c++20
 endif
 
 ifeq ($(CYGWIN),1)

diff --git a/android/app/jni/Application.mk b/android/app/jni/Application.mk
@@ -2,7 +2,7 @@
 
 # See CPLUSPLUS-SUPPORT.html in the NDK documentation for more information
 APP_STL := c++_shared
-APP_CPPFLAGS += -std=c++17
+APP_CPPFLAGS += -std=c++20
 ifneq ($(OS),Windows_NT)
     APP_LDFLAGS += -fuse-ld=lld
 endif

diff --git a/android/app/src/main/java/org/libsdl/app/SDL.java b/android/app/src/main/java/org/libsdl/app/SDL.java
@@ -42,42 +42,34 @@ public static void loadLibrary(String libraryName) throws UnsatisfiedLinkError,
         }
 
         try {
-            // Let's see if we have ReLinker available in the project.  This is necessary for 
-            // some projects that have huge numbers of local libraries bundled, and thus may 
+            // Let's see if we have ReLinker available in the project.  This is necessary for
+            // some projects that have huge numbers of local libraries bundled, and thus may
             // trip a bug in Android's native library loader which ReLinker works around.  (If
             // loadLibrary works properly, ReLinker will simply use the normal Android method
             // internally.)
             //
-            // To use ReLinker, just add it as a dependency.  For more information, see 
+            // To use ReLinker, just add it as a dependency.  For more information, see
             // https://github.com/KeepSafe/ReLinker for ReLinker's repository.
             //
-            Class relinkClass = mContext.getClassLoader().loadClass("com.getkeepsafe.relinker.ReLinker");
-            Class relinkListenerClass = mContext.getClassLoader().loadClass("com.getkeepsafe.relinker.ReLinker$LoadListener");
-            Class contextClass = mContext.getClassLoader().loadClass("android.content.Context");
-            Class stringClass = mContext.getClassLoader().loadClass("java.lang.String");
+            Class<?> relinkClass = mContext.getClassLoader().loadClass("com.getkeepsafe.relinker.ReLinker");
+            Class<?> relinkListenerClass = mContext.getClassLoader().loadClass("com.getkeepsafe.relinker.ReLinker$LoadListener");
+            Class<?> contextClass = mContext.getClassLoader().loadClass("android.content.Context");
+            Class<?> stringClass = mContext.getClassLoader().loadClass("java.lang.String");
 
-            // Get a 'force' instance of the ReLinker, so we can ensure libraries are reinstalled if 
+            // Get a 'force' instance of the ReLinker, so we can ensure libraries are reinstalled if
             // they've changed during updates.
             Method forceMethod = relinkClass.getDeclaredMethod("force");
             Object relinkInstance = forceMethod.invoke(null);
-            Class relinkInstanceClass = relinkInstance.getClass();
+            Class<?> relinkInstanceClass = relinkInstance.getClass();
 
             // Actually load the library!
             Method loadMethod = relinkInstanceClass.getDeclaredMethod("loadLibrary", contextClass, stringClass, stringClass, relinkListenerClass);
             loadMethod.invoke(relinkInstance, mContext, libraryName, null, null);
         }
         catch (final Throwable e) {
             // Fall back
-            try {
-                System.loadLibrary(libraryName);
-            }
-            catch (final UnsatisfiedLinkError ule) {
-                throw ule;
-            }
-            catch (final SecurityException se) {
-                throw se;
-            }
-        }        
+            System.loadLibrary(libraryName);
+        }
     }
 
     protected static Context mContext;

diff --git a/build-scripts/run-clang-tidy-plugin.sh b/build-scripts/run-clang-tidy-plugin.sh
@@ -20,5 +20,6 @@ else
   echo "Analyzing $(wc -l < "$AFFECTED_FILES") files"
   # shellcheck disable=SC2002
   # cat "$AFFECTED_FILES" | parallel  -j"$NUM_JOBS" --no-notice -a ./build-scripts/clang-tidy-wrapper.sh {}
+  set +e # so we can run all the checks
   < "$AFFECTED_FILES" xargs -n1 -P"$NUM_JOBS" ./build-scripts/clang-tidy-wrapper.sh -quiet
 fi
diff --git a/doc/src/content/docs/en/dev/guides/building/makefile.md b/doc/src/content/docs/en/dev/guides/building/makefile.md
@@ -19,19 +19,19 @@ You have three major choices here: GCC, Clang and MXE.
 (Note that your distro may have separate packages e.g. `gcc` only includes the C compiler and for
 C++ you'll need to install `g++`.)
 
-Cataclysm is targeting C++17 standard and that means you'll need a compiler that supports it. You
-can easily check if your version of `g++` supports C++17 by running:
+Cataclysm is targeting C++20 standard and that means you'll need a compiler that supports it. You
+can easily check if your version of `g++` supports C++20 by running:
 
 ```sh
-$ g++ --std=c++17
+$ g++ --std=c++20
 g++: fatal error: no input files
 compilation terminated.
 ```
 
 If you get a line like:
 
 ```sh
-g++: error: unrecognized command line option ‘--std=c++17’
+g++: error: unrecognized command line option ‘--std=c++20’
 ```
 
 This means you'll need a newer version of GCC (`g++`).
@@ -377,7 +377,7 @@ builds the SDL version with all features enabled, including tiles, sound and loc
 
 ### Dependencies
 
-- Java JDK 8
+- Java JDK 11
 - SDL2 (tested with 2.0.8, though a custom fork is recommended with project-specific bugfixes)
 - SDL2_ttf (tested with 2.0.14)
 - SDL2_mixer (tested with 2.0.2)

diff --git a/msvc-full-features/Cataclysm-lib-vcpkg-static.vcxproj b/msvc-full-features/Cataclysm-lib-vcpkg-static.vcxproj
@@ -86,7 +86,7 @@
       <ForcedIncludeFiles>stdafx.h</ForcedIncludeFiles>
       <AdditionalOptions>/bigobj /utf-8 %(AdditionalOptions)</AdditionalOptions>
       <PreprocessorDefinitions>_SCL_SECURE_NO_WARNINGS;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_CONSOLE;SDL_SOUND;TILES;LUA;SDL_BUILDING_LIBRARY;USE_VCPKG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
-      <LanguageStandard>stdcpp17</LanguageStandard>
+      <LanguageStandard>stdcpp20</LanguageStandard>
       <AdditionalIncludeDirectories>..\src;..\src\lua;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <Link>

diff --git a/msvc-full-features/Cataclysm-test-vcpkg-static.vcxproj b/msvc-full-features/Cataclysm-test-vcpkg-static.vcxproj
@@ -84,7 +84,7 @@
       <ForcedIncludeFiles>stdafx.h</ForcedIncludeFiles>
       <AdditionalOptions>/bigobj /utf-8 %(AdditionalOptions)</AdditionalOptions>
       <PreprocessorDefinitions>_SCL_SECURE_NO_WARNINGS;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_CONSOLE;SDL_SOUND;TILES;LUA;SDL_MAIN_HANDLED;USE_VCPKG;CATCH_CONFIG_ENABLE_BENCHMARKING;%(PreprocessorDefinitions)</PreprocessorDefinitions>
-      <LanguageStandard>stdcpp17</LanguageStandard>
+      <LanguageStandard>stdcpp20</LanguageStandard>
       <AdditionalIncludeDirectories>..\src;..\src\lua;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <Link>

diff --git a/msvc-full-features/Cataclysm-vcpkg-static.vcxproj b/msvc-full-features/Cataclysm-vcpkg-static.vcxproj
@@ -103,7 +103,7 @@
       <ForcedIncludeFiles>stdafx.h</ForcedIncludeFiles>
       <AdditionalOptions>/bigobj /utf-8 %(AdditionalOptions)</AdditionalOptions>
       <PreprocessorDefinitions>_SCL_SECURE_NO_WARNINGS;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_WINDOWS;SDL_SOUND;TILES;LUA;USE_VCPKG;USE_WINMAIN;%(PreprocessorDefinitions)</PreprocessorDefinitions>
-      <LanguageStandard>stdcpp17</LanguageStandard>
+      <LanguageStandard>stdcpp20</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Windows</SubSystem>

diff --git a/msvc-full-features/JsonFormatter-vcpkg-static.vcxproj b/msvc-full-features/JsonFormatter-vcpkg-static.vcxproj
@@ -96,7 +96,7 @@
       <ForcedIncludeFiles>stdafx.h</ForcedIncludeFiles>
       <AdditionalOptions>/bigobj /utf-8 %(AdditionalOptions)</AdditionalOptions>
       <PreprocessorDefinitions>_SCL_SECURE_NO_WARNINGS;_CRT_SECURE_NO_WARNINGS;WIN32_LEAN_AND_MEAN;_CONSOLE;USE_VCPKG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
-      <LanguageStandard>stdcpp17</LanguageStandard>
+      <LanguageStandard>stdcpp20</LanguageStandard>
       <AdditionalIncludeDirectories>..\src;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <Link>

diff --git a/src/achievement.cpp b/src/achievement.cpp
@@ -9,7 +9,6 @@
 #include "character.h"
 #include "color.h"
 #include "debug.h"
-#include "enums.h"
 #include "event.h"
 #include "event_statistics.h"
 #include "game.h"
@@ -124,63 +123,6 @@ static nc_color color_from_completion( achievement_completion comp )
     abort();
 }
 
-struct achievement_requirement {
-    string_id<event_statistic> statistic;
-    achievement_comparison comparison;
-    int target;
-    bool becomes_false = false;
-
-    void deserialize( JsonIn &jin ) {
-        const JsonObject &jo = jin.get_object();
-        if( !( jo.read( "event_statistic", statistic ) &&
-               jo.read( "is", comparison ) &&
-               ( comparison == achievement_comparison::anything ||
-                 jo.read( "target", target ) ) ) ) {
-            jo.throw_error( "Mandatory field missing for achievement requirement" );
-        }
-    }
-
-    void finalize() {
-        switch( comparison ) {
-            case achievement_comparison::less_equal:
-                becomes_false = is_increasing( statistic->monotonicity() );
-                return;
-            case achievement_comparison::greater_equal:
-                becomes_false = is_decreasing( statistic->monotonicity() );
-                return;
-            case achievement_comparison::anything:
-                becomes_false = true;
-                return;
-            case achievement_comparison::last:
-                break;
-        }
-        debugmsg( "Invalid achievement_comparison" );
-        abort();
-    }
-
-    void check( const string_id<achievement> &id ) const {
-        if( !statistic.is_valid() ) {
-            debugmsg( "score %s refers to invalid statistic %s", id.str(), statistic.str() );
-        }
-    }
-
-    bool satisifed_by( const cata_variant &v ) const {
-        int value = v.get<int>();
-        switch( comparison ) {
-            case achievement_comparison::less_equal:
-                return value <= target;
-            case achievement_comparison::greater_equal:
-                return value >= target;
-            case achievement_comparison::anything:
-                return true;
-            case achievement_comparison::last:
-                break;
-        }
-        debugmsg( "Invalid achievement_requirement comparison value" );
-        abort();
-    }
-};
-
 static time_point epoch_to_time_point( achievement::time_bound::epoch e )
 {
     switch( e ) {
@@ -1076,11 +1018,65 @@ void achievements_tracker::deserialize( JsonIn &jsin )
 void achievements_tracker::init_watchers()
 {
     for( const achievement *a : valid_achievements() ) {
-        if( achievements_status_.count( a->id ) ) {
+        if( achievements_status_.contains( a->id ) ) {
             continue;
         }
         trackers_.emplace(
             std::piecewise_construct, std::forward_as_tuple( a->id ),
             std::forward_as_tuple( *a, *this, *stats_ ) );
     }
 }
+
+void achievement_requirement::deserialize( JsonIn &jin )
+{
+    const JsonObject &jo = jin.get_object();
+    if( !jo.read( "event_statistic", statistic ) ||
+        !jo.read( "is", comparison ) ||
+        ( comparison != achievement_comparison::anything &&
+          !jo.read( "target", target ) ) ) {
+        jo.throw_error( "Mandatory field missing for achievement requirement" );
+    }
+}
+
+void achievement_requirement::finalize()
+{
+    switch( comparison ) {
+        case achievement_comparison::less_equal:
+            becomes_false = is_increasing( statistic->monotonicity() );
+            return;
+        case achievement_comparison::greater_equal:
+            becomes_false = is_decreasing( statistic->monotonicity() );
+            return;
+        case achievement_comparison::anything:
+            becomes_false = true;
+            return;
+        case achievement_comparison::last:
+            break;
+    }
+    debugmsg( "Invalid achievement_comparison" );
+    abort();
+}
+
+void achievement_requirement::check( const string_id<achievement> &id ) const
+{
+    if( !statistic.is_valid() ) {
+        debugmsg( "score %s refers to invalid statistic %s", id.str(), statistic.str() );
+    }
+}
+
+bool achievement_requirement::satisifed_by( const cata_variant &v ) const
+{
+    int value = v.get<int>();
+    switch( comparison ) {
+        case achievement_comparison::less_equal:
+            return value <= target;
+        case achievement_comparison::greater_equal:
+            return value >= target;
+        case achievement_comparison::anything:
+            return true;
+        case achievement_comparison::last:
+            break;
+    }
+    debugmsg( "Invalid achievement_requirement comparison value" );
+    abort();
+}
diff --git a/src/achievement.h b/src/achievement.h
@@ -12,7 +12,10 @@
 
 #include "calendar.h"
 #include "cata_variant.h"
+#include "enums.h"
 #include "event_bus.h"
+#include "event_statistics.h"
+#include "json.h"
 #include "string_id.h"
 #include "translations.h"
 
@@ -143,6 +146,20 @@ class achievement
         void add_skill_requirement( const JsonObject &inner, const std::string &src );
 };
 
+
+struct achievement_requirement {
+    string_id<event_statistic> statistic;
+    achievement_comparison comparison;
+    int target;
+    bool becomes_false = false;
+
+    void deserialize( JsonIn &jin );
+    void finalize();
+    void check( const string_id<achievement> &id ) const;
+    bool satisifed_by( const cata_variant &v ) const;
+};
+
+
 template<>
 struct enum_traits<achievement::time_bound::epoch> {
     static constexpr achievement::time_bound::epoch last = achievement::time_bound::epoch::last;

diff --git a/src/calendar.cpp b/src/calendar.cpp
@@ -76,8 +76,10 @@ moon_phase get_moon_phase( const time_point &p )
     const int num_middays = to_days<int>( p_year - calendar::turn_zero + 1_days / 2 );
     const time_duration nearest_midnight = num_middays * 1_days;
     const double phase_change = nearest_midnight / moon_phase_duration;
-    const int current_phase = static_cast<int>( std::round( phase_change * MOON_PHASE_MAX ) ) %
+    const int current_phase = static_cast<int>( std::round( phase_change * static_cast<int>
+                              ( MOON_PHASE_MAX ) ) ) %
                               static_cast<int>( MOON_PHASE_MAX );
+
     return static_cast<moon_phase>( current_phase );
 }
 

diff --git a/src/catalua.cpp b/src/catalua.cpp
@@ -247,8 +247,10 @@ bool load_world_lua_state( const std::string &path )
 
 std::unique_ptr<lua_state, lua_state_deleter> make_wrapped_state()
 {
+    auto state = new lua_state{};
+    state->lua = make_lua_state();
     std::unique_ptr<lua_state, lua_state_deleter> ret(
-        new lua_state{ make_lua_state() },
+        state,
         lua_state_deleter{}
     );