Replace shared_ptrs with unique_ptrs in CumulativeReporterBase nodes

Closes #2089

src/catch2/reporters/catch_reporter_cumulative_base.cpp

@@ -17,7 +17,7 @@ namespace Catch {
             BySectionInfo( BySectionInfo const& other ):
                 m_other( other.m_other ) {}
             bool operator()(
-                std::shared_ptr<CumulativeReporterBase::SectionNode> const&
+                Detail::unique_ptr<CumulativeReporterBase::SectionNode> const&
                     node ) const {
                 return (
                     ( node->stats.sectionInfo.name == m_other.name ) &&
@@ -29,9 +29,6 @@ namespace Catch {
             SectionInfo const& m_other;
         };
 
-        void prepareExpandedExpression( AssertionResult& result ) {
-            result.getExpandedExpression();
-        }
     } // namespace
 
 
@@ -40,27 +37,30 @@ namespace Catch {
     void
     CumulativeReporterBase::sectionStarting( SectionInfo const& sectionInfo ) {
         SectionStats incompleteStats( sectionInfo, Counts(), 0, false );
-        std::shared_ptr<SectionNode> node;
+        SectionNode* node;
         if ( m_sectionStack.empty() ) {
-            if ( !m_rootSection )
+            if ( !m_rootSection ) {
                 m_rootSection =
-                    std::make_shared<SectionNode>( incompleteStats );
-            node = m_rootSection;
+                    Detail::make_unique<SectionNode>( incompleteStats );
+            }
+            node = m_rootSection.get();
         } else {
             SectionNode& parentNode = *m_sectionStack.back();
             auto it = std::find_if( parentNode.childSections.begin(),
                                     parentNode.childSections.end(),
                                     BySectionInfo( sectionInfo ) );
             if ( it == parentNode.childSections.end() ) {
-                node = std::make_shared<SectionNode>( incompleteStats );
-                parentNode.childSections.push_back( node );
+                auto newNode =
+                    Detail::make_unique<SectionNode>( incompleteStats );
+                node = newNode.get();
+                parentNode.childSections.push_back( std::move( newNode ) );
             } else {
-                node = *it;
+                node = it->get();
             }
         }
 
-        m_deepestSection = node.get();
-        m_sectionStack.push_back( std::move(node) );
+        m_deepestSection = node;
+        m_sectionStack.push_back( node );
     }
 
     bool CumulativeReporterBase::assertionEnded(
@@ -71,8 +71,8 @@ namespace Catch {
         // Our section stack copy of the assertionResult will likely outlive the
         // temporary, so it must be expanded or discarded now to avoid calling
         // a destroyed object later.
-        prepareExpandedExpression(
-            const_cast<AssertionResult&>( assertionStats.assertionResult ) );
+        static_cast<void>(
+            assertionStats.assertionResult.getExpandedExpression() );
         SectionNode& sectionNode = *m_sectionStack.back();
         sectionNode.assertions.push_back( assertionStats );
         return true;
@@ -87,11 +87,10 @@ namespace Catch {
 
     void CumulativeReporterBase::testCaseEnded(
         TestCaseStats const& testCaseStats ) {
-        auto node = std::make_shared<TestCaseNode>( testCaseStats );
+        auto node = Detail::make_unique<TestCaseNode>( testCaseStats );
         assert( m_sectionStack.size() == 0 );
-        node->children.push_back( m_rootSection );
-        m_testCases.push_back( node );
-        m_rootSection.reset();
+        node->children.push_back( std::move(m_rootSection) );
+        m_testCases.push_back( std::move(node) );
 
         assert( m_deepestSection );
         m_deepestSection->stdOut = testCaseStats.stdOut;
@@ -100,15 +99,14 @@ namespace Catch {
 
     void CumulativeReporterBase::testGroupEnded(
         TestGroupStats const& testGroupStats ) {
-        auto node = std::make_shared<TestGroupNode>( testGroupStats );
+        auto node = Detail::make_unique<TestGroupNode>( testGroupStats );
         node->children.swap( m_testCases );
-        m_testGroups.push_back( node );
+        m_testGroups.push_back( std::move(node) );
     }
 
     void CumulativeReporterBase::testRunEnded( TestRunStats const& testRunStats ) {
-        auto node = Detail::make_unique<TestRunNode>( testRunStats );
-        node->children.swap( m_testGroups );
-        m_testRuns.push_back( std::move(node) );
+        m_testRuns.emplace_back( testRunStats );
+        m_testRuns.back().children.swap( m_testGroups );
         testRunEndedCumulative();
     }
 

src/catch2/reporters/catch_reporter_cumulative_base.hpp

@@ -22,9 +22,8 @@ namespace Catch {
         template<typename T, typename ChildNodeT>
         struct Node {
             explicit Node( T const& _value ) : value( _value ) {}
-            virtual ~Node() = default;
 
-            using ChildNodes = std::vector<std::shared_ptr<ChildNodeT>>;
+            using ChildNodes = std::vector<Detail::unique_ptr<ChildNodeT>>;
             T value;
             ChildNodes children;
         };
@@ -36,10 +35,8 @@ namespace Catch {
             }
 
             SectionStats stats;
-            using ChildSections = std::vector<std::shared_ptr<SectionNode>>;
-            using Assertions = std::vector<AssertionStats>;
-            ChildSections childSections;
-            Assertions assertions;
+            std::vector<Detail::unique_ptr<SectionNode>> childSections;
+            std::vector<AssertionStats> assertions;
             std::string stdOut;
             std::string stdErr;
         };
@@ -67,20 +64,23 @@ namespace Catch {
         void testCaseEnded( TestCaseStats const& testCaseStats ) override;
         void testGroupEnded( TestGroupStats const& testGroupStats ) override;
         void testRunEnded( TestRunStats const& testRunStats ) override;
+        //! Customization point: called after last test finishes (testRunEnded has been handled)
         virtual void testRunEndedCumulative() = 0;
 
         void skipTest(TestCaseInfo const&) override {}
 
         IConfig const* m_config;
         std::ostream& stream;
-        std::vector<std::shared_ptr<TestCaseNode>> m_testCases;
-        std::vector<std::shared_ptr<TestGroupNode>> m_testGroups;
+        // Note: We rely on pointer identity being stable, which is why
+        //       which is why we store around pointers rather than values.
+        std::vector<Detail::unique_ptr<TestCaseNode>> m_testCases;
+        std::vector<Detail::unique_ptr<TestGroupNode>> m_testGroups;
 
-        std::vector<Detail::unique_ptr<TestRunNode>> m_testRuns;
+        std::vector<TestRunNode> m_testRuns;
 
-        std::shared_ptr<SectionNode> m_rootSection;
+        Detail::unique_ptr<SectionNode> m_rootSection;
         SectionNode* m_deepestSection = nullptr;
-        std::vector<std::shared_ptr<SectionNode>> m_sectionStack;
+        std::vector<SectionNode*> m_sectionStack;
     };
 
 } // end namespace Catch

src/catch2/reporters/catch_reporter_sonarqube.cpp

@@ -28,15 +28,17 @@ namespace Catch {
     }
 
     void SonarQubeReporter::writeGroup(TestGroupNode const& groupNode) {
-        std::map<std::string, TestGroupNode::ChildNodes> testsPerFile;
-        for (auto const& child : groupNode.children)
-            testsPerFile[child->value.testInfo->lineInfo.file].push_back(child);
+        std::map<std::string, std::vector<TestCaseNode const*>> testsPerFile;
+        for ( auto const& child : groupNode.children ) {
+            testsPerFile[child->value.testInfo->lineInfo.file].push_back(
+                child.get() );
+        }
 
         for (auto const& kv : testsPerFile)
             writeTestFile(kv.first, kv.second);
     }
 
-    void SonarQubeReporter::writeTestFile(std::string const& filename, TestGroupNode::ChildNodes const& testCaseNodes) {
+    void SonarQubeReporter::writeTestFile(std::string const& filename, std::vector<TestCaseNode const*> const& testCaseNodes) {
         XmlWriter::ScopedElement e = xml.scopedElement("file");
         xml.writeAttribute("path", filename);
 

src/catch2/reporters/catch_reporter_sonarqube.hpp

@@ -42,7 +42,7 @@ namespace Catch {
 
         void writeGroup(TestGroupNode const& groupNode);
 
-        void writeTestFile(std::string const& filename, TestGroupNode::ChildNodes const& testCaseNodes);
+        void writeTestFile(std::string const& filename, std::vector<TestCaseNode const*> const& testCaseNodes);
 
         void writeTestCase(TestCaseNode const& testCaseNode);