Slightly rework the stacktrace transform callback, optimize to only copy if needed

jeremy-rifkin · jeremy-rifkin · commit fe794b19e8dc · 2025-04-07T21:07:39.000-05:00
diff --git a/include/cpptrace/formatting.hpp b/include/cpptrace/formatting.hpp
@@ -46,7 +46,7 @@ namespace cpptrace {
         formatter& columns(bool);
         formatter& filtered_frame_placeholders(bool);
         formatter& filter(std::function<bool(const stacktrace_frame&)>);
-        formatter& transform(std::function<stacktrace_frame(stacktrace_frame&&)>);
+        formatter& transform(std::function<stacktrace_frame(stacktrace_frame)>);
 
         std::string format(const stacktrace_frame&) const;
         std::string format(const stacktrace_frame&, bool color) const;
diff --git a/src/formatting.cpp b/src/formatting.cpp
@@ -24,7 +24,7 @@ namespace cpptrace {
             bool columns = true;
             bool show_filtered_frames = true;
             std::function<bool(const stacktrace_frame&)> filter;
-            std::function<stacktrace_frame(stacktrace_frame&&)> transform;
+            std::function<stacktrace_frame(stacktrace_frame)> transform;
         } options;
 
     public:
@@ -55,15 +55,20 @@ namespace cpptrace {
         void filter(std::function<bool(const stacktrace_frame&)> filter) {
             options.filter = filter;
         }
-        void transform(std::function<stacktrace_frame(stacktrace_frame&&)> transform) {
+        void transform(std::function<stacktrace_frame(stacktrace_frame)> transform) {
             options.transform = std::move(transform);
         }
 
-        std::string format(stacktrace_frame frame, detail::optional<bool> color_override = detail::nullopt) const {
+        std::string format(
+            const stacktrace_frame& input_frame,
+            detail::optional<bool> color_override = detail::nullopt
+        ) const {
             std::ostringstream oss;
+            detail::optional<stacktrace_frame> transformed_frame;
             if(options.transform) {
-                frame = options.transform(std::move(frame));
+                transformed_frame = options.transform(input_frame);
             }
+            const stacktrace_frame& frame = options.transform ? transformed_frame.unwrap() : input_frame;
             print_frame_inner(oss, frame, color_override.value_or(options.color == color_mode::always));
             return std::move(oss).str();
         }
@@ -156,10 +161,11 @@ namespace cpptrace {
             }
             const auto frame_number_width = detail::n_digits(static_cast<int>(frames.size()) - 1);
             for(size_t i = 0; i < frames.size(); ++i) {
-                auto frame = frames[i];
+                detail::optional<stacktrace_frame> transformed_frame;
                 if(options.transform) {
-                    frame = options.transform(std::move(frame));
+                    transformed_frame = options.transform(frames[i]);
                 }
+                const stacktrace_frame& frame = options.transform ? transformed_frame.unwrap() : frames[i];
                 if(options.filter && !options.filter(frame)) {
                     if(!options.show_filtered_frames) {
                         counter++;
@@ -304,7 +310,7 @@ namespace cpptrace {
         pimpl->filter(std::move(filter));
         return *this;
     }
-    formatter& formatter::transform(std::function<stacktrace_frame(stacktrace_frame&&)> transform) {
+    formatter& formatter::transform(std::function<stacktrace_frame(stacktrace_frame)> transform) {
         pimpl->transform(std::move(transform));
         return *this;
     }
diff --git a/test/unit/lib/formatting.cpp b/test/unit/lib/formatting.cpp
@@ -257,7 +257,34 @@ TEST(FormatterTest, DontShowFilteredFrames) {
 
 TEST(FormatterTest, Transforming) {
     auto formatter = cpptrace::formatter{}
-        .transform([](cpptrace::stacktrace_frame &&frame) {
+        .transform([](cpptrace::stacktrace_frame frame) {
+            static size_t count = 0;
+            frame.symbol = cpptrace::microfmt::format("sym{}", count++);
+            return frame;
+        });
+    auto res = split(formatter.format(make_test_stacktrace()), "\n");
+    EXPECT_THAT(
+        res,
+        ElementsAre(
+            "Stack trace (most recent call first):",
+            "#0 0x0000000000000001 in sym0 at foo.cpp:20:30",
+            "#1 0x0000000000000002 in sym1 at bar.cpp:30:40",
+            "#2 0x0000000000000003 in sym2 at foo.cpp:40:25"
+        )
+    );
+
+    auto frame_res = split(formatter.format(make_test_stacktrace().frames[1]), "\n");
+    EXPECT_THAT(
+        frame_res,
+        ElementsAre(
+            "0x0000000000000002 in sym3 at bar.cpp:30:40"
+        )
+    );
+}
+
+TEST(FormatterTest, TransformingRvalueRef) {
+    auto formatter = cpptrace::formatter{}
+        .transform([](cpptrace::stacktrace_frame&& frame) {
             static size_t count = 0;
             frame.symbol = cpptrace::microfmt::format("sym{}", count++);
             return std::move(frame);
