diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index 506dc5c9..e0b5ccaa 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -29,12 +29,12 @@ namespace Catch { struct GeneratorTracker : TestCaseTracking::TrackerBase, IGeneratorTracker { GeneratorBasePtr m_generator; - GeneratorTracker( TestCaseTracking::NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ) - : TrackerBase( nameAndLocation, ctx, parent ) + GeneratorTracker( TestCaseTracking::NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ) + : TrackerBase( CATCH_MOVE(nameAndLocation), ctx, parent ) {} ~GeneratorTracker() override; - static GeneratorTracker* acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocation const& nameAndLocation ) { + static GeneratorTracker* acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocationRef nameAndLocation ) { GeneratorTracker* tracker; ITracker& currentTracker = ctx.currentTracker(); @@ -226,7 +226,7 @@ namespace Catch { uint64_t testRuns = 0; do { m_trackerContext.startCycle(); - m_testCaseTracker = &SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocation(testInfo.name, testInfo.lineInfo)); + m_testCaseTracker = &SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocationRef(testInfo.name, testInfo.lineInfo)); m_reporter->testCasePartialStarting(testInfo, testRuns); @@ -298,7 +298,8 @@ namespace Catch { } bool RunContext::sectionStarted(SectionInfo const & sectionInfo, Counts & assertions) { - ITracker& sectionTracker = SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocation(sectionInfo.name, sectionInfo.lineInfo)); + ITracker& sectionTracker = SectionTracker::acquire(m_trackerContext, TestCaseTracking::NameAndLocationRef(sectionInfo.name, sectionInfo.lineInfo)); + if (!sectionTracker.isOpen()) return false; m_activeSections.push_back(§ionTracker); @@ -317,8 +318,8 @@ namespace Catch { using namespace Generators; GeneratorTracker* tracker = GeneratorTracker::acquire( m_trackerContext, - TestCaseTracking::NameAndLocation( - static_cast( generatorName ), lineInfo ) ); + TestCaseTracking::NameAndLocationRef( + generatorName, lineInfo ) ); m_lastAssertionInfo.lineInfo = lineInfo; return tracker; } @@ -335,7 +336,7 @@ namespace Catch { "Trying to create tracker for a genreator that already has one" ); auto newTracker = Catch::Detail::make_unique( - nameAndLoc, m_trackerContext, ¤tTracker ); + CATCH_MOVE(nameAndLoc), m_trackerContext, ¤tTracker ); auto ret = newTracker.get(); currentTracker.addChild( CATCH_MOVE( newTracker ) ); diff --git a/src/catch2/internal/catch_test_case_tracker.cpp b/src/catch2/internal/catch_test_case_tracker.cpp index e230fac1..cd0990b5 100644 --- a/src/catch2/internal/catch_test_case_tracker.cpp +++ b/src/catch2/internal/catch_test_case_tracker.cpp @@ -22,8 +22,8 @@ namespace Catch { namespace TestCaseTracking { - NameAndLocation::NameAndLocation( std::string const& _name, SourceLineInfo const& _location ) - : name( _name ), + NameAndLocation::NameAndLocation( std::string&& _name, SourceLineInfo const& _location ) + : name( CATCH_MOVE(_name) ), location( _location ) {} @@ -38,14 +38,12 @@ namespace TestCaseTracking { m_children.push_back( CATCH_MOVE(child) ); } - ITracker* ITracker::findChild( NameAndLocation const& nameAndLocation ) { + ITracker* ITracker::findChild( NameAndLocationRef nameAndLocation ) { auto it = std::find_if( m_children.begin(), m_children.end(), [&nameAndLocation]( ITrackerPtr const& tracker ) { - return tracker->nameAndLocation().location == - nameAndLocation.location && - tracker->nameAndLocation().name == nameAndLocation.name; + return tracker->nameAndLocation() == nameAndLocation; } ); return ( it != m_children.end() ) ? it->get() : nullptr; } @@ -108,8 +106,8 @@ namespace TestCaseTracking { } - TrackerBase::TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ): - ITracker(nameAndLocation, parent), + TrackerBase::TrackerBase( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ): + ITracker(CATCH_MOVE(nameAndLocation), parent), m_ctx( ctx ) {} @@ -169,13 +167,14 @@ namespace TestCaseTracking { m_ctx.setCurrentTracker( this ); } - SectionTracker::SectionTracker( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ) - : TrackerBase( nameAndLocation, ctx, parent ), - m_trimmed_name(trim(nameAndLocation.name)) + SectionTracker::SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ) + : TrackerBase( CATCH_MOVE(nameAndLocation), ctx, parent ), + m_trimmed_name(trim(ITracker::nameAndLocation().name)) { if( parent ) { - while( !parent->isSectionTracker() ) + while ( !parent->isSectionTracker() ) { parent = parent->parent(); + } SectionTracker& parentSection = static_cast( *parent ); addNextFilters( parentSection.m_filters ); @@ -195,24 +194,30 @@ namespace TestCaseTracking { bool SectionTracker::isSectionTracker() const { return true; } - SectionTracker& SectionTracker::acquire( TrackerContext& ctx, NameAndLocation const& nameAndLocation ) { - SectionTracker* section; + SectionTracker& SectionTracker::acquire( TrackerContext& ctx, NameAndLocationRef nameAndLocation ) { + SectionTracker* tracker; ITracker& currentTracker = ctx.currentTracker(); if ( ITracker* childTracker = currentTracker.findChild( nameAndLocation ) ) { assert( childTracker ); assert( childTracker->isSectionTracker() ); - section = static_cast( childTracker ); + tracker = static_cast( childTracker ); } else { - auto newSection = Catch::Detail::make_unique( - nameAndLocation, ctx, ¤tTracker ); - section = newSection.get(); - currentTracker.addChild( CATCH_MOVE( newSection ) ); + auto newTracker = Catch::Detail::make_unique( + NameAndLocation{ static_cast(nameAndLocation.name), + nameAndLocation.location }, + ctx, + ¤tTracker ); + tracker = newTracker.get(); + currentTracker.addChild( CATCH_MOVE( newTracker ) ); } - if( !ctx.completedCycle() ) - section->tryOpen(); - return *section; + + if ( !ctx.completedCycle() ) { + tracker->tryOpen(); + } + + return *tracker; } void SectionTracker::tryOpen() { diff --git a/src/catch2/internal/catch_test_case_tracker.hpp b/src/catch2/internal/catch_test_case_tracker.hpp index 1e31e425..0021d254 100644 --- a/src/catch2/internal/catch_test_case_tracker.hpp +++ b/src/catch2/internal/catch_test_case_tracker.hpp @@ -22,7 +22,7 @@ namespace TestCaseTracking { std::string name; SourceLineInfo location; - NameAndLocation( std::string const& _name, SourceLineInfo const& _location ); + NameAndLocation( std::string&& _name, SourceLineInfo const& _location ); friend bool operator==(NameAndLocation const& lhs, NameAndLocation const& rhs) { return lhs.name == rhs.name && lhs.location == rhs.location; @@ -33,6 +33,32 @@ namespace TestCaseTracking { } }; + /** + * This is a variant of `NameAndLocation` that does not own the name string + * + * This avoids extra allocations when trying to locate a tracker by its + * name and location, as long as we make sure that trackers only keep + * around the owning variant. + */ + struct NameAndLocationRef { + StringRef name; + SourceLineInfo location; + + constexpr NameAndLocationRef( StringRef name_, + SourceLineInfo location_ ): + name( name_ ), location( location_ ) {} + + friend bool operator==( NameAndLocation const& lhs, + NameAndLocationRef rhs ) { + return StringRef( lhs.name ) == rhs.name && + lhs.location == rhs.location; + } + friend bool operator==( NameAndLocationRef lhs, + NameAndLocation const& rhs ) { + return rhs == lhs; + } + }; + class ITracker; using ITrackerPtr = Catch::Detail::unique_ptr; @@ -57,8 +83,8 @@ namespace TestCaseTracking { CycleState m_runState = NotStarted; public: - ITracker( NameAndLocation const& nameAndLoc, ITracker* parent ): - m_nameAndLocation( nameAndLoc ), + ITracker( NameAndLocation&& nameAndLoc, ITracker* parent ): + m_nameAndLocation( CATCH_MOVE(nameAndLoc) ), m_parent( parent ) {} @@ -97,7 +123,7 @@ namespace TestCaseTracking { * * Returns nullptr if not found. */ - ITracker* findChild( NameAndLocation const& nameAndLocation ); + ITracker* findChild( NameAndLocationRef nameAndLocation ); //! Have any children been added? bool hasChildren() const { return !m_children.empty(); @@ -154,7 +180,7 @@ namespace TestCaseTracking { TrackerContext& m_ctx; public: - TrackerBase( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ); + TrackerBase( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ); bool isComplete() const override; @@ -172,13 +198,13 @@ namespace TestCaseTracking { std::vector m_filters; std::string m_trimmed_name; public: - SectionTracker( NameAndLocation const& nameAndLocation, TrackerContext& ctx, ITracker* parent ); + SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ); bool isSectionTracker() const override; bool isComplete() const override; - static SectionTracker& acquire( TrackerContext& ctx, NameAndLocation const& nameAndLocation ); + static SectionTracker& acquire( TrackerContext& ctx, NameAndLocationRef nameAndLocation ); void tryOpen(); diff --git a/tests/SelfTest/IntrospectiveTests/PartTracker.tests.cpp b/tests/SelfTest/IntrospectiveTests/PartTracker.tests.cpp index ac8be645..c13ec573 100644 --- a/tests/SelfTest/IntrospectiveTests/PartTracker.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/PartTracker.tests.cpp @@ -14,8 +14,8 @@ using namespace Catch; namespace { -Catch::TestCaseTracking::NameAndLocation makeNAL( std::string const& name ) { - return Catch::TestCaseTracking::NameAndLocation( name, Catch::SourceLineInfo("",0) ); +Catch::TestCaseTracking::NameAndLocationRef makeNAL( StringRef name ) { + return Catch::TestCaseTracking::NameAndLocationRef( name, Catch::SourceLineInfo("",0) ); } }