Regex: use iterators for match results

* Use `smatch` to match std::string instead of char pointer * Use iterators to walk the results instead of integer indices * Store position as ptrdiff_t to always have the correct size This prevents signed/unsigned and narrowing (64-bit to 32-bit) conversion warnings on MSVC.
cucumber · Aug 27, 2017 · a7ff906 · a7ff906
1 parent 4dec371
commit a7ff906
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 21 deletions.
diff --git a/include/cucumber-cpp/internal/CukeEngine.hpp b/include/cucumber-cpp/internal/CukeEngine.hpp
@@ -1,6 +1,7 @@
 #ifndef CUKE_CUKEENGINE_HPP_
 #define CUKE_CUKEENGINE_HPP_
 
+#include <cstddef>
 #include <string>
 #include <vector>
 
@@ -12,7 +13,7 @@ namespace internal {
 class StepMatchArg {
 public:
     std::string value;
-    int position;
+    std::ptrdiff_t position;
 };
 
 class StepMatch {

diff --git a/include/cucumber-cpp/internal/utils/Regex.hpp b/include/cucumber-cpp/internal/utils/Regex.hpp
@@ -1,6 +1,7 @@
 #ifndef CUKE_REGEX_HPP_
 #define CUKE_REGEX_HPP_
 
+#include <cstddef>
 #include <vector>
 
 #include <boost/shared_ptr.hpp>
@@ -11,7 +12,7 @@ namespace internal {
 
 struct RegexSubmatch {
     std::string value;
-    int position;
+    std::ptrdiff_t position;
 };
 
 

diff --git a/src/Regex.cpp b/src/Regex.cpp
@@ -25,14 +25,18 @@ boost::shared_ptr<RegexMatch> Regex::find(const std::string &expression) const {
 }
 
 FindRegexMatch::FindRegexMatch(const boost::regex &regexImpl, const std::string &expression) {
-    boost::cmatch matchResults;
-    regexMatched = boost::regex_search(expression.c_str(), matchResults, regexImpl, boost::regex_constants::match_extra);
+    boost::smatch matchResults;
+    regexMatched = boost::regex_search(
+        expression, matchResults, regexImpl, boost::regex_constants::match_extra);
     if (regexMatched) {
-        for (boost::cmatch::size_type i = 1; i < matchResults.size(); ++i) {
-            RegexSubmatch s;
-            s.value = matchResults.str(i);
-            s.position = matchResults.position(i);
-            submatches.push_back(s);
+        boost::smatch::const_iterator i = matchResults.begin();
+        if (i != matchResults.end())
+            ++i;
+        for (; i != matchResults.end(); ++i) {
+            if (i->matched) {
+                RegexSubmatch s = {*i, i->first - expression.begin()};
+                submatches.push_back(s);
+            }
         }
     }
 }
@@ -42,17 +46,13 @@ boost::shared_ptr<RegexMatch> Regex::findAll(const std::string &expression) cons
 }
 
 FindAllRegexMatch::FindAllRegexMatch(const boost::regex &regexImpl, const std::string &expression) {
-    regexMatched = false;
     boost::sregex_token_iterator i(expression.begin(), expression.end(), regexImpl, 1, boost::regex_constants::match_continuous);
-    boost::sregex_token_iterator j;
-    while (i != j) {
-        regexMatched = true;
-        RegexSubmatch s;
-        s.value = *i;
-        s.position = -1;
+    const boost::sregex_token_iterator end;
+    for (; i != end; ++i) {
+        RegexSubmatch s = {*i, -1};
         submatches.push_back(s);
-        ++i;
     }
+    regexMatched = !submatches.empty();
 }
 
 }

diff --git a/src/connectors/wire/WireProtocol.cpp b/src/connectors/wire/WireProtocol.cpp
@@ -268,7 +268,7 @@ namespace {
                 BOOST_FOREACH(StepMatchArg ma, m.args) {
                     mObject jsonMa;
                     jsonMa["val"] = ma.value;
-                    jsonMa["pos"] = ma.position;
+                    jsonMa["pos"] = static_cast<boost::int64_t>(ma.position);
                     jsonArgs.push_back(jsonMa);
                 }
                 jsonM["args"] = jsonArgs;

diff --git a/tests/unit/StepManagerTest.cpp b/tests/unit/StepManagerTest.cpp
@@ -19,7 +19,7 @@ class StepManagerTest : public ::testing::Test {
     const static char *another_matcher;
     const static char *no_match;
     const static char *a_third_matcher;
-    const map<int, string> no_params;
+    const map<std::ptrdiff_t, string> no_params;
 
     int getUniqueMatchIdOrZeroFor(const string &stepMatch) {
         MatchResult::match_results_type resultSet = getResultSetFor(stepMatch);
@@ -30,7 +30,7 @@ class StepManagerTest : public ::testing::Test {
         }
     }
 
-    int countMatches(const string &stepMatch) {
+    size_t countMatches(const string &stepMatch) {
         return getResultSetFor(stepMatch).size();
     }
 
@@ -42,7 +42,7 @@ class StepManagerTest : public ::testing::Test {
         return (countMatches(stepMatch) > 0);
     }
 
-    bool extractedParamsAre(const string stepMatch, map<int, string> params) {
+    bool extractedParamsAre(const string stepMatch, map<std::ptrdiff_t, string> params) {
         MatchResult::match_results_type resultSet = getResultSetFor(stepMatch);
         if (resultSet.size() != 1) {
             return false; // more than one match