Don't check tags for being special twice during test case registration

Previously we would check if the tag is special to validate that it isn't
reserved, and then to actually get the test case properties. We also did
the first check in particularly terrible order, leading to a lot of wasted
work.

The new implementation does property extraction while checking validity,
and is structured to avoid as much work as possible by early checks.

1 files changed, 22 insertions, 28 deletions

diff --git a/src/catch2/catch_test_case_info.cpp b/src/catch2/catch_test_case_info.cpp
index 4ccc3d52..6c20c93e 100644
--- a/src/catch2/catch_test_case_info.cpp
+++ b/src/catch2/catch_test_case_info.cpp
@@ -48,32 +48,27 @@ namespace Catch {
             return tcp != TestCaseProperties::None;
         }
 
-        TestCaseProperties parseSpecialTag( StringRef tag ) {
-            if( !tag.empty() && tag[0] == '.' )
-                return TestCaseProperties::IsHidden;
-            else if( tag == "!throws"_sr )
-                return TestCaseProperties::Throws;
-            else if( tag == "!shouldfail"_sr )
-                return TestCaseProperties::ShouldFail;
-            else if( tag == "!mayfail"_sr )
-                return TestCaseProperties::MayFail;
-            else if( tag == "!nonportable"_sr )
-                return TestCaseProperties::NonPortable;
-            else if( tag == "!benchmark"_sr )
-                return TestCaseProperties::Benchmark | TestCaseProperties::IsHidden;
-            else
-                return TestCaseProperties::None;
-        }
-        bool isReservedTag( StringRef tag ) {
-            return parseSpecialTag( tag ) == TestCaseProperties::None
-                && tag.size() > 0
-                && !std::isalnum( static_cast<unsigned char>(tag[0]) );
-        }
-        void enforceNotReservedTag( StringRef tag, SourceLineInfo const& _lineInfo ) {
-            CATCH_ENFORCE( !isReservedTag(tag),
-                          "Tag name: [" << tag << "] is not allowed.\n"
-                          << "Tag names starting with non alphanumeric characters are reserved\n"
-                          << _lineInfo );
+        TestCaseProperties propertiesFromTag( StringRef tag, SourceLineInfo const& _lineInfo ) {
+            if ( tag.empty() ) { return TestCaseProperties::None; }
+            // All tags starting with alphanumeric characters are normal.
+            if ( std::isalnum( tag[0] ) ) { return TestCaseProperties::None; }
+
+            // Explicitly check known special tags, and error out if it is not one of them
+            if ( tag[0] == '.' ) { return TestCaseProperties::IsHidden; }
+            else if ( tag == "!throws"_sr ) { return TestCaseProperties::Throws; }
+            else if ( tag == "!shouldfail"_sr ) { return TestCaseProperties::ShouldFail; }
+            else if ( tag == "!mayfail"_sr ) { return TestCaseProperties::MayFail; }
+            else if ( tag == "!nonportable"_sr ) { return TestCaseProperties::NonPortable; }
+            else if ( tag == "!benchmark"_sr ) {
+                return TestCaseProperties::Benchmark |
+                       TestCaseProperties::IsHidden;
+            } else {
+                CATCH_RUNTIME_ERROR(
+                    "Tag name: [" << tag << "] is not allowed.\n"
+                                  << "Tag names starting with non-alphanumeric "
+                                     "characters are reserved.\n"
+                                  << _lineInfo );
+            }
         }
 
         std::string makeDefaultName() {
@@ -182,8 +177,7 @@ namespace Catch {
                                    << _nameAndTags.name << "' at "
                                    << _lineInfo );
 
-                enforceNotReservedTag(tagStr, lineInfo);
-                properties |= parseSpecialTag(tagStr);
+                properties |= propertiesFromTag(tagStr, _lineInfo);
                 // When copying a tag to the backing storage, we need to
                 // check if it is a merged hide tag, such as [.foo], and
                 // if it is, we need to handle it as if it was [foo].