Skip to content

Commit

Permalink
Avoid copying TokenStreams while parsing
Browse files Browse the repository at this point in the history 
The code is now even worse mess than before, due to the ad-hoc
implementation of Result-ish type based on virtual functions in
Clara, but it has dropped the allocations for empty binary down
to 151.
  • Loading branch information
@horenmar
horenmar committed Dec 27, 2023
1 parent 5d637d4 commit eaafd07
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 40 deletions.
53 changes: 26 additions & 27 deletions src/catch2/internal/catch_clara.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ namespace Catch {
}

ParseState::ParseState( ParseResultType type,
TokenStream const& remainingTokens ):
m_type( type ), m_remainingTokens( remainingTokens ) {}
TokenStream remainingTokens ):
m_type( type ), m_remainingTokens( CATCH_MOVE(remainingTokens) ) {}

ParserResult BoundFlagRef::setFlag( bool flag ) {
m_ref = flag;
Expand All @@ -164,27 +164,27 @@ namespace Catch {
} // namespace Detail

Detail::InternalParseResult Arg::parse(std::string const&,
Detail::TokenStream const& tokens) const {
Detail::TokenStream tokens) const {
auto validationResult = validate();
if (!validationResult)
return Detail::InternalParseResult(validationResult);

auto remainingTokens = tokens;
auto const& token = *remainingTokens;
auto token = *tokens;
if (token.type != Detail::TokenType::Argument)
return Detail::InternalParseResult::ok(Detail::ParseState(
ParseResultType::NoMatch, remainingTokens));
ParseResultType::NoMatch, CATCH_MOVE(tokens)));

assert(!m_ref->isFlag());
auto valueRef =
static_cast<Detail::BoundValueRefBase*>(m_ref.get());

auto result = valueRef->setValue(static_cast<std::string>(token.token));
if (!result)
return Detail::InternalParseResult(result);
if ( !result )
return Detail::InternalParseResult( result );
else
return Detail::InternalParseResult::ok(Detail::ParseState(
ParseResultType::Matched, ++remainingTokens));
return Detail::InternalParseResult::ok(
Detail::ParseState( ParseResultType::Matched,
CATCH_MOVE( ++tokens ) ) );
}

Opt::Opt(bool& ref) :
Expand Down Expand Up @@ -215,15 +215,14 @@ namespace Catch {
}

Detail::InternalParseResult Opt::parse(std::string const&,
Detail::TokenStream const& tokens) const {
Detail::TokenStream tokens) const {
auto validationResult = validate();
if (!validationResult)
return Detail::InternalParseResult(validationResult);

auto remainingTokens = tokens;
if (remainingTokens &&
remainingTokens->type == Detail::TokenType::Option) {
auto const& token = *remainingTokens;
if (tokens &&
tokens->type == Detail::TokenType::Option) {
auto const& token = *tokens;
if (isMatch(token.token)) {
if (m_ref->isFlag()) {
auto flagRef =
Expand All @@ -235,17 +234,17 @@ namespace Catch {
if (result.value() ==
ParseResultType::ShortCircuitAll)
return Detail::InternalParseResult::ok(Detail::ParseState(
result.value(), remainingTokens));
result.value(), CATCH_MOVE(tokens)));
} else {
auto valueRef =
static_cast<Detail::BoundValueRefBase*>(
m_ref.get());
++remainingTokens;
if (!remainingTokens)
++tokens;
if (!tokens)
return Detail::InternalParseResult::runtimeError(
"Expected argument following " +
token.token);
auto const& argToken = *remainingTokens;
auto const& argToken = *tokens;
if (argToken.type != Detail::TokenType::Argument)
return Detail::InternalParseResult::runtimeError(
"Expected argument following " +
Expand All @@ -256,14 +255,14 @@ namespace Catch {
if (result.value() ==
ParseResultType::ShortCircuitAll)
return Detail::InternalParseResult::ok(Detail::ParseState(
result.value(), remainingTokens));
result.value(), CATCH_MOVE(tokens)));
}
return Detail::InternalParseResult::ok(Detail::ParseState(
ParseResultType::Matched, ++remainingTokens));
ParseResultType::Matched, CATCH_MOVE(++tokens)));
}
}
return Detail::InternalParseResult::ok(
Detail::ParseState(ParseResultType::NoMatch, remainingTokens));
Detail::ParseState(ParseResultType::NoMatch, CATCH_MOVE(tokens)));
}

Detail::Result Opt::validate() const {
Expand Down Expand Up @@ -295,9 +294,9 @@ namespace Catch {

Detail::InternalParseResult
ExeName::parse(std::string const&,
Detail::TokenStream const& tokens) const {
Detail::TokenStream tokens) const {
return Detail::InternalParseResult::ok(
Detail::ParseState(ParseResultType::NoMatch, tokens));
Detail::ParseState(ParseResultType::NoMatch, CATCH_MOVE(tokens)));
}

ParserResult ExeName::set(std::string const& newName) {
Expand Down Expand Up @@ -394,7 +393,7 @@ namespace Catch {

Detail::InternalParseResult
Parser::parse( std::string const& exeName,
Detail::TokenStream const& tokens ) const {
Detail::TokenStream tokens ) const {

struct ParserInfo {
ParserBase const* parser = nullptr;
Expand All @@ -412,15 +411,15 @@ namespace Catch {
m_exeName.set( exeName );

auto result = Detail::InternalParseResult::ok(
Detail::ParseState( ParseResultType::NoMatch, tokens ) );
Detail::ParseState( ParseResultType::NoMatch, CATCH_MOVE(tokens) ) );
while ( result.value().remainingTokens() ) {
bool tokenParsed = false;

for ( auto& parseInfo : parseInfos ) {
if ( parseInfo.parser->cardinality() == 0 ||
parseInfo.count < parseInfo.parser->cardinality() ) {
result = parseInfo.parser->parse(
exeName, result.value().remainingTokens() );
exeName, CATCH_MOVE(result).value().remainingTokens() );
if ( !result )
return result;
if ( result.value().type() !=
Expand Down
51 changes: 38 additions & 13 deletions src/catch2/internal/catch_clara.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,14 @@ namespace Catch {
template <typename T>
class ResultValueBase : public ResultBase {
public:
auto value() const -> T const& {
T const& value() const& {
enforceOk();
return m_value;
}
T&& value() && {
enforceOk();
return CATCH_MOVE( m_value );
}

protected:
ResultValueBase( ResultType type ): ResultBase( type ) {}
Expand All @@ -179,20 +183,38 @@ namespace Catch {
if ( m_type == ResultType::Ok )
new ( &m_value ) T( other.m_value );
}
ResultValueBase( ResultValueBase&& other ):
ResultBase( other ) {
if ( m_type == ResultType::Ok )
new ( &m_value ) T( CATCH_MOVE(other.m_value) );
}


ResultValueBase( ResultType, T const& value ): ResultBase( ResultType::Ok ) {
ResultValueBase( ResultType, T const& value ):
ResultBase( ResultType::Ok ) {
new ( &m_value ) T( value );
}
ResultValueBase( ResultType, T&& value ):
ResultBase( ResultType::Ok ) {
new ( &m_value ) T( CATCH_MOVE(value) );
}

auto operator=( ResultValueBase const& other )
-> ResultValueBase& {
ResultValueBase& operator=( ResultValueBase const& other ) {
if ( m_type == ResultType::Ok )
m_value.~T();
ResultBase::operator=( other );
if ( m_type == ResultType::Ok )
new ( &m_value ) T( other.m_value );
return *this;
}
ResultValueBase& operator=( ResultValueBase&& other ) {
if ( m_type == ResultType::Ok ) m_value.~T();
ResultBase::operator=( other );
if ( m_type == ResultType::Ok )
new ( &m_value ) T( CATCH_MOVE(other.m_value) );
return *this;
}


~ResultValueBase() override {
if ( m_type == ResultType::Ok )
Expand Down Expand Up @@ -220,8 +242,8 @@ namespace Catch {
}

template <typename U>
static auto ok( U const& value ) -> BasicResult {
return { ResultType::Ok, value };
static auto ok( U&& value ) -> BasicResult {
return { ResultType::Ok, CATCH_FORWARD(value) };
}
static auto ok() -> BasicResult { return { ResultType::Ok }; }
static auto logicError( std::string&& message )
Expand Down Expand Up @@ -268,12 +290,15 @@ namespace Catch {
class ParseState {
public:
ParseState( ParseResultType type,
TokenStream const& remainingTokens );
TokenStream remainingTokens );

ParseResultType type() const { return m_type; }
TokenStream const& remainingTokens() const {
TokenStream const& remainingTokens() const& {
return m_remainingTokens;
}
TokenStream&& remainingTokens() && {
return CATCH_MOVE( m_remainingTokens );
}

private:
ParseResultType m_type;
Expand Down Expand Up @@ -446,7 +471,7 @@ namespace Catch {
virtual ~ParserBase() = default;
virtual auto validate() const -> Result { return Result::ok(); }
virtual auto parse( std::string const& exeName,
TokenStream const& tokens ) const
TokenStream tokens ) const
-> InternalParseResult = 0;
virtual size_t cardinality() const;

Expand Down Expand Up @@ -538,7 +563,7 @@ namespace Catch {

Detail::InternalParseResult
parse(std::string const&,
Detail::TokenStream const& tokens) const override;
Detail::TokenStream tokens) const override;
};

// A parser for options
Expand Down Expand Up @@ -587,7 +612,7 @@ namespace Catch {

Detail::InternalParseResult
parse(std::string const&,
Detail::TokenStream const& tokens) const override;
Detail::TokenStream tokens) const override;

Detail::Result validate() const override;
};
Expand All @@ -610,7 +635,7 @@ namespace Catch {
// handled specially
Detail::InternalParseResult
parse(std::string const&,
Detail::TokenStream const& tokens) const override;
Detail::TokenStream tokens) const override;

std::string const& name() const { return *m_name; }
Detail::ParserResult set(std::string const& newName);
Expand Down Expand Up @@ -672,7 +697,7 @@ namespace Catch {
using ParserBase::parse;
Detail::InternalParseResult
parse(std::string const& exeName,
Detail::TokenStream const& tokens) const override;
Detail::TokenStream tokens) const override;
};

/**
Expand Down

0 comments on commit eaafd07

Please sign in to comment.