Make StringRef's operator std::string explicit

This way it is explicit when there is a `StringRef` -> `std::string`
conversion and makes it easier to look for allocations that could
be avoided.

Doing this has already removed one allocation per registered test
case, as there was a completely pointless `StringRef` -> `std::string`
conversion when parsing tags of a test case.
This commit is contained in:
Martin Hořeňovský 2019-09-08 14:49:40 +02:00
parent 14362533bb
commit 7b865daccc
No known key found for this signature in database
GPG Key ID: DE48307B8B0D381A
13 changed files with 23 additions and 67 deletions

View File

@ -56,13 +56,13 @@ namespace Catch {
if( isFalseTest( m_info.resultDisposition ) ) if( isFalseTest( m_info.resultDisposition ) )
return "!(" + m_info.capturedExpression + ")"; return "!(" + m_info.capturedExpression + ")";
else else
return m_info.capturedExpression; return static_cast<std::string>(m_info.capturedExpression);
} }
std::string AssertionResult::getExpressionInMacro() const { std::string AssertionResult::getExpressionInMacro() const {
std::string expr; std::string expr;
if( m_info.macroName[0] == 0 ) if( m_info.macroName[0] == 0 )
expr = m_info.capturedExpression; expr = static_cast<std::string>(m_info.capturedExpression);
else { else {
expr.reserve( m_info.macroName.size() + m_info.capturedExpression.size() + 4 ); expr.reserve( m_info.macroName.size() + m_info.capturedExpression.size() + 4 );
expr += m_info.macroName; expr += m_info.macroName;

View File

@ -113,7 +113,7 @@ namespace Catch {
case ',': case ',':
if (start != pos && openings.size() == 0) { if (start != pos && openings.size() == 0) {
m_messages.emplace_back(macroName, lineInfo, resultType); m_messages.emplace_back(macroName, lineInfo, resultType);
m_messages.back().message = trimmed(start, pos); m_messages.back().message = static_cast<std::string>(trimmed(start, pos));
m_messages.back().message += " := "; m_messages.back().message += " := ";
start = pos; start = pos;
} }
@ -121,7 +121,7 @@ namespace Catch {
} }
assert(openings.size() == 0 && "Mismatched openings"); assert(openings.size() == 0 && "Mismatched openings");
m_messages.emplace_back(macroName, lineInfo, resultType); m_messages.emplace_back(macroName, lineInfo, resultType);
m_messages.back().message = trimmed(start, names.size() - 1); m_messages.back().message = static_cast<std::string>(trimmed(start, names.size() - 1));
m_messages.back().message += " := "; m_messages.back().message += " := ";
} }
Capturer::~Capturer() { Capturer::~Capturer() {

View File

@ -230,7 +230,7 @@ namespace Catch {
m_unfinishedSections.push_back(endInfo); m_unfinishedSections.push_back(endInfo);
} }
#if defined(CATCH_CONFIG_ENABLE_BENCHMARKING) #if defined(CATCH_CONFIG_ENABLE_BENCHMARKING)
void RunContext::benchmarkPreparing(std::string const& name) { void RunContext::benchmarkPreparing(std::string const& name) {
m_reporter->benchmarkPreparing(name); m_reporter->benchmarkPreparing(name);
@ -279,7 +279,7 @@ namespace Catch {
// Don't rebuild the result -- the stringification itself can cause more fatal errors // Don't rebuild the result -- the stringification itself can cause more fatal errors
// Instead, fake a result data. // Instead, fake a result data.
AssertionResultData tempResult( ResultWas::FatalErrorCondition, { false } ); AssertionResultData tempResult( ResultWas::FatalErrorCondition, { false } );
tempResult.message = message; tempResult.message = static_cast<std::string>(message);
AssertionResult result(m_lastAssertionInfo, tempResult); AssertionResult result(m_lastAssertionInfo, tempResult);
assertionEnded(result); assertionEnded(result);
@ -442,7 +442,7 @@ namespace Catch {
m_lastAssertionInfo = info; m_lastAssertionInfo = info;
AssertionResultData data( resultType, LazyExpression( false ) ); AssertionResultData data( resultType, LazyExpression( false ) );
data.message = message; data.message = static_cast<std::string>(message);
AssertionResult assertionResult{ m_lastAssertionInfo, data }; AssertionResult assertionResult{ m_lastAssertionInfo, data };
assertionEnded( assertionResult ); assertionEnded( assertionResult );
if( !assertionResult.isOk() ) if( !assertionResult.isOk() )

View File

@ -23,6 +23,7 @@ namespace Catch {
class StringRef { class StringRef {
public: public:
using size_type = std::size_t; using size_type = std::size_t;
using const_iterator = const char*;
private: private:
friend struct StringRefTestAccess; friend struct StringRefTestAccess;
@ -78,7 +79,7 @@ namespace Catch {
return *this; return *this;
} }
operator std::string() const; explicit operator std::string() const;
void swap( StringRef& other ) noexcept; void swap( StringRef& other ) noexcept;
@ -105,6 +106,10 @@ namespace Catch {
// Note that the pointer can change when if the StringRef is a substring // Note that the pointer can change when if the StringRef is a substring
auto currentData() const noexcept -> char const*; auto currentData() const noexcept -> char const*;
public: // iterators
const_iterator begin() const { return m_start; }
const_iterator end() const { return m_start + m_size; }
private: // ownership queries - may not be consistent between calls private: // ownership queries - may not be consistent between calls
auto isOwned() const noexcept -> bool; auto isOwned() const noexcept -> bool;
auto isSubstring() const noexcept -> bool; auto isSubstring() const noexcept -> bool;

View File

@ -59,8 +59,7 @@ namespace Catch {
std::vector<std::string> tags; std::vector<std::string> tags;
std::string desc, tag; std::string desc, tag;
bool inTag = false; bool inTag = false;
std::string _descOrTags = nameAndTags.tags; for (char c : nameAndTags.tags) {
for (char c : _descOrTags) {
if( !inTag ) { if( !inTag ) {
if( c == '[' ) if( c == '[' )
inTag = true; inTag = true;
@ -93,7 +92,7 @@ namespace Catch {
tags.push_back( "." ); tags.push_back( "." );
} }
TestCaseInfo info( nameAndTags.name, _className, desc, tags, _lineInfo ); TestCaseInfo info( static_cast<std::string>(nameAndTags.name), _className, desc, tags, _lineInfo );
return TestCase( _testCase, std::move(info) ); return TestCase( _testCase, std::move(info) );
} }

View File

@ -105,7 +105,7 @@ namespace Catch {
} }
std::string extractClassName( StringRef const& classOrQualifiedMethodName ) { std::string extractClassName( StringRef const& classOrQualifiedMethodName ) {
std::string className = classOrQualifiedMethodName; std::string className(classOrQualifiedMethodName);
if( startsWith( className, '&' ) ) if( startsWith( className, '&' ) )
{ {
std::size_t lastColons = className.rfind( "::" ); std::size_t lastColons = className.rfind( "::" );

View File

@ -654,7 +654,7 @@ namespace Catch { \
template<> struct StringMaker<enumName> { \ template<> struct StringMaker<enumName> { \
static std::string convert( enumName value ) { \ static std::string convert( enumName value ) { \
static const auto& enumInfo = ::Catch::getMutableRegistryHub().getMutableEnumValuesRegistry().registerEnum( #enumName, #__VA_ARGS__, { __VA_ARGS__ } ); \ static const auto& enumInfo = ::Catch::getMutableRegistryHub().getMutableEnumValuesRegistry().registerEnum( #enumName, #__VA_ARGS__, { __VA_ARGS__ } ); \
return enumInfo.lookup( static_cast<int>( value ) ); \ return static_cast<std::string>(enumInfo.lookup( static_cast<int>( value ) )); \
} \ } \
}; \ }; \
} }

View File

@ -1043,8 +1043,6 @@ String.tests.cpp:<line number>: passed: stdStr == "a stringref" for: "a stringre
String.tests.cpp:<line number>: passed: stdStr.size() == sr.size() for: 11 == 11 String.tests.cpp:<line number>: passed: stdStr.size() == sr.size() for: 11 == 11
String.tests.cpp:<line number>: passed: stdStr == "a stringref" for: "a stringref" == "a stringref" String.tests.cpp:<line number>: passed: stdStr == "a stringref" for: "a stringref" == "a stringref"
String.tests.cpp:<line number>: passed: stdStr.size() == sr.size() for: 11 == 11 String.tests.cpp:<line number>: passed: stdStr.size() == sr.size() for: 11 == 11
String.tests.cpp:<line number>: passed: stdStr == "a stringref" for: "a stringref" == "a stringref"
String.tests.cpp:<line number>: passed: stdStr.size() == sr.size() for: 11 == 11
ToStringChrono.tests.cpp:<line number>: passed: minute == seconds for: 1 m == 60 s ToStringChrono.tests.cpp:<line number>: passed: minute == seconds for: 1 m == 60 s
ToStringChrono.tests.cpp:<line number>: passed: hour != seconds for: 1 h != 60 s ToStringChrono.tests.cpp:<line number>: passed: hour != seconds for: 1 h != 60 s
ToStringChrono.tests.cpp:<line number>: passed: micro != milli for: 1 us != 1 ms ToStringChrono.tests.cpp:<line number>: passed: micro != milli for: 1 us != 1 ms

View File

@ -1381,5 +1381,5 @@ due to unexpected exception with message:
=============================================================================== ===============================================================================
test cases: 300 | 226 passed | 70 failed | 4 failed as expected test cases: 300 | 226 passed | 70 failed | 4 failed as expected
assertions: 1559 | 1407 passed | 131 failed | 21 failed as expected assertions: 1557 | 1405 passed | 131 failed | 21 failed as expected

View File

@ -7783,24 +7783,6 @@ String.tests.cpp:<line number>: PASSED:
with expansion: with expansion:
17 == 17 17 == 17
-------------------------------------------------------------------------------
StringRef
to std::string
implicitly constructed
-------------------------------------------------------------------------------
String.tests.cpp:<line number>
...............................................................................
String.tests.cpp:<line number>: PASSED:
REQUIRE( stdStr == "a stringref" )
with expansion:
"a stringref" == "a stringref"
String.tests.cpp:<line number>: PASSED:
REQUIRE( stdStr.size() == sr.size() )
with expansion:
11 == 11
------------------------------------------------------------------------------- -------------------------------------------------------------------------------
StringRef StringRef
to std::string to std::string
@ -12462,5 +12444,5 @@ Misc.tests.cpp:<line number>: PASSED:
=============================================================================== ===============================================================================
test cases: 300 | 210 passed | 86 failed | 4 failed as expected test cases: 300 | 210 passed | 86 failed | 4 failed as expected
assertions: 1576 | 1407 passed | 148 failed | 21 failed as expected assertions: 1574 | 1405 passed | 148 failed | 21 failed as expected

View File

@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?> <?xml version="1.0" encoding="UTF-8"?>
<testsuitesloose text artifact <testsuitesloose text artifact
> >
<testsuite name="<exe-name>" errors="17" failures="132" tests="1577" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}"> <testsuite name="<exe-name>" errors="17" failures="132" tests="1575" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
<properties> <properties>
<property name="filters" value="~[!nonportable]~[!benchmark]~[approvals]"/> <property name="filters" value="~[!nonportable]~[!benchmark]~[approvals]"/>
<property name="random-seed" value="1"/> <property name="random-seed" value="1"/>
@ -726,7 +726,6 @@ Matchers.tests.cpp:<line number>
<testcase classname="<exe-name>.global" name="StringRef/from std::string/implicitly constructed" time="{duration}"/> <testcase classname="<exe-name>.global" name="StringRef/from std::string/implicitly constructed" time="{duration}"/>
<testcase classname="<exe-name>.global" name="StringRef/from std::string/explicitly constructed" time="{duration}"/> <testcase classname="<exe-name>.global" name="StringRef/from std::string/explicitly constructed" time="{duration}"/>
<testcase classname="<exe-name>.global" name="StringRef/from std::string/assigned" time="{duration}"/> <testcase classname="<exe-name>.global" name="StringRef/from std::string/assigned" time="{duration}"/>
<testcase classname="<exe-name>.global" name="StringRef/to std::string/implicitly constructed" time="{duration}"/>
<testcase classname="<exe-name>.global" name="StringRef/to std::string/explicitly constructed" time="{duration}"/> <testcase classname="<exe-name>.global" name="StringRef/to std::string/explicitly constructed" time="{duration}"/>
<testcase classname="<exe-name>.global" name="StringRef/to std::string/assigned" time="{duration}"/> <testcase classname="<exe-name>.global" name="StringRef/to std::string/assigned" time="{duration}"/>
<testcase classname="<exe-name>.global" name="Stringifying std::chrono::duration helpers" time="{duration}"/> <testcase classname="<exe-name>.global" name="Stringifying std::chrono::duration helpers" time="{duration}"/>

View File

@ -9601,28 +9601,6 @@ Message from section two
</Section> </Section>
<OverallResults successes="2" failures="0" expectedFailures="0"/> <OverallResults successes="2" failures="0" expectedFailures="0"/>
</Section> </Section>
<Section name="to std::string" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Section name="implicitly constructed" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Expression success="true" type="REQUIRE" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Original>
stdStr == "a stringref"
</Original>
<Expanded>
"a stringref" == "a stringref"
</Expanded>
</Expression>
<Expression success="true" type="REQUIRE" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Original>
stdStr.size() == sr.size()
</Original>
<Expanded>
11 == 11
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
</Section>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
</Section>
<Section name="to std::string" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" > <Section name="to std::string" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Section name="explicitly constructed" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" > <Section name="explicitly constructed" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
<Expression success="true" type="REQUIRE" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" > <Expression success="true" type="REQUIRE" filename="projects/<exe-name>/IntrospectiveTests/String.tests.cpp" >
@ -14834,7 +14812,7 @@ loose text artifact
</Section> </Section>
<OverallResult success="true"/> <OverallResult success="true"/>
</TestCase> </TestCase>
<OverallResults successes="1407" failures="149" expectedFailures="21"/> <OverallResults successes="1405" failures="149" expectedFailures="21"/>
</Group> </Group>
<OverallResults successes="1407" failures="148" expectedFailures="21"/> <OverallResults successes="1405" failures="148" expectedFailures="21"/>
</Catch> </Catch>

View File

@ -142,11 +142,6 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) {
SECTION( "to std::string" ) { SECTION( "to std::string" ) {
StringRef sr = "a stringref"; StringRef sr = "a stringref";
SECTION( "implicitly constructed" ) {
std::string stdStr = sr;
REQUIRE( stdStr == "a stringref" );
REQUIRE( stdStr.size() == sr.size() );
}
SECTION( "explicitly constructed" ) { SECTION( "explicitly constructed" ) {
std::string stdStr( sr ); std::string stdStr( sr );
REQUIRE( stdStr == "a stringref" ); REQUIRE( stdStr == "a stringref" );
@ -154,7 +149,7 @@ TEST_CASE( "StringRef", "[Strings][StringRef]" ) {
} }
SECTION( "assigned" ) { SECTION( "assigned" ) {
std::string stdStr; std::string stdStr;
stdStr = sr; stdStr = static_cast<std::string>(sr);
REQUIRE( stdStr == "a stringref" ); REQUIRE( stdStr == "a stringref" );
REQUIRE( stdStr.size() == sr.size() ); REQUIRE( stdStr.size() == sr.size() );
} }