From 4ffd62572df29c3445fb3edd96a7a826259c6d0f Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 8 May 2025 11:16:41 +0100 Subject: [PATCH] Use C++20 in the C++ wrapper This commit mostly just fixes compilation errors when compiling in C++20 mode, though the change to std::filesystem::path::u8string()'s return type means it's not backwards-compatible with C++17. --- cpp/CMakeLists.txt | 2 +- cpp/build.rs | 2 +- cpp/src/api/convert.cpp | 10 ++++++ cpp/src/api/convert.h | 6 ++++ cpp/src/api/database.cpp | 12 +++---- cpp/src/api/game.cpp | 13 ++++---- .../api/interface/create_game_handle_test.h | 22 ++++++------- .../tests/api/interface/game_interface_test.h | 5 +-- .../api/interface/plugin_interface_test.h | 8 ++--- cpp/src/tests/api/internals/main.cpp | 16 +++++----- cpp/src/tests/common_game_test_fixture.h | 32 +++++++++---------- cpp/src/tests/test_helpers.h | 4 +-- 12 files changed, 75 insertions(+), 57 deletions(-) diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 704e58ff..56f0bdfd 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -17,7 +17,7 @@ option(LIBLOOT_BUILD_TESTS "Whether or not to build libloot's tests." ON) option(LIBLOOT_INSTALL_DOCS "Whether or not to install libloot's docs (which need to be built separately)." ON) set(CMAKE_POSITION_INDEPENDENT_CODE ON) -set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) ############################## diff --git a/cpp/build.rs b/cpp/build.rs index 9e470942..92328c5e 100644 --- a/cpp/build.rs +++ b/cpp/build.rs @@ -1,6 +1,6 @@ fn main() { cxx_build::bridge("src/lib.rs") - .std("c++17") + .std("c++20") .flag_if_supported("/Zc:__cplusplus") .flag_if_supported("/permissive-") .compile("libloot-cpp"); diff --git a/cpp/src/api/convert.cpp b/cpp/src/api/convert.cpp index 1d40da1b..4def33e4 100644 --- a/cpp/src/api/convert.cpp +++ b/cpp/src/api/convert.cpp @@ -45,6 +45,12 @@ std::string convert(const ::rust::String& string) { return std::string(string); } +std::filesystem::path to_path(const ::rust::String& string) { + std::u8string_view view(reinterpret_cast(string.data()), + string.length()); + return std::filesystem::path(view); +} + // Although there's an explicit conversion operator declared, it seems that // building the CXX wrapper with MSVC doesn't set __cplusplus correctly as using // the operator causes a linker error, so this just reimpls it as a function. @@ -148,6 +154,10 @@ loot::Vertex convert(const loot::rust::Vertex& vertex) { return ::rust::Str(view.data(), view.length()); } +::rust::String convert(const std::filesystem::path& path) { + return ::rust::String(reinterpret_cast(path.u8string().data())); +} + ::rust::Box convert(const loot::Group& group) { return loot::rust::new_group( group.GetName(), group.GetDescription(), convert(group.GetAfterGroups())); diff --git a/cpp/src/api/convert.h b/cpp/src/api/convert.h index 99cfae5c..b05d14a5 100644 --- a/cpp/src/api/convert.h +++ b/cpp/src/api/convert.h @@ -1,6 +1,8 @@ #ifndef LOOT_API_CONVERT #define LOOT_API_CONVERT +#include + #include "libloot-cpp/src/lib.rs.h" #include "loot/metadata/group.h" #include "loot/metadata/plugin_metadata.h" @@ -12,6 +14,8 @@ namespace loot { std::string convert(const ::rust::String& string); +std::filesystem::path to_path(const ::rust::String& string); + std::string_view convert(::rust::Str string); loot::Group convert(const loot::rust::Group& group); @@ -41,6 +45,8 @@ loot::Vertex convert(const loot::rust::Vertex& vertex); ::rust::Str convert(std::string_view view); +::rust::String convert(const std::filesystem::path& path); + ::rust::Box convert(const loot::Group& group); ::rust::Box convert(const loot::File& file); diff --git a/cpp/src/api/database.cpp b/cpp/src/api/database.cpp index d0a174b8..8ad6ec99 100644 --- a/cpp/src/api/database.cpp +++ b/cpp/src/api/database.cpp @@ -10,7 +10,7 @@ Database::Database(::rust::Box&& database) : void Database::LoadMasterlist(const std::filesystem::path& masterlistPath) { try { - database_->load_masterlist(masterlistPath.u8string()); + database_->load_masterlist(convert(masterlistPath)); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } @@ -20,8 +20,8 @@ void Database::LoadMasterlistWithPrelude( const std::filesystem::path& masterlistPath, const std::filesystem::path& masterlistPreludePath) { try { - database_->load_masterlist_with_prelude(masterlistPath.u8string(), - masterlistPreludePath.u8string()); + database_->load_masterlist_with_prelude(convert(masterlistPath), + convert(masterlistPreludePath)); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } @@ -29,7 +29,7 @@ void Database::LoadMasterlistWithPrelude( void Database::LoadUserlist(const std::filesystem::path& userlistPath) { try { - database_->load_userlist(userlistPath.u8string()); + database_->load_userlist(convert(userlistPath)); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } @@ -38,7 +38,7 @@ void Database::LoadUserlist(const std::filesystem::path& userlistPath) { void Database::WriteUserMetadata(const std::filesystem::path& outputFile, const bool overwrite) const { try { - database_->write_user_metadata(outputFile.u8string(), overwrite); + database_->write_user_metadata(convert(outputFile), overwrite); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } @@ -165,7 +165,7 @@ void Database::DiscardAllUserMetadata() { void Database::WriteMinimalList(const std::filesystem::path& outputFile, const bool overwrite) const { try { - database_->write_minimal_list(outputFile.u8string(), overwrite); + database_->write_minimal_list(convert(outputFile), overwrite); } catch (const ::rust::Error& e) { std::rethrow_exception(mapError(e)); } diff --git a/cpp/src/api/game.cpp b/cpp/src/api/game.cpp index fafac74d..a2892c7d 100644 --- a/cpp/src/api/game.cpp +++ b/cpp/src/api/game.cpp @@ -77,10 +77,11 @@ rust::Box constructGame( const std::filesystem::path& localDataPath) { try { if (localDataPath.empty()) { - return loot::rust::new_game(convert(gameType), gamePath.u8string()); + return loot::rust::new_game(convert(gameType), loot::convert(gamePath)); } else { - return loot::rust::new_game_with_local_path( - convert(gameType), gamePath.u8string(), localDataPath.u8string()); + return loot::rust::new_game_with_local_path(convert(gameType), + loot::convert(gamePath), + loot::convert(localDataPath)); } } catch (const ::rust::Error& e) { std::rethrow_exception(loot::mapError(e)); @@ -134,7 +135,7 @@ void Game::SetAdditionalDataPaths( std::vector<::rust::String> path_strings; std::vector<::rust::Str> path_strs; for (const auto& path : additionalDataPaths) { - path_strings.push_back(path.u8string()); + path_strings.push_back(convert(path)); path_strs.push_back(path_strings.back()); } try { @@ -146,7 +147,7 @@ void Game::SetAdditionalDataPaths( } bool Game::IsValidPlugin(const std::filesystem::path& pluginPath) const { - return game_->is_valid_plugin(pluginPath.u8string()); + return game_->is_valid_plugin(convert(pluginPath)); } void Game::LoadPlugins(const std::vector& pluginPaths, @@ -154,7 +155,7 @@ void Game::LoadPlugins(const std::vector& pluginPaths, std::vector<::rust::String> path_strings; std::vector<::rust::Str> path_strs; for (const auto& path : pluginPaths) { - path_strings.push_back(path.u8string()); + path_strings.push_back(convert(path)); path_strs.push_back(path_strings.back()); } diff --git a/cpp/src/tests/api/interface/create_game_handle_test.h b/cpp/src/tests/api/interface/create_game_handle_test.h index c92dd4dc..2660b3a4 100644 --- a/cpp/src/tests/api/interface/create_game_handle_test.h +++ b/cpp/src/tests/api/interface/create_game_handle_test.h @@ -154,28 +154,28 @@ TEST_P( } } #else -TEST_P(CreateGameHandleTest, - shouldNotThrowOnWindowsIfLocalPathIsNotGiven) { +TEST_P(CreateGameHandleTest, shouldNotThrowOnWindowsIfLocalPathIsNotGiven) { EXPECT_NO_THROW(CreateGameHandle(GetParam(), gamePath)); } #endif -TEST_P(CreateGameHandleTest, - shouldNotThrowIfGameAndLocalPathsAreNotEmpty) { +TEST_P(CreateGameHandleTest, shouldNotThrowIfGameAndLocalPathsAreNotEmpty) { EXPECT_NO_THROW(CreateGameHandle(GetParam(), gamePath, localPath)); } -TEST_P( - CreateGameHandleTest, +TEST_P(CreateGameHandleTest, shouldSetAdditionalDataPathsForFallout4FromMicrosoftStoreOrStarfield) { if (GetParam() == GameType::fo4) { // Create the file that indicates it's a Microsoft Store install. touch(gamePath / "appxmanifest.xml"); } else if (GetParam() == GameType::openmw) { std::ofstream out(gamePath / "openmw.cfg"); - out << "data-local=\"" << (localPath / "data").u8string() << "\"" - << std::endl - << "config=\"" << localPath.u8string() << "\""; + out << "data-local=\"" + << reinterpret_cast( + (localPath / "data").u8string().c_str()) + << "\"" << std::endl + << "config=\"" + << reinterpret_cast(localPath.u8string().c_str()) << "\""; } const auto game = CreateGameHandle(GetParam(), gamePath, localPath); @@ -200,8 +200,8 @@ TEST_P( const auto expectedSuffix = std::filesystem::u8path("Documents") / "My Games" / "Starfield" / "Data"; - EXPECT_TRUE(endsWith(game->GetAdditionalDataPaths()[0].u8string(), - expectedSuffix.u8string())); + EXPECT_TRUE(game->GetAdditionalDataPaths()[0].u8string().ends_with( + expectedSuffix.u8string())); } else if (GetParam() == GameType::openmw) { EXPECT_EQ(std::vector{localPath / "data"}, game->GetAdditionalDataPaths()); diff --git a/cpp/src/tests/api/interface/game_interface_test.h b/cpp/src/tests/api/interface/game_interface_test.h index 9682deda..c3883601 100644 --- a/cpp/src/tests/api/interface/game_interface_test.h +++ b/cpp/src/tests/api/interface/game_interface_test.h @@ -35,7 +35,7 @@ constexpr unsigned int ESP_ERROR_PLUGIN_METADATA_NOT_FOUND = 14; class GameInterfaceTest : public ApiGameOperationsTest { protected: GameInterfaceTest() : - emptyFile("EmptyFile.esm"), nonAsciiEsm(u8"non\u00C1scii.esm") { + emptyFile("EmptyFile.esm"), nonAsciiEsm(reinterpret_cast(u8"non\u00C1scii.esm")) { // Make sure the plugin with a non-ASCII filename exists. std::filesystem::copy_file(dataPath / blankEsm, dataPath / std::filesystem::u8path(nonAsciiEsm)); @@ -531,7 +531,8 @@ TEST_P(GameInterfaceTest, sortPluginsShouldSucceedIfPassedValidArguments) { std::vector pluginsToSort; for (const auto& plugin : pluginsToLoad) { - pluginsToSort.push_back(plugin.filename().u8string()); + const auto filename = plugin.filename().u8string(); + pluginsToSort.push_back(std::string(reinterpret_cast(filename.data()), filename.length())); } std::vector actualOrder = handle_->SortPlugins(pluginsToSort); diff --git a/cpp/src/tests/api/interface/plugin_interface_test.h b/cpp/src/tests/api/interface/plugin_interface_test.h index ed8da8ba..bc99480f 100644 --- a/cpp/src/tests/api/interface/plugin_interface_test.h +++ b/cpp/src/tests/api/interface/plugin_interface_test.h @@ -33,8 +33,8 @@ class PluginInterfaceTest : public ApiGameOperationsTest { protected: PluginInterfaceTest() : ApiGameOperationsTest(), - nonAsciiEsp(u8"non\u00C1scii.esp"), - otherNonAsciiEsp(u8"other non\u00C1scii.esp"), + nonAsciiEsp(reinterpret_cast(u8"non\u00C1scii.esp")), + otherNonAsciiEsp(reinterpret_cast(u8"other non\u00C1scii.esp")), blankArchive("Blank" + GetArchiveFileExtension(GetParam())), blankSuffixArchive("Blank - Different - suffix" + GetArchiveFileExtension(GetParam())) {} @@ -90,12 +90,12 @@ protected: touch(dataPath / blankSuffixArchive); auto nonAsciiArchivePath = - dataPath / std::filesystem::u8path(u8"non\u00E1scii" + + dataPath / std::filesystem::u8path(reinterpret_cast(u8"non\u00E1scii") + GetArchiveFileExtension(GetParam())); touch(dataPath / nonAsciiArchivePath); auto nonAsciiPrefixArchivePath = - dataPath / std::filesystem::u8path(u8"other non\u00E1scii2 - suffix" + + dataPath / std::filesystem::u8path(reinterpret_cast(u8"other non\u00E1scii2 - suffix") + GetArchiveFileExtension(GetParam())); touch(dataPath / nonAsciiPrefixArchivePath); } diff --git a/cpp/src/tests/api/internals/main.cpp b/cpp/src/tests/api/internals/main.cpp index ed295dc9..3c4e04b3 100644 --- a/cpp/src/tests/api/internals/main.cpp +++ b/cpp/src/tests/api/internals/main.cpp @@ -29,7 +29,7 @@ #ifdef _WIN32 TEST(Filesystem, pathStringConstructorDoesNotConvertCharacterEncodingFromUtf8ToNative) { - std::string utf8 = u8"Andr\u00E9_settings.toml"; + std::string utf8 = reinterpret_cast(u8"Andr\u00E9_settings.toml"); std::u16string utf16 = u"Andr\u00E9_settings.toml"; ASSERT_EQ('\xc3', utf8[4]); @@ -38,14 +38,14 @@ TEST(Filesystem, std::filesystem::path path(utf8); EXPECT_EQ(utf8, path.string()); - EXPECT_NE(utf8, path.u8string()); + EXPECT_NE(utf8, reinterpret_cast(path.u8string().c_str())); EXPECT_NE(utf16, path.u16string()); } TEST( Filesystem, pathStringAndLocaleConstructorDoesNotConvertCharacterEncodingFromUtf8WithClassicLocale) { - std::string utf8 = u8"Andr\u00E9_settings.toml"; + std::string utf8 = reinterpret_cast(u8"Andr\u00E9_settings.toml"); std::u16string utf16 = u"Andr\u00E9_settings.toml"; ASSERT_EQ('\xc3', utf8[4]); @@ -55,12 +55,12 @@ TEST( EXPECT_EQ(utf8, path.string()); - EXPECT_NE(utf8, path.u8string()); + EXPECT_NE(utf8, reinterpret_cast(path.u8string().c_str())); EXPECT_NE(utf16, path.u16string()); } #else TEST(Filesystem, pathStringConstructorUsesNativeEncodingOfUtf8) { - std::string utf8 = u8"Andr\u00E9_settings.toml"; + std::string utf8 = reinterpret_cast(u8"Andr\u00E9_settings.toml"); std::u16string utf16 = u"Andr\u00E9_settings.toml"; ASSERT_EQ('\xc3', utf8[4]); @@ -69,13 +69,13 @@ TEST(Filesystem, pathStringConstructorUsesNativeEncodingOfUtf8) { std::filesystem::path path(utf8); EXPECT_EQ(utf8, path.string()); - EXPECT_EQ(utf8, path.u8string()); + EXPECT_EQ(utf8, reinterpret_cast(path.u8string().c_str())); EXPECT_EQ(utf16, path.u16string()); } #endif TEST(Filesystem, u8pathConvertsCharacterEncodingFromUtf8ToNative) { - std::string utf8 = u8"Andr\u00E9_settings.toml"; + std::string utf8 = reinterpret_cast(u8"Andr\u00E9_settings.toml"); std::u16string utf16 = u"Andr\u00E9_settings.toml"; ASSERT_EQ('\xc3', utf8[4]); @@ -89,7 +89,7 @@ TEST(Filesystem, u8pathConvertsCharacterEncodingFromUtf8ToNative) { EXPECT_EQ(utf8, path.string()); #endif - EXPECT_EQ(utf8, path.u8string()); + EXPECT_EQ(utf8, reinterpret_cast(path.u8string().c_str())); EXPECT_EQ(utf16, path.u16string()); } diff --git a/cpp/src/tests/common_game_test_fixture.h b/cpp/src/tests/common_game_test_fixture.h index dc47b44f..b212e08b 100644 --- a/cpp/src/tests/common_game_test_fixture.h +++ b/cpp/src/tests/common_game_test_fixture.h @@ -226,14 +226,25 @@ protected: it != std::filesystem::directory_iterator(); ++it) { if (std::filesystem::is_regular_file(it->status())) { - std::string filename = it->path().filename().u8string(); - if (filename == nonPluginFile) + std::u8string u8Filename = it->path().filename().u8string(); + std::string filename(reinterpret_cast(u8Filename.data()), + u8Filename.length()); + + if (filename == nonPluginFile) { continue; - if (endsWith(filename, ".ghost")) - filename = it->path().stem().u8string(); - if (endsWith(filename, ".esp") || endsWith(filename, ".esm")) + } + + if (filename.ends_with(".ghost")) { + u8Filename = it->path().stem().u8string(); + filename = + std::string(reinterpret_cast(u8Filename.data()), + u8Filename.length()); + } + + if (filename.ends_with(".esp") || filename.ends_with(".esm")) { loadOrder.emplace(std::filesystem::last_write_time(it->path()), filename); + } } } for (const auto& plugin : loadOrder) actual.push_back(plugin.second); @@ -384,17 +395,6 @@ protected: WriteFile(path, bytes); } - static bool endsWith(const std::string& str, const std::string& suffix) { - if (str.length() < suffix.length()) { - return false; - } - - auto view = std::string_view(str); - view.remove_prefix(str.length() - suffix.length()); - - return view == suffix; - } - private: GameType gameType_; const std::filesystem::path rootTestPath; diff --git a/cpp/src/tests/test_helpers.h b/cpp/src/tests/test_helpers.h index 9b8d5a68..83ce2e60 100644 --- a/cpp/src/tests/test_helpers.h +++ b/cpp/src/tests/test_helpers.h @@ -71,10 +71,10 @@ std::filesystem::path getRootTestPath() { // The non-ASCII character is there to ensure test coverage of non-ASCII path // handling. - std::string directoryName = u8"libloot-t\u00E9st-"; + std::u8string directoryName = u8"libloot-t\u00E9st-"; for (int i = 0; i < 16; i += 1) { - directoryName.push_back(static_cast(dist(prng))); + directoryName.push_back(static_cast(dist(prng))); } return std::filesystem::absolute(std::filesystem::temp_directory_path() /