Devirtualize more things in trackers

We were already using it mostly to get pointers rather than
references, so this makes it make more sense.

src/catch2/internal/catch_run_context.cpp

@@ -50,7 +50,7 @@ namespace Catch {
                 // without it, the code above creates 5 nested generators.
                 if ( currentTracker.nameAndLocation() == nameAndLocation ) {
                     auto thisTracker =
-                        currentTracker.parent().findChild( nameAndLocation );
+                        currentTracker.parent()->findChild( nameAndLocation );
                     assert( thisTracker );
                     assert( thisTracker->isGeneratorTracker() );
                     tracker = static_cast<GeneratorTracker*>( thisTracker );
@@ -110,7 +110,7 @@ namespace Catch {
                     // This is safe: there is always at least one section
                     // tracker in a test case tracking tree
                     while ( !parent->isSectionTracker() ) {
-                        parent = &( parent->parent() );
+                        parent = parent->parent();
                     }
                     assert( parent &&
                             "Missing root (test case) level section" );
@@ -156,7 +156,7 @@ namespace Catch {
                 m_generator = std::move( generator );
             }
         };
-        GeneratorTracker::~GeneratorTracker() {}
+        GeneratorTracker::~GeneratorTracker() = default;
     }
 
     RunContext::RunContext(IConfig const* _config, IStreamingReporterPtr&& reporter)

src/catch2/internal/catch_test_case_tracker.cpp

@@ -29,6 +29,10 @@ namespace TestCaseTracking {
 
     ITracker::~ITracker() = default;
 
+    void ITracker::markAsNeedingAnotherRun() {
+        m_runState = NeedsAnotherRun;
+    }
+
     void ITracker::addChild( ITrackerPtr&& child ) {
         m_children.push_back( std::move(child) );
     }
@@ -45,7 +49,27 @@ namespace TestCaseTracking {
         return ( it != m_children.end() ) ? it->get() : nullptr;
     }
 
+    bool ITracker::isSectionTracker() const { return false; }
+    bool ITracker::isGeneratorTracker() const { return false; }
 
+    bool ITracker::isSuccessfullyCompleted() const {
+        return m_runState == CompletedSuccessfully;
+    }
+
+    bool ITracker::isOpen() const {
+        return m_runState != NotStarted && !isComplete();
+    }
+
+    bool ITracker::hasStarted() const { return m_runState != NotStarted; }
+
+    void ITracker::openChild() {
+        if (m_runState != ExecutingChildren) {
+            m_runState = ExecutingChildren;
+            if (m_parent) {
+                m_parent->openChild();
+            }
+        }
+    }
 
     ITracker& TrackerContext::startRun() {
         using namespace std::string_literals;
@@ -84,36 +108,13 @@ namespace TestCaseTracking {
 
 
     TrackerBase::TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ):
-        ITracker(nameAndLocation),
-        m_ctx( ctx ),
-        m_parent( parent )
+        ITracker(nameAndLocation, parent),
+        m_ctx( ctx )
     {}
 
     bool TrackerBase::isComplete() const {
         return m_runState == CompletedSuccessfully || m_runState == Failed;
     }
-    bool TrackerBase::isSuccessfullyCompleted() const {
-        return m_runState == CompletedSuccessfully;
-    }
-    bool TrackerBase::isOpen() const {
-        return m_runState != NotStarted && !isComplete();
-    }
-
-    ITracker& TrackerBase::parent() {
-        assert( m_parent ); // Should always be non-null except for root
-        return *m_parent;
-    }
-
-    void TrackerBase::openChild() {
-        if( m_runState != ExecutingChildren ) {
-            m_runState = ExecutingChildren;
-            if( m_parent )
-                m_parent->openChild();
-        }
-    }
-
-    bool TrackerBase::isSectionTracker() const { return false; }
-    bool TrackerBase::isGeneratorTracker() const { return false; }
 
     void TrackerBase::open() {
         m_runState = Executing;
@@ -158,9 +159,6 @@ namespace TestCaseTracking {
         moveToParent();
         m_ctx.completeCycle();
     }
-    void TrackerBase::markAsNeedingAnotherRun() {
-        m_runState = NeedsAnotherRun;
-    }
 
     void TrackerBase::moveToParent() {
         assert( m_parent );
@@ -176,7 +174,7 @@ namespace TestCaseTracking {
     {
         if( parent ) {
             while( !parent->isSectionTracker() )
-                parent = &parent->parent();
+                parent = parent->parent();
 
             SectionTracker& parentSection = static_cast<SectionTracker&>( *parent );
             addNextFilters( parentSection.m_filters );
@@ -244,10 +242,6 @@ namespace TestCaseTracking {
 
 } // namespace TestCaseTracking
 
-using TestCaseTracking::ITracker;
-using TestCaseTracking::TrackerContext;
-using TestCaseTracking::SectionTracker;
-
 } // namespace Catch
 
 #if defined(__clang__)

src/catch2/internal/catch_test_case_tracker.hpp

@@ -32,17 +32,29 @@ namespace TestCaseTracking {
 
     using ITrackerPtr = Catch::Detail::unique_ptr<ITracker>;
 
-    class  ITracker {
+    class ITracker {
         NameAndLocation m_nameAndLocation;
 
         using Children = std::vector<ITrackerPtr>;
 
     protected:
+        enum CycleState {
+            NotStarted,
+            Executing,
+            ExecutingChildren,
+            NeedsAnotherRun,
+            CompletedSuccessfully,
+            Failed
+        };
+
+        ITracker* m_parent = nullptr;
         Children m_children;
+        CycleState m_runState = NotStarted;
 
     public:
-        ITracker(NameAndLocation const& nameAndLoc) :
-            m_nameAndLocation(nameAndLoc)
+        ITracker( NameAndLocation const& nameAndLoc, ITracker* parent ):
+            m_nameAndLocation( nameAndLoc ),
+            m_parent( parent )
         {}
 
 
@@ -50,22 +62,28 @@ namespace TestCaseTracking {
         NameAndLocation const& nameAndLocation() const {
             return m_nameAndLocation;
         }
+        ITracker* parent() const {
+            return m_parent;
+        }
 
         virtual ~ITracker(); // = default
 
 
         // dynamic queries
-        virtual bool isComplete() const = 0; // Successfully completed or failed
-        virtual bool isSuccessfullyCompleted() const = 0;
-        virtual bool isOpen() const = 0; // Started but not complete
-        virtual bool hasStarted() const = 0;
 
-        virtual ITracker& parent() = 0;
+        //! Returns true if tracker run to completion (successfully or not)
+        virtual bool isComplete() const = 0;
+        //! Returns true if tracker run to completion succesfully
+        bool isSuccessfullyCompleted() const;
+        //! Returns true if tracker has started but hasn't been completed
+        bool isOpen() const;
+        //! Returns true iff tracker has started
+        bool hasStarted() const;
 
         // actions
         virtual void close() = 0; // Successfully complete
         virtual void fail() = 0;
-        virtual void markAsNeedingAnotherRun() = 0;
+        void markAsNeedingAnotherRun();
 
         //! Register a nested ITracker
         void addChild( ITrackerPtr&& child );
@@ -81,11 +99,23 @@ namespace TestCaseTracking {
         }
 
 
-        virtual void openChild() = 0;
+        //! Marks tracker as executing a child, doing se recursively up the tree
+        void openChild();
 
-        // Debug/ checking
-        virtual bool isSectionTracker() const = 0;
-        virtual bool isGeneratorTracker() const = 0;
+        /**
+         * Returns true if the instance is a section tracker
+         *
+         * Subclasses should override to true if they are, replaces RTTI
+         * for internal debug checks.
+         */
+        virtual bool isSectionTracker() const;
+        /**
+         * Returns true if the instance is a generator tracker
+         *
+         * Subclasses should override to true if they are, replaces RTTI
+         * for internal debug checks.
+         */
+        virtual bool isGeneratorTracker() const;
     };
 
     class TrackerContext {
@@ -115,41 +145,18 @@ namespace TestCaseTracking {
 
     class TrackerBase : public ITracker {
     protected:
-        enum CycleState {
-            NotStarted,
-            Executing,
-            ExecutingChildren,
-            NeedsAnotherRun,
-            CompletedSuccessfully,
-            Failed
-        };
 
         TrackerContext& m_ctx;
-        ITracker* m_parent;
-        CycleState m_runState = NotStarted;
 
     public:
         TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent );
 
         bool isComplete() const override;
-        bool isSuccessfullyCompleted() const override;
-        bool isOpen() const override;
-        bool hasStarted() const override {
-            return m_runState != NotStarted;
-        }
-
-        ITracker& parent() override;
-
-        void openChild() override;
-
-        bool isSectionTracker() const override;
-        bool isGeneratorTracker() const override;
 
         void open();
 
         void close() override;
         void fail() override;
-        void markAsNeedingAnotherRun() override;
 
     private:
         void moveToParent();

src/catch2/reporters/catch_reporter_junit.hpp

@@ -38,13 +38,14 @@ namespace Catch {
 
         void testRunEndedCumulative() override;
 
+    private:
         void writeGroup(TestGroupNode const& groupNode, double suiteTime);
 
         void writeTestCase(TestCaseNode const& testCaseNode);
 
-        void writeSection(std::string const& className,
-                          std::string const& rootName,
-                          SectionNode const& sectionNode);
+        void writeSection( std::string const& className,
+                           std::string const& rootName,
+                           SectionNode const& sectionNode );
 
         void writeAssertions(SectionNode const& sectionNode);
         void writeAssertion(AssertionStats const& stats);