From 24c9a41fc4c7a5df297f1eb64253806f034571cc Mon Sep 17 00:00:00 2001 From: Anutosh Bhat <87052487+anutosh491@users.noreply.github.com> Date: Tue, 18 Jun 2024 15:39:03 +0530 Subject: [PATCH] Refactored register_preamble (#149) --- include/xeus-cpp/xholder.hpp | 6 +++--- include/xeus-cpp/xmanager.hpp | 8 ++++---- include/xeus-cpp/xpreamble.hpp | 2 +- src/xholder.cpp | 14 ++++++-------- src/xinspect.hpp | 4 ++-- src/xinterpreter.cpp | 7 +++---- src/xsystem.hpp | 4 ++-- test/test_interpreter.cpp | 25 ++++++++++++------------- 8 files changed, 33 insertions(+), 37 deletions(-) diff --git a/include/xeus-cpp/xholder.hpp b/include/xeus-cpp/xholder.hpp index 4f7a38d6..450c45a2 100644 --- a/include/xeus-cpp/xholder.hpp +++ b/include/xeus-cpp/xholder.hpp @@ -30,12 +30,12 @@ namespace xcpp ~xholder_preamble(); xholder_preamble(const xholder_preamble& rhs); xholder_preamble(xholder_preamble&& rhs); - xholder_preamble(xpreamble* holder); + xholder_preamble(std::unique_ptr holder); xholder_preamble& operator=(const xholder_preamble& rhs); xholder_preamble& operator=(xholder_preamble&& rhs); - xholder_preamble& operator=(xpreamble* holder); + xholder_preamble& operator=(std::unique_ptr holder); void swap(xholder_preamble& rhs) { @@ -53,7 +53,7 @@ namespace xcpp private: - xpreamble* p_holder; + std::unique_ptr p_holder; }; } #endif diff --git a/include/xeus-cpp/xmanager.hpp b/include/xeus-cpp/xmanager.hpp index 483c0632..3292ce39 100644 --- a/include/xeus-cpp/xmanager.hpp +++ b/include/xeus-cpp/xmanager.hpp @@ -31,9 +31,9 @@ namespace xcpp std::map preamble; template - void register_preamble(const std::string& name, preamble_type* pre) + void register_preamble(const std::string& name, std::unique_ptr pre) { - preamble[name] = xholder_preamble(pre); + preamble[name] = xholder_preamble(std::move(pre)); } void unregister_preamble(const std::string& name) @@ -186,9 +186,9 @@ namespace xcpp } } - virtual xpreamble* clone() const override + [[nodiscard]] std::unique_ptr clone() const override { - return new xmagics_manager(*this); + return std::make_unique(*this); } private: diff --git a/include/xeus-cpp/xpreamble.hpp b/include/xeus-cpp/xpreamble.hpp index 9d6b66ea..60055578 100644 --- a/include/xeus-cpp/xpreamble.hpp +++ b/include/xeus-cpp/xpreamble.hpp @@ -30,7 +30,7 @@ namespace xcpp } virtual void apply(const std::string& s, nl::json& kernel_res) = 0; - virtual xpreamble* clone() const = 0; + virtual std::unique_ptr clone() const = 0; virtual ~xpreamble(){}; }; } diff --git a/src/xholder.cpp b/src/xholder.cpp index 44938564..6a4ad768 100644 --- a/src/xholder.cpp +++ b/src/xholder.cpp @@ -24,23 +24,22 @@ namespace xcpp { } - xholder_preamble::xholder_preamble(xpreamble* holder) - : p_holder(holder) + xholder_preamble::xholder_preamble(std::unique_ptr holder) + : p_holder(std::move(holder)) { } xholder_preamble::~xholder_preamble() { - delete p_holder; } xholder_preamble::xholder_preamble(const xholder_preamble& rhs) - : p_holder(rhs.p_holder ? rhs.p_holder->clone() : nullptr) + : p_holder(rhs.p_holder ? std::move(rhs.p_holder->clone()) : nullptr) { } xholder_preamble::xholder_preamble(xholder_preamble&& rhs) - : p_holder(rhs.p_holder) + : p_holder(std::move(rhs.p_holder)) { rhs.p_holder = nullptr; } @@ -59,10 +58,9 @@ namespace xcpp return *this; } - xholder_preamble& xholder_preamble::operator=(xpreamble* holder) + xholder_preamble& xholder_preamble::operator=(std::unique_ptr holder) { - delete p_holder; - p_holder = holder; + p_holder = std::move(holder); return *this; } diff --git a/src/xinspect.hpp b/src/xinspect.hpp index 8ee760de..bdc4a13e 100644 --- a/src/xinspect.hpp +++ b/src/xinspect.hpp @@ -279,9 +279,9 @@ namespace xcpp inspect(to_inspect[1], kernel_res); } - virtual xpreamble* clone() const override + [[nodiscard]] std::unique_ptr clone() const override { - return new xintrospection(*this); + return std::make_unique(*this); } }; diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 121b90a6..e715d457 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -391,11 +391,10 @@ __get_cxx_version () void interpreter::init_preamble() { - // FIXME: Make register_preamble take a unique_ptr. //NOLINTBEGIN(cppcoreguidelines-owning-memory) - preamble_manager.register_preamble("introspection", new xintrospection()); - preamble_manager.register_preamble("magics", new xmagics_manager()); - preamble_manager.register_preamble("shell", new xsystem()); + preamble_manager.register_preamble("introspection", std::make_unique()); + preamble_manager.register_preamble("magics", std::make_unique()); + preamble_manager.register_preamble("shell", std::make_unique()); //NOLINTEND(cppcoreguidelines-owning-memory) } diff --git a/src/xsystem.hpp b/src/xsystem.hpp index 99d67b1e..159dbeb9 100644 --- a/src/xsystem.hpp +++ b/src/xsystem.hpp @@ -67,9 +67,9 @@ namespace xcpp } } - virtual xpreamble* clone() const override + [[nodiscard]] std::unique_ptr clone() const override { - return new xsystem(*this); + return std::make_unique(*this); } }; } diff --git a/test/test_interpreter.cpp b/test/test_interpreter.cpp index a53a9042..364e1fd7 100644 --- a/test/test_interpreter.cpp +++ b/test/test_interpreter.cpp @@ -407,7 +407,7 @@ TEST_SUITE("clone_magics_manager") { xcpp::xmagics_manager manager; - xcpp::xpreamble* clone = manager.clone(); + std::unique_ptr clone = manager.clone(); REQUIRE(clone != nullptr); } @@ -419,11 +419,9 @@ TEST_SUITE("clone_magics_manager") { xcpp::xmagics_manager manager; - xcpp::xpreamble* clone = manager.clone(); + std::unique_ptr clone = manager.clone(); - REQUIRE(dynamic_cast(clone) != nullptr); - - delete clone; + REQUIRE(clone.get() != nullptr); } } @@ -436,12 +434,13 @@ TEST_SUITE("xpreamble_manager_operator") { std::string name = "test"; xcpp::xpreamble_manager manager; - xcpp::xmagics_manager* magics = new xcpp::xmagics_manager(); - manager.register_preamble(name, magics); + std::unique_ptr magics = std::make_unique(); + auto* raw_ptr = magics.get(); + manager.register_preamble(name, std::move(magics)); xcpp::xholder_preamble& result = manager.operator[](name); - REQUIRE(&(result.get_cast()) == magics); + REQUIRE(&(result.get_cast()) == raw_ptr); } } @@ -642,7 +641,7 @@ TEST_SUITE("xsystem_clone") { xcpp::xsystem system; - xcpp::xpreamble* clone = system.clone(); + std::unique_ptr clone = system.clone(); REQUIRE(clone != nullptr); } @@ -651,9 +650,9 @@ TEST_SUITE("xsystem_clone") { xcpp::xsystem system; - xcpp::xpreamble* clone = system.clone(); + std::unique_ptr clone = system.clone(); - REQUIRE(dynamic_cast(clone) != nullptr); + REQUIRE(clone.get() != nullptr); } } @@ -727,7 +726,7 @@ TEST_SUITE("xmagics_apply"){ xcpp::xpreamble_manager preamble_manager; - preamble_manager.register_preamble("magics", new xcpp::xmagics_manager()); + preamble_manager.register_preamble("magics", std::make_unique()); preamble_manager["magics"].get_cast().register_magic("magic2", MyMagicCell()); @@ -742,7 +741,7 @@ TEST_SUITE("xmagics_apply"){ xcpp::xpreamble_manager preamble_manager; - preamble_manager.register_preamble("magics", new xcpp::xmagics_manager()); + preamble_manager.register_preamble("magics", std::make_unique()); preamble_manager["magics"].get_cast().register_magic("magic1", MyMagicLine());