From ce74ce585c3cd01e45f21b276d549c88add08a55 Mon Sep 17 00:00:00 2001 From: kindem Date: Sat, 20 Jun 2026 12:06:15 +0800 Subject: [PATCH 1/3] docs: refresh README build notes and third party list - drop the obsolete Windows long-path notice (qt recipe now ships prebuilt binaries, so the build tree no longer overflows) - add Linux to the supported platform/toolchain/generator table - list Google Benchmark under third party usage - remove the sponsor section and bump the license year to 2026 --- README.md | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 9044479d..12f34e81 100644 --- a/README.md +++ b/README.md @@ -57,10 +57,6 @@ If the output not equals `/Applications/Xcode.app/Contents/Developer`, you need sudo xcode-select -s /Applications/Xcode.app/Contents/Developer ``` -## Windows Notice - -Some third-party libraries managed by conan may fail to install or compile on Windows due to excessively long build tree paths. So you need to configure the environment variable `CONAN_HOME` and set it to a relatively short path, such as `C:\t` - ## Build Project The following table contains supported platform, toolchain and generator: @@ -99,6 +95,18 @@ The following table contains supported platform, toolchain and generator: Ninja Multi-Config + + Linux + x64 + GCC + Unix Makefiles + + + Ninja + + + Ninja Multi-Config + By cause of cmake, we recommend [CLion](https://www.jetbrains.com/clion/) as Explosion's IDE, which can help you build and manage project simplely, and brings best coding experience to you. When use CLion as IDE, you just need open the project and configure cmake toolchain and generator in settings, press build and everything is down. @@ -135,6 +143,7 @@ Thanks all those following projects: * [debugbreak](https://github.com/scottt/debugbreak) * [LLVM](https://llvm.org/) * [googletest](https://github.com/google/googletest) +* [Google Benchmark](https://github.com/google/benchmark) * [taskflow](https://github.com/taskflow/taskflow) * [cityhash](https://github.com/google/cityhash) * [stb](https://github.com/nothings/stb) @@ -150,10 +159,6 @@ Thanks all those following projects: * [HeroUI](https://www.heroui.com) * [TailWindCSS](https://tailwindcss.com) -# Sponsor - -JetBrains Open Source - # License -[MIT](https://github.com/ExplosionEngine/Explosion/blob/master/LICENSE) @ Explosion Development Team All right Reserved 2025. +[MIT](https://github.com/ExplosionEngine/Explosion/blob/master/LICENSE) @ Explosion Development Team All right Reserved 2026. From 6fd05f4b60b74a6ef41fb463579dd7959301316d Mon Sep 17 00:00:00 2001 From: kindem Date: Sat, 20 Jun 2026 12:41:55 +0800 Subject: [PATCH 2/3] feat: add Rust-like Result to Common Introduce Common/Result.h: a Result holding either an Ok value or an Err error, built via the Ok()/Err() helpers (Result mirrors Rust's Result<(), E>). Provides the usual accessors and combinators: Value/Error, Unwrap/UnwrapErr, Expect/ExpectErr, UnwrapOr/UnwrapOrElse, Map/MapErr, AndThen/OrElse and ToOptional. Replace the ad-hoc std::pair error results that were really Result-shaped with it, and update all call sites and tests: - MirrorTool::Parser -> Result - MirrorTool::Generator -> Result - Core::Cli::Parse -> Result --- Engine/Source/Common/Include/Common/Result.h | 311 ++++++++++++++++++ Engine/Source/Common/Test/ResultTest.cpp | 141 ++++++++ Engine/Source/Core/Include/Core/Cmdline.h | 3 +- Engine/Source/Core/Src/Cmdline.cpp | 6 +- Engine/Source/Core/Test/CmdlineTest.cpp | 4 +- Tool/MirrorTool/ExeSrc/Main.cpp | 14 +- .../MirrorTool/Include/MirrorTool/Generator.h | 3 +- Tool/MirrorTool/Include/MirrorTool/Parser.h | 4 +- Tool/MirrorTool/Src/Generator.cpp | 8 +- Tool/MirrorTool/Src/Parser.cpp | 4 +- Tool/MirrorTool/Test/Main.cpp | 16 +- 11 files changed, 484 insertions(+), 30 deletions(-) create mode 100644 Engine/Source/Common/Include/Common/Result.h create mode 100644 Engine/Source/Common/Test/ResultTest.cpp diff --git a/Engine/Source/Common/Include/Common/Result.h b/Engine/Source/Common/Include/Common/Result.h new file mode 100644 index 00000000..6f2d5a95 --- /dev/null +++ b/Engine/Source/Common/Include/Common/Result.h @@ -0,0 +1,311 @@ +// +// Created by johnk on 2026/6/20. +// + +#pragma once + +#include +#include +#include +#include +#include + +#include + +namespace Common::Internal { + template + struct OkWrapper { + T value; + }; + + template <> + struct OkWrapper {}; + + template + struct ErrWrapper { + E error; + }; +} + +namespace Common { + template + Internal::OkWrapper> Ok(T&& inValue); + + Internal::OkWrapper Ok(); + + template + Internal::ErrWrapper> Err(E&& inError); + + // A Rust-like Result: holds either a success value of type T or an error of type E, never both. Build one with + // the Ok()/Err() helpers (Result mirrors Rust's Result<(), E> and is constructed from the argument-less + // Ok()). Value()/Error() hand out references to the active alternative; the Unwrap()/Expect() family moves or copies + // it out; Map()/AndThen() and friends chain transformations. Every accessor asserts the active alternative, so check + // IsOk()/IsErr() first when a branch is possible. The value-side accessors take a defaulted T2 = T template + // parameter purely so the void specialization stays well-formed (a member's signature is only ill-formed once it is + // actually instantiated, which never happens for the disabled overloads). + template + class Result { + public: + using ValueType = T; + using ErrorType = E; + + Result(Internal::OkWrapper inOk); // NOLINT + Result(Internal::ErrWrapper inErr); // NOLINT + + bool IsOk() const; + bool IsErr() const; + explicit operator bool() const; + + template requires (!std::is_void_v) T2& Value() &; + template requires (!std::is_void_v) const T2& Value() const&; + E& Error() &; + const E& Error() const&; + + template requires (!std::is_void_v) T2 Unwrap() const&; + template requires (!std::is_void_v) T2 Unwrap() &&; + E UnwrapErr() const&; + E UnwrapErr() &&; + template requires (!std::is_void_v) T2 Expect(std::string_view inReason) const&; + template requires (!std::is_void_v) T2 Expect(std::string_view inReason) &&; + E ExpectErr(std::string_view inReason) const&; + E ExpectErr(std::string_view inReason) &&; + + template requires (!std::is_void_v) T2 UnwrapOr(T2 inFallback) const&; + template requires (!std::is_void_v) T2 UnwrapOrElse(F&& inFunc) const&; + + template requires (!std::is_void_v) Result, E> Map(F&& inFunc) const&; + template Result> MapErr(F&& inFunc) const&; + template requires (!std::is_void_v) std::invoke_result_t AndThen(F&& inFunc) const&; + template std::invoke_result_t OrElse(F&& inFunc) const&; + + template requires (!std::is_void_v) std::optional ToOptional() const&; + + private: + using StoredValueType = std::conditional_t, Internal::OkWrapper, T>; + + static StoredValueType ExtractOk(Internal::OkWrapper&& inOk); + + std::variant storage; + }; +} + +namespace Common { + template + Internal::OkWrapper> Ok(T&& inValue) + { + return { std::forward(inValue) }; + } + + inline Internal::OkWrapper Ok() + { + return {}; + } + + template + Internal::ErrWrapper> Err(E&& inError) + { + return { std::forward(inError) }; + } + + template + Result::Result(Internal::OkWrapper inOk) + : storage(std::in_place_index<0>, ExtractOk(std::move(inOk))) + { + } + + template + Result::Result(Internal::ErrWrapper inErr) + : storage(std::in_place_index<1>, std::move(inErr.error)) + { + } + + template + bool Result::IsOk() const + { + return storage.index() == 0; + } + + template + bool Result::IsErr() const + { + return storage.index() == 1; + } + + template + Result::operator bool() const + { + return IsOk(); + } + + template + template requires (!std::is_void_v) + T2& Result::Value() & + { + Assert(IsOk()); + return std::get<0>(storage); + } + + template + template requires (!std::is_void_v) + const T2& Result::Value() const& + { + Assert(IsOk()); + return std::get<0>(storage); + } + + template + E& Result::Error() & + { + Assert(IsErr()); + return std::get<1>(storage); + } + + template + const E& Result::Error() const& + { + Assert(IsErr()); + return std::get<1>(storage); + } + + template + template requires (!std::is_void_v) + T2 Result::Unwrap() const& + { + Assert(IsOk()); + return std::get<0>(storage); + } + + template + template requires (!std::is_void_v) + T2 Result::Unwrap() && + { + Assert(IsOk()); + return std::get<0>(std::move(storage)); + } + + template + E Result::UnwrapErr() const& + { + Assert(IsErr()); + return std::get<1>(storage); + } + + template + E Result::UnwrapErr() && + { + Assert(IsErr()); + return std::get<1>(std::move(storage)); + } + + template + template requires (!std::is_void_v) + T2 Result::Expect(std::string_view inReason) const& + { + AssertWithReason(IsOk(), inReason); + return std::get<0>(storage); + } + + template + template requires (!std::is_void_v) + T2 Result::Expect(std::string_view inReason) && + { + AssertWithReason(IsOk(), inReason); + return std::get<0>(std::move(storage)); + } + + template + E Result::ExpectErr(std::string_view inReason) const& + { + AssertWithReason(IsErr(), inReason); + return std::get<1>(storage); + } + + template + E Result::ExpectErr(std::string_view inReason) && + { + AssertWithReason(IsErr(), inReason); + return std::get<1>(std::move(storage)); + } + + template + template requires (!std::is_void_v) + T2 Result::UnwrapOr(T2 inFallback) const& + { + return IsOk() ? std::get<0>(storage) : std::move(inFallback); + } + + template + template requires (!std::is_void_v) + T2 Result::UnwrapOrElse(F&& inFunc) const& + { + return IsOk() ? std::get<0>(storage) : std::forward(inFunc)(std::get<1>(storage)); + } + + template + template requires (!std::is_void_v) + Result, E> Result::Map(F&& inFunc) const& + { + if (IsErr()) { + return Err(std::get<1>(storage)); + } + return Ok(std::forward(inFunc)(std::get<0>(storage))); + } + + template + template + Result> Result::MapErr(F&& inFunc) const& + { + if (IsErr()) { + return Err(std::forward(inFunc)(std::get<1>(storage))); + } + if constexpr (std::is_void_v) { + return Ok(); + } else { + return Ok(std::get<0>(storage)); + } + } + + template + template requires (!std::is_void_v) + std::invoke_result_t Result::AndThen(F&& inFunc) const& + { + if (IsErr()) { + return Err(std::get<1>(storage)); + } + return std::forward(inFunc)(std::get<0>(storage)); + } + + template + template + std::invoke_result_t Result::OrElse(F&& inFunc) const& + { + if (IsErr()) { + return std::forward(inFunc)(std::get<1>(storage)); + } + if constexpr (std::is_void_v) { + return Ok(); + } else { + return Ok(std::get<0>(storage)); + } + } + + template + template requires (!std::is_void_v) + std::optional Result::ToOptional() const& + { + if (IsErr()) { + return std::nullopt; + } + return std::get<0>(storage); + } + + template + typename Result::StoredValueType Result::ExtractOk(Internal::OkWrapper&& inOk) + { + if constexpr (std::is_void_v) { + return Internal::OkWrapper {}; + } else { + return std::move(inOk.value); + } + } +} diff --git a/Engine/Source/Common/Test/ResultTest.cpp b/Engine/Source/Common/Test/ResultTest.cpp new file mode 100644 index 00000000..7ae5b216 --- /dev/null +++ b/Engine/Source/Common/Test/ResultTest.cpp @@ -0,0 +1,141 @@ +// +// Created by johnk on 2026/6/20. +// + +#include +#include + +#include + +#include +using namespace Common; + +TEST(ResultTest, OkTest) +{ + const Result result = Ok(42); + ASSERT_TRUE(result.IsOk()); + ASSERT_FALSE(result.IsErr()); + ASSERT_TRUE(static_cast(result)); + ASSERT_EQ(result.Value(), 42); +} + +TEST(ResultTest, ErrTest) +{ + const Result result = Err(std::string("bad")); + ASSERT_FALSE(result.IsOk()); + ASSERT_TRUE(result.IsErr()); + ASSERT_FALSE(static_cast(result)); + ASSERT_EQ(result.Error(), "bad"); +} + +TEST(ResultTest, UnwrapTest) +{ + const Result ok = Ok(42); + ASSERT_EQ(ok.Unwrap(), 42); + + const Result err = Err(std::string("bad")); + ASSERT_EQ(err.UnwrapErr(), "bad"); +} + +TEST(ResultTest, ExpectTest) +{ + const Result ok = Ok(42); + ASSERT_EQ(ok.Expect("must be ok"), 42); + + const Result err = Err(std::string("bad")); + ASSERT_EQ(err.ExpectErr("must be err"), "bad"); +} + +TEST(ResultTest, UnwrapOrTest) +{ + const Result ok = Ok(1); + ASSERT_EQ(ok.UnwrapOr(7), 1); + + const Result err = Err(std::string("bad")); + ASSERT_EQ(err.UnwrapOr(7), 7); + ASSERT_EQ(err.UnwrapOrElse([](const std::string& e) -> int { return static_cast(e.size()); }), 3); +} + +TEST(ResultTest, MapTest) +{ + const Result ok = Ok(21); + const Result mapped = ok.Map([](const int& v) -> int { return v * 2; }); + ASSERT_TRUE(mapped.IsOk()); + ASSERT_EQ(mapped.Value(), 42); + + const Result err = Err(std::string("bad")); + const Result mappedErr = err.Map([](const int& v) -> int { return v * 2; }); + ASSERT_TRUE(mappedErr.IsErr()); + ASSERT_EQ(mappedErr.Error(), "bad"); +} + +TEST(ResultTest, MapErrTest) +{ + const Result err = Err(std::string("bad")); + const Result mapped = err.MapErr([](const std::string& e) -> size_t { return e.size(); }); + ASSERT_TRUE(mapped.IsErr()); + ASSERT_EQ(mapped.Error(), 3); +} + +TEST(ResultTest, AndThenTest) +{ + const Result ok = Ok(4); + const Result chained = ok.AndThen([](const int& v) -> Result { + return v > 0 ? Result(Ok(v * 10)) : Result(Err(std::string("non positive"))); + }); + ASSERT_TRUE(chained.IsOk()); + ASSERT_EQ(chained.Value(), 40); +} + +TEST(ResultTest, OrElseTest) +{ + const Result err = Err(std::string("bad")); + const Result recovered = err.OrElse([](const std::string& e) -> Result { + return Err(static_cast(e.size())); + }); + ASSERT_TRUE(recovered.IsErr()); + ASSERT_EQ(recovered.Error(), 3); +} + +TEST(ResultTest, ToOptionalTest) +{ + const Result ok = Ok(42); + ASSERT_EQ(ok.ToOptional(), std::optional(42)); + + const Result err = Err(std::string("bad")); + ASSERT_FALSE(err.ToOptional().has_value()); +} + +TEST(ResultTest, VoidResultTest) +{ + const Result ok = Ok(); + ASSERT_TRUE(ok.IsOk()); + + const Result err = Err(std::string("bad")); + ASSERT_TRUE(err.IsErr()); + ASSERT_EQ(err.Error(), "bad"); + + const Result mapped = err.MapErr([](const std::string& e) -> size_t { return e.size(); }); + ASSERT_TRUE(mapped.IsErr()); + ASSERT_EQ(mapped.Error(), 3); +} + +TEST(ResultTest, MoveOnlyValueTest) +{ + Result, std::string> result = Ok(std::make_unique(5)); + ASSERT_TRUE(result.IsOk()); + + const std::unique_ptr value = std::move(result).Unwrap(); + ASSERT_EQ(*value, 5); +} + +TEST(ResultTest, SameValueAndErrorTypeTest) +{ + const Result ok = Ok(std::string("value")); + ASSERT_TRUE(ok.IsOk()); + ASSERT_EQ(ok.Value(), "value"); + + const Result err = Err(std::string("error")); + ASSERT_TRUE(err.IsErr()); + ASSERT_EQ(err.Error(), "error"); +} diff --git a/Engine/Source/Core/Include/Core/Cmdline.h b/Engine/Source/Core/Include/Core/Cmdline.h index 4a88457a..614f9ea5 100644 --- a/Engine/Source/Core/Include/Core/Cmdline.h +++ b/Engine/Source/Core/Include/Core/Cmdline.h @@ -11,6 +11,7 @@ #include #include +#include namespace Core { class CORE_API CmdlineArg { @@ -32,7 +33,7 @@ namespace Core { static Cli& Get(); ~Cli(); - std::pair Parse(int argc, char* argv[], bool force = false); + Common::Result Parse(int argc, char* argv[], bool force = false); CmdlineArg* FindArg(const std::string& name) const; CmdlineArg& GetArg(const std::string& name) const; diff --git a/Engine/Source/Core/Src/Cmdline.cpp b/Engine/Source/Core/Src/Cmdline.cpp index 8a2fc88f..7bf1170e 100644 --- a/Engine/Source/Core/Src/Cmdline.cpp +++ b/Engine/Source/Core/Src/Cmdline.cpp @@ -24,7 +24,7 @@ namespace Core { Cli::~Cli() = default; - std::pair Cli::Parse(int argc, char* argv[], bool force) + Common::Result Cli::Parse(int argc, char* argv[], bool force) { if (!force) { Assert(!parsed); @@ -41,9 +41,9 @@ namespace Core { if (!clipp::parse(argc, argv, cli)) { std::stringstream stream; stream << clipp::make_man_page(cli, argv[0]); - return std::make_pair(false, stream.str()); + return Common::Err(stream.str()); } - return std::make_pair(true, ""); + return Common::Ok(); } CmdlineArg* Cli::FindArg(const std::string& name) const diff --git a/Engine/Source/Core/Test/CmdlineTest.cpp b/Engine/Source/Core/Test/CmdlineTest.cpp index e0a45e65..f52f2782 100644 --- a/Engine/Source/Core/Test/CmdlineTest.cpp +++ b/Engine/Source/Core/Test/CmdlineTest.cpp @@ -33,8 +33,8 @@ TEST(CmdlineTest, BasicTest) const_cast("world"), }; - const auto [result, errorInfo] = Core::Cli::Get().Parse(static_cast(args.size()), args.data(), true); - ASSERT_TRUE(result); + const auto result = Core::Cli::Get().Parse(static_cast(args.size()), args.data(), true); + ASSERT_TRUE(result.IsOk()); ASSERT_TRUE(arg0.GetValue()); ASSERT_EQ(arg1.GetValue(), 1); ASSERT_EQ(arg2.GetValue(), "hello"); diff --git a/Tool/MirrorTool/ExeSrc/Main.cpp b/Tool/MirrorTool/ExeSrc/Main.cpp index 855a5382..49df956a 100644 --- a/Tool/MirrorTool/ExeSrc/Main.cpp +++ b/Tool/MirrorTool/ExeSrc/Main.cpp @@ -97,16 +97,16 @@ int main(int argc, char* argv[]) // NOLINT } MirrorTool::Parser parser(inputFile, headerDirs, frameworkDirs); - auto [parseSuccess, parseResultOrError] = parser.Parse(); - if (!parseSuccess) { - outputErrorWithDebugContext(std::get(parseResultOrError)); + const auto parseResult = parser.Parse(); + if (parseResult.IsErr()) { + outputErrorWithDebugContext(parseResult.Error()); return 1; } - MirrorTool::Generator generator(inputFile, outputFile, headerDirs, std::get(parseResultOrError), dynamic); - if (auto [generateSuccess, generateError] = generator.Generate(); - !generateSuccess) { - outputErrorWithDebugContext(generateError); + MirrorTool::Generator generator(inputFile, outputFile, headerDirs, parseResult.Value(), dynamic); + if (const auto generateResult = generator.Generate(); + generateResult.IsErr()) { + outputErrorWithDebugContext(generateResult.Error()); return 1; } return 0; diff --git a/Tool/MirrorTool/Include/MirrorTool/Generator.h b/Tool/MirrorTool/Include/MirrorTool/Generator.h index f0f3b155..36605c7e 100644 --- a/Tool/MirrorTool/Include/MirrorTool/Generator.h +++ b/Tool/MirrorTool/Include/MirrorTool/Generator.h @@ -7,6 +7,7 @@ #include #include +#include #include @@ -15,7 +16,7 @@ namespace MirrorTool { class Generator { public: - using Result = std::pair; + using Result = Common::Result; NonCopyable(Generator) explicit Generator(std::string inInputFile, std::string inOutputFile, std::vector inHeaderDirs, const MetaInfo& inMetaInfo, bool inDynamic); diff --git a/Tool/MirrorTool/Include/MirrorTool/Parser.h b/Tool/MirrorTool/Include/MirrorTool/Parser.h index c298b209..6fdf9f59 100644 --- a/Tool/MirrorTool/Include/MirrorTool/Parser.h +++ b/Tool/MirrorTool/Include/MirrorTool/Parser.h @@ -6,12 +6,12 @@ #include #include -#include #include #include #include +#include namespace MirrorTool { enum class FieldAccess : uint8_t { @@ -91,7 +91,7 @@ namespace MirrorTool { class Parser { public: - using Result = std::pair>; + using Result = Common::Result; NonCopyable(Parser) explicit Parser(std::string inSourceFile, std::vector inHeaderDirs, std::vector inFrameworkDirs); diff --git a/Tool/MirrorTool/Src/Generator.cpp b/Tool/MirrorTool/Src/Generator.cpp index b2c12fd6..c989aff4 100644 --- a/Tool/MirrorTool/Src/Generator.cpp +++ b/Tool/MirrorTool/Src/Generator.cpp @@ -456,12 +456,12 @@ namespace MirrorTool { std::ifstream inFile(inputFile); if (inFile.fail()) { - return std::make_pair(false, "failed to open input file"); + return Common::Err("failed to open input file"); } std::ofstream outFile(outputFile); if (outFile.fail()) { - return std::make_pair(false, "failed to open output file"); + return Common::Err("failed to open output file"); } auto result = GenerateCode(inFile, outFile, Common::HashUtils::CityHash(outputFile.data(), outputFile.size())); @@ -473,7 +473,7 @@ namespace MirrorTool { { std::string bestMatchHeaderPath = GetBestMatchHeaderPath(inputFile, headerDirs); if (bestMatchHeaderPath.empty()) { - return std::make_pair(false, "failed to compute best match header path"); + return Common::Err("failed to compute best match header path"); } outFile << GetHeaderNote() << Common::newline; @@ -482,6 +482,6 @@ namespace MirrorTool { outFile << GetGlobalCode(metaInfo, uniqueId, dynamic); outFile << GetEnumsCode(metaInfo, uniqueId, dynamic); outFile << GetClassesCode(metaInfo, dynamic); - return std::make_pair(true, ""); + return Common::Ok(); } } diff --git a/Tool/MirrorTool/Src/Parser.cpp b/Tool/MirrorTool/Src/Parser.cpp index a1641180..4adb890e 100644 --- a/Tool/MirrorTool/Src/Parser.cpp +++ b/Tool/MirrorTool/Src/Parser.cpp @@ -539,7 +539,7 @@ namespace MirrorTool { VisitChildren(OutermostVisitor, MetaInfo, cursor, metaInfo); Cleanup(index, translationUnit); - return std::make_pair(true, std::move(metaInfo)); + return Common::Ok(std::move(metaInfo)); } void Parser::Cleanup(CXIndex index, CXTranslationUnit translationUnit) @@ -555,6 +555,6 @@ namespace MirrorTool { Parser::Result Parser::CleanUpAndConstructFailResult(CXIndex index, CXTranslationUnit translationUnit, std::string reason) { Cleanup(index, translationUnit); - return std::make_pair(false, std::move(reason)); + return Common::Err(std::move(reason)); } } diff --git a/Tool/MirrorTool/Test/Main.cpp b/Tool/MirrorTool/Test/Main.cpp index cb0eaaa3..bd9d9ab2 100644 --- a/Tool/MirrorTool/Test/Main.cpp +++ b/Tool/MirrorTool/Test/Main.cpp @@ -121,10 +121,10 @@ void AssertNamespaceInfoEqual(const NamespaceInfo& lhs, const NamespaceInfo& rhs TEST(MirrorTest, ParserTest) { const Parser parser("../Test/Resource/Mirror/MirrorToolInput.h", { "../Test/Resource/Mirror" }, {}); - auto [parseSuccess, parseResultOrError] = parser.Parse(); - ASSERT_TRUE(parseSuccess); + const auto parseResult = parser.Parse(); + ASSERT_TRUE(parseResult.IsOk()); - const auto& [namespaces, global] = std::get(parseResultOrError); + const auto& [namespaces, global] = parseResult.Value(); ASSERT_EQ(namespaces.size(), 0); NamespaceInfo predicatedGlobalNamespace = { "", "", {} }; @@ -161,12 +161,12 @@ TEST(MirrorTest, ParserTest) TEST(MirrorTest, GeneratorTest) { const Parser parser("../Test/Resource/Mirror/MirrorToolInput.h", { "../Test/Resource/Mirror" }, {}); - auto [parseSuccess, parseResultOrError] = parser.Parse(); - ASSERT_TRUE(parseSuccess); + const auto parseResult = parser.Parse(); + ASSERT_TRUE(parseResult.IsOk()); - const Generator generator("../Test/Resource/Mirror/MirrorToolInput.h", "../Test/Generated/Mirror/MirrorToolTest.generated.cpp", { "../" }, std::get(parseResultOrError), false); - auto [generateSuccess, generateResultOrError] = generator.Generate(); - ASSERT_EQ(generateSuccess, true); + const Generator generator("../Test/Resource/Mirror/MirrorToolInput.h", "../Test/Generated/Mirror/MirrorToolTest.generated.cpp", { "../" }, parseResult.Value(), false); + const auto generateResult = generator.Generate(); + ASSERT_TRUE(generateResult.IsOk()); } int main(int argc, char* argv[]) From 8c49d906e95429116ebbdb45182e631b3956793b Mon Sep 17 00:00:00 2001 From: kindem Date: Sat, 20 Jun 2026 13:11:02 +0800 Subject: [PATCH 3/3] feat: make FileUtils return Result instead of asserting Change FileUtils::ReadTextFile/WriteTextFile/ReadJsonFile/WriteJsonFile to return Result<..., std::string> so callers can handle IO failures instead of the functions hard-asserting (or, for ReadJsonFile, silently passing a null FILE* into rapidjson and crashing). ReadJsonFile now also reports fopen failures and JSON parse errors as Err. Update all call sites (Serialization, Render, Core, Runtime, Editor, Sample) to the new signatures, and cover the error path with new FileTest cases. --- Editor/Src/Widget/GraphicsSampleWidget.cpp | 4 +- Engine/Source/Common/Include/Common/File.h | 10 +-- .../Common/Include/Common/Serialization.h | 4 +- Engine/Source/Common/Src/File.cpp | 63 ++++++++++++------- Engine/Source/Common/Test/FileTest.cpp | 20 +++++- Engine/Source/Core/Src/Console.cpp | 2 +- Engine/Source/Render/Src/Shader.cpp | 2 +- Engine/Source/Render/Src/ShaderCompiler.cpp | 2 +- Engine/Source/Runtime/Src/Asset/Material.cpp | 4 +- .../Source/Runtime/Src/Settings/Registry.cpp | 4 +- Sample/Base/Application.cpp | 2 +- 11 files changed, 75 insertions(+), 42 deletions(-) diff --git a/Editor/Src/Widget/GraphicsSampleWidget.cpp b/Editor/Src/Widget/GraphicsSampleWidget.cpp index 74fb23cb..0148b8de 100644 --- a/Editor/Src/Widget/GraphicsSampleWidget.cpp +++ b/Editor/Src/Widget/GraphicsSampleWidget.cpp @@ -34,7 +34,7 @@ namespace Editor { { Render::ShaderCompileInput shaderCompileInput; - shaderCompileInput.source = Common::FileUtils::ReadTextFile("../Shader/Editor/GraphicsWindowSample.esl"); + shaderCompileInput.source = Common::FileUtils::ReadTextFile("../Shader/Editor/GraphicsWindowSample.esl").Unwrap(); shaderCompileInput.stage = RHI::ShaderStageBits::sVertex; shaderCompileInput.entryPoint = "VSMain"; shaderCompileInput.includeDirectories.emplace_back("../Test/Sample/ShaderInclude"); @@ -43,7 +43,7 @@ namespace Editor { { Render::ShaderCompileInput shaderCompileInput; - shaderCompileInput.source = Common::FileUtils::ReadTextFile("../Shader/Editor/GraphicsWindowSample.esl"); + shaderCompileInput.source = Common::FileUtils::ReadTextFile("../Shader/Editor/GraphicsWindowSample.esl").Unwrap(); shaderCompileInput.stage = RHI::ShaderStageBits::sPixel; shaderCompileInput.entryPoint = "PSMain"; shaderCompileInput.includeDirectories.emplace_back("../Test/Sample/ShaderInclude"); diff --git a/Engine/Source/Common/Include/Common/File.h b/Engine/Source/Common/Include/Common/File.h index bb685591..1f8385e8 100644 --- a/Engine/Source/Common/Include/Common/File.h +++ b/Engine/Source/Common/Include/Common/File.h @@ -8,12 +8,14 @@ #include +#include + namespace Common { class FileUtils { public: - static std::string ReadTextFile(const std::string& inFileName); - static void WriteTextFile(const std::string& inFileName, const std::string& inContent); - static rapidjson::Document ReadJsonFile(const std::string& inFileName); - static void WriteJsonFile(const std::string& inFileName, const rapidjson::Document& inJsonDocument, bool inPretty = true); + static Result ReadTextFile(const std::string& inFileName); + static Result WriteTextFile(const std::string& inFileName, const std::string& inContent); + static Result ReadJsonFile(const std::string& inFileName); + static Result WriteJsonFile(const std::string& inFileName, const rapidjson::Document& inJsonDocument, bool inPretty = true); }; } diff --git a/Engine/Source/Common/Include/Common/Serialization.h b/Engine/Source/Common/Include/Common/Serialization.h index 897231f7..9c575d7f 100644 --- a/Engine/Source/Common/Include/Common/Serialization.h +++ b/Engine/Source/Common/Include/Common/Serialization.h @@ -477,12 +477,12 @@ namespace Common { { rapidjson::Document document; JsonSerialize(document, document.GetAllocator(), inValue); - FileUtils::WriteJsonFile(inFile, document, inPretty); + Assert(FileUtils::WriteJsonFile(inFile, document, inPretty).IsOk()); } template void JsonDeserializeFromFile(const std::string& inFile, T& outValue) { - const rapidjson::Document document = FileUtils::ReadJsonFile(inFile); + const rapidjson::Document document = FileUtils::ReadJsonFile(inFile).Unwrap(); JsonDeserialize(document, outValue); } diff --git a/Engine/Source/Common/Src/File.cpp b/Engine/Source/Common/Src/File.cpp index 5b3255e2..461fccc8 100644 --- a/Engine/Source/Common/Src/File.cpp +++ b/Engine/Source/Common/Src/File.cpp @@ -4,33 +4,35 @@ #include #include +#include #include #include #include #include +#include #include -#include #include namespace Common { - std::string FileUtils::ReadTextFile(const std::string& inFileName) + Result FileUtils::ReadTextFile(const std::string& inFileName) { - std::string result; - { - std::ifstream file(inFileName, std::ios::ate | std::ios::binary); - Assert(file.is_open()); - const size_t size = file.tellg(); - result.resize(size); - file.seekg(0); - file.read(result.data(), static_cast(size)); - file.close(); + std::ifstream file(inFileName, std::ios::ate | std::ios::binary); + if (!file.is_open()) { + return Err(std::format("failed to open file '{}' for reading", inFileName)); } - return result; + + const size_t size = file.tellg(); + std::string result; + result.resize(size); + file.seekg(0); + file.read(result.data(), static_cast(size)); + file.close(); + return Ok(std::move(result)); } - void FileUtils::WriteTextFile(const std::string& inFileName, const std::string& inContent) + Result FileUtils::WriteTextFile(const std::string& inFileName, const std::string& inContent) { const Path path(inFileName); if (const Path parentPath = path.Parent(); @@ -39,34 +41,48 @@ namespace Common { } std::ofstream file(inFileName, std::ios::out | std::ios::binary | std::ios::trunc); - Assert(file.is_open()); + if (!file.is_open()) { + return Err(std::format("failed to open file '{}' for writing", inFileName)); + } file.write(inContent.data(), static_cast(inContent.size())); file.close(); + return Ok(); } - rapidjson::Document FileUtils::ReadJsonFile(const std::string& inFileName) + Result FileUtils::ReadJsonFile(const std::string& inFileName) { - char buffer[65536]; std::FILE* file = fopen(inFileName.c_str(), "rb"); // NOLINT - rapidjson::FileReadStream stream(file, buffer, sizeof(buffer)); + if (file == nullptr) { + return Err(std::format("failed to open json file '{}' for reading", inFileName)); + } + char buffer[65536]; + rapidjson::FileReadStream stream(file, buffer, sizeof(buffer)); rapidjson::Document document; document.ParseStream(stream); (void) fclose(file); - return document; + + if (document.HasParseError()) { + return Err(std::format("failed to parse json file '{}' (error code {} at offset {})", + inFileName, static_cast(document.GetParseError()), document.GetErrorOffset())); + } + return Ok(std::move(document)); } - void FileUtils::WriteJsonFile(const std::string& inFileName, const rapidjson::Document& inJsonDocument, bool inPretty) + Result FileUtils::WriteJsonFile(const std::string& inFileName, const rapidjson::Document& inJsonDocument, bool inPretty) { - Common::Path parentPath = Common::Path(inFileName).Parent(); - if (!parentPath.Exists()) { + if (Path parentPath = Path(inFileName).Parent(); + !parentPath.Exists()) { parentPath.MakeDir(); } - char buffer[65536]; std::FILE* file = fopen(inFileName.c_str(), "wb"); // NOLINT - rapidjson::FileWriteStream stream(file, buffer, sizeof(buffer)); + if (file == nullptr) { + return Err(std::format("failed to open json file '{}' for writing", inFileName)); + } + char buffer[65536]; + rapidjson::FileWriteStream stream(file, buffer, sizeof(buffer)); if (inPretty) { rapidjson::PrettyWriter writer(stream); inJsonDocument.Accept(writer); @@ -75,5 +91,6 @@ namespace Common { inJsonDocument.Accept(writer); } (void) fclose(file); + return Ok(); } } diff --git a/Engine/Source/Common/Test/FileTest.cpp b/Engine/Source/Common/Test/FileTest.cpp index 8e14fba1..89fefcfb 100644 --- a/Engine/Source/Common/Test/FileTest.cpp +++ b/Engine/Source/Common/Test/FileTest.cpp @@ -10,7 +10,21 @@ TEST(FileTest, ReadWriteTextFileTest) { static Common::Path file = "../Test/Generated/Common/ReadTextFileTest.txt"; - Common::FileUtils::WriteTextFile(file.Absolute().String(), "hello"); - const std::string content = Common::FileUtils::ReadTextFile(file.Absolute().String()); - ASSERT_EQ(content, "hello"); + ASSERT_TRUE(Common::FileUtils::WriteTextFile(file.Absolute().String(), "hello").IsOk()); + const auto readResult = Common::FileUtils::ReadTextFile(file.Absolute().String()); + ASSERT_TRUE(readResult.IsOk()); + ASSERT_EQ(readResult.Value(), "hello"); +} + +TEST(FileTest, ReadMissingTextFileTest) +{ + const auto readResult = Common::FileUtils::ReadTextFile("../Test/Generated/Common/DoesNotExist.txt"); + ASSERT_TRUE(readResult.IsErr()); + ASSERT_FALSE(readResult.Error().empty()); +} + +TEST(FileTest, ReadMissingJsonFileTest) +{ + const auto readResult = Common::FileUtils::ReadJsonFile("../Test/Generated/Common/DoesNotExist.json"); + ASSERT_TRUE(readResult.IsErr()); } diff --git a/Engine/Source/Core/Src/Console.cpp b/Engine/Source/Core/Src/Console.cpp index 0abf12fa..dd7c4b28 100644 --- a/Engine/Source/Core/Src/Console.cpp +++ b/Engine/Source/Core/Src/Console.cpp @@ -102,7 +102,7 @@ namespace Core { continue; } - rapidjson::Document document = Common::FileUtils::ReadJsonFile(path.String()); + rapidjson::Document document = Common::FileUtils::ReadJsonFile(path.String()).Unwrap(); Assert(document.IsObject()); for (auto iter = document.MemberBegin(); iter != document.MemberEnd(); ++iter) { diff --git a/Engine/Source/Render/Src/Shader.cpp b/Engine/Source/Render/Src/Shader.cpp index d5f64996..73c510e0 100644 --- a/Engine/Source/Render/Src/Shader.cpp +++ b/Engine/Source/Render/Src/Shader.cpp @@ -152,7 +152,7 @@ namespace Render { void ShaderUtils::GatherShaderSources(std::unordered_map& outFileAndSource, const std::string& inSourceFile, const std::vector& inIncludeDirectories) { - const std::string text = Common::FileUtils::ReadTextFile(inSourceFile); + const std::string text = Common::FileUtils::ReadTextFile(inSourceFile).Unwrap(); outFileAndSource.emplace(inSourceFile, text); for (const auto includes = Common::StringUtils::RegexSearch(text, "#include \\<.*\\>"); diff --git a/Engine/Source/Render/Src/ShaderCompiler.cpp b/Engine/Source/Render/Src/ShaderCompiler.cpp index d3097b6c..4ea7e989 100644 --- a/Engine/Source/Render/Src/ShaderCompiler.cpp +++ b/Engine/Source/Render/Src/ShaderCompiler.cpp @@ -414,7 +414,7 @@ namespace Render { const auto stage = shaderType->GetStage(); const auto& entryPoint = shaderType->GetEntryPoint(); const auto& variantFields = shaderType->GetVariantFields(); - const auto source = Common::FileUtils::ReadTextFile(sourceFile); + const auto source = Common::FileUtils::ReadTextFile(sourceFile).Unwrap(); Assert(!compileOutputs.contains(typeKey)); compileOutputs.emplace(std::make_pair(typeKey, std::unordered_map> {})); diff --git a/Engine/Source/Runtime/Src/Asset/Material.cpp b/Engine/Source/Runtime/Src/Asset/Material.cpp index 6d99b1c4..509eccec 100644 --- a/Engine/Source/Runtime/Src/Asset/Material.cpp +++ b/Engine/Source/Runtime/Src/Asset/Material.cpp @@ -250,8 +250,8 @@ namespace Runtime { !absoluteMaterialRootCacheDir.Exists()) { absoluteMaterialRootCacheDir.MakeDir(); } - Common::FileUtils::WriteTextFile(Core::Paths::Translate(materialHintFile).Absolute().String(), ""); - Common::FileUtils::WriteTextFile(Core::Paths::Translate(materialHeader).Absolute().String(), source); + Assert(Common::FileUtils::WriteTextFile(Core::Paths::Translate(materialHintFile).Absolute().String(), "").IsOk()); + Assert(Common::FileUtils::WriteTextFile(Core::Paths::Translate(materialHeader).Absolute().String(), source).IsOk()); shaderTypes.clear(); for (const Render::VertexFactoryType* vertexFactoryType : Render::VertexFactoryTypeRegistry::Get().AllTypes()) { diff --git a/Engine/Source/Runtime/Src/Settings/Registry.cpp b/Engine/Source/Runtime/Src/Settings/Registry.cpp index e39f07d8..62f59d1b 100644 --- a/Engine/Source/Runtime/Src/Settings/Registry.cpp +++ b/Engine/Source/Runtime/Src/Settings/Registry.cpp @@ -46,7 +46,7 @@ namespace Runtime { return; } - const rapidjson::Document document = Common::FileUtils::ReadJsonFile(configPath.String()); + const rapidjson::Document document = Common::FileUtils::ReadJsonFile(configPath.String()).Unwrap(); Assert(document.IsObject()); settingsMap.at(inClass).JsonDeserialize(document); } @@ -68,7 +68,7 @@ namespace Runtime { settingsMap.at(inClass).JsonSerialize(document, document.GetAllocator()); const auto configPath = Internal::GetConfigPathForSettings(inClass); - Common::FileUtils::WriteJsonFile(configPath.String(), document); + Assert(Common::FileUtils::WriteJsonFile(configPath.String(), document).IsOk()); } void SettingsRegistry::SaveAllSettings() const diff --git a/Sample/Base/Application.cpp b/Sample/Base/Application.cpp index 6052ac49..37319791 100644 --- a/Sample/Base/Application.cpp +++ b/Sample/Base/Application.cpp @@ -263,7 +263,7 @@ Camera& Application::GetCamera() const Application::ShaderCompileOutput Application::CompileShader(const std::string& fileName, const std::string& entryPoint, RHI::ShaderStageBits shaderStage, std::vector includePaths) const { - std::string shaderSource = FileUtils::ReadTextFile(fileName); + std::string shaderSource = FileUtils::ReadTextFile(fileName).Unwrap(); Render::ShaderCompileInput input; input.source = shaderSource;