Regex: hide implementation details

Author: firewave

Makefile

@@ -51,6 +51,7 @@
 #include <iostream>
 #include <iterator>
 #include <list>
+#include <memory>
 #include <set>
 #include <sstream>
 #include <unordered_set>
@@ -1260,9 +1261,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 return Result::Fail;
             }
 
-            Regex regex;
-            const std::string regex_err = regex.compile(rule.pattern);
-            if (!regex_err.empty()) {
+            std::string regex_err;
+            auto regex = Regex::create(rule.pattern, regex_err);
+            if (!regex) {
                 mLogger.printError("failed to compile rule pattern '" + rule.pattern + "' (" + regex_err + ").");
                 return Result::Fail;
             }
@@ -1344,11 +1345,13 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                         return Result::Fail;
                     }
 
-                    const std::string regex_err = rule.regex.compile(rule.pattern);
-                    if (!regex_err.empty()) {
+                    std::string regex_err;
+                    auto regex = Regex::create(rule.pattern, regex_err);
+                    if (!regex) {
                         mLogger.printError("unable to load rule-file '" + ruleFile + "' - pattern '" + rule.pattern + "' failed to compile (" + regex_err + ").");
                         return Result::Fail;
                     }
+                    rule.regex = std::move(regex);
 
                     if (rule.severity == Severity::none) {
                         mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule has an invalid severity.");

cli/cmdlineparser.cpp

@@ -1481,7 +1481,7 @@ void CppCheck::executeRules(const std::string &tokenlist, const TokenList &list)
             mErrorLogger.reportErr(errmsg);
         };
 
-        const std::string err = rule.regex.match(str, f);
+        const std::string err = rule.regex->match(str, f);
         if (!err.empty()) {
             const ErrorMessage errmsg(std::list<ErrorMessage::FileLocation>(),
                                       emptyString,

lib/cppcheck.cpp

@@ -156,115 +156,105 @@ namespace {
         }
         return "unknown PCRE error " + std::to_string(pcreExecRet);
     }
-}
-
-class Regex::Data
-{
-public:
-    explicit Data(std::string pattern)
-        : mPattern(std::move(pattern))
-    {}
 
-    ~Data()
+    class PcreRegex : public Regex
     {
-        if (mExtra) {
-            pcre_free(mExtra);
-            mExtra = nullptr;
-        }
-        if (mRe) {
-            pcre_free(mRe);
-            mRe = nullptr;
+    public:
+        explicit PcreRegex(std::string pattern)
+            : mPattern(std::move(pattern))
+        {}
+
+        ~PcreRegex() override
+        {
+            if (mExtra) {
+                pcre_free(mExtra);
+                mExtra = nullptr;
+            }
+            if (mRe) {
+                pcre_free(mRe);
+                mRe = nullptr;
+            }
         }
-    }
 
-    std::string compile();
-    std::string match(const std::string& str, const MatchFn& match) const;
+        std::string compile();
+        std::string match(const std::string& str, const MatchFn& match) const override;
 
-private:
-    std::string mPattern;
-    pcre* mRe{};
-    pcre_extra* mExtra{};
-};
+    private:
+        std::string mPattern;
+        pcre* mRe{};
+        pcre_extra* mExtra{};
+    };
 
-std::string Regex::Data::compile()
-{
-    if (mRe)
-        return "pcre_compile failed: regular expression has already been compiled";
+    std::string PcreRegex::compile()
+    {
+        if (mRe)
+            return "pcre_compile failed: regular expression has already been compiled";
 
-    const char *pcreCompileErrorStr = nullptr;
-    int erroffset = 0;
-    pcre * const re = pcre_compile(mPattern.c_str(),0,&pcreCompileErrorStr,&erroffset,nullptr);
-    if (!re) {
-        if (pcreCompileErrorStr)
-            return "pcre_compile failed: " + std::string(pcreCompileErrorStr);
-        return "pcre_compile failed: unknown error";
-    }
+        const char *pcreCompileErrorStr = nullptr;
+        int erroffset = 0;
+        pcre * const re = pcre_compile(mPattern.c_str(),0,&pcreCompileErrorStr,&erroffset,nullptr);
+        if (!re) {
+            if (pcreCompileErrorStr)
+                return "pcre_compile failed: " + std::string(pcreCompileErrorStr);
+            return "pcre_compile failed: unknown error";
+        }
 
-    // Optimize the regex, but only if PCRE_CONFIG_JIT is available
+        // Optimize the regex, but only if PCRE_CONFIG_JIT is available
 #ifdef PCRE_CONFIG_JIT
-    const char *pcreStudyErrorStr = nullptr;
-    pcre_extra * const pcreExtra = pcre_study(re, PCRE_STUDY_JIT_COMPILE, &pcreStudyErrorStr);
-    // pcre_study() returns NULL for both errors and when it can not optimize the regex.
-    // The last argument is how one checks for errors.
-    // It is NULL if everything works, and points to an error string otherwise.
-    if (pcreStudyErrorStr) {
-        // pcre_compile() worked, but pcre_study() returned an error. Free the resources allocated by pcre_compile().
-        pcre_free(re);
-        return "pcre_study failed: " + std::string(pcreStudyErrorStr);
-    }
-    mExtra = pcreExtra;
+        const char *pcreStudyErrorStr = nullptr;
+        pcre_extra * const pcreExtra = pcre_study(re, PCRE_STUDY_JIT_COMPILE, &pcreStudyErrorStr);
+        // pcre_study() returns NULL for both errors and when it can not optimize the regex.
+        // The last argument is how one checks for errors.
+        // It is NULL if everything works, and points to an error string otherwise.
+        if (pcreStudyErrorStr) {
+            // pcre_compile() worked, but pcre_study() returned an error. Free the resources allocated by pcre_compile().
+            pcre_free(re);
+            return "pcre_study failed: " + std::string(pcreStudyErrorStr);
+        }
+        mExtra = pcreExtra;
 #endif
 
-    mRe = re;
+        mRe = re;
 
-    return "";
-}
-
-std::string Regex::Data::match(const std::string& str, const MatchFn& match) const
-{
-    if (!mRe)
-        return "pcre_exec failed: regular expression has not been compiled yet";
-
-    int pos = 0;
-    int ovector[30]= {0};
-    while (pos < static_cast<int>(str.size())) {
-        const int pcreExecRet = pcre_exec(mRe, mExtra, str.c_str(), static_cast<int>(str.size()), pos, 0, ovector, 30);
-        if (pcreExecRet == PCRE_ERROR_NOMATCH)
-            return "";
-        if (pcreExecRet < 0) {
-            return std::string("pcre_exec failed (pos: " + std::to_string(pos) + "): ") + pcreErrorCodeToString(pcreExecRet);
-        }
-        const auto pos1 = static_cast<unsigned int>(ovector[0]);
-        const auto pos2 = static_cast<unsigned int>(ovector[1]);
+        return "";
+    }
 
-        match(pos1, pos2);
+    std::string PcreRegex::match(const std::string& str, const MatchFn& match) const
+    {
+        if (!mRe)
+            return "pcre_exec failed: regular expression has not been compiled yet";
 
-        // jump to the end of the match for the next pcre_exec
-        pos = static_cast<int>(pos2);
-    }
+        int pos = 0;
+        int ovector[30]= {0};
+        while (pos < static_cast<int>(str.size())) {
+            const int pcreExecRet = pcre_exec(mRe, mExtra, str.c_str(), static_cast<int>(str.size()), pos, 0, ovector, 30);
+            if (pcreExecRet == PCRE_ERROR_NOMATCH)
+                return "";
+            if (pcreExecRet < 0) {
+                return std::string("pcre_exec failed (pos: " + std::to_string(pos) + "): ") + pcreErrorCodeToString(pcreExecRet);
+            }
+            const auto pos1 = static_cast<unsigned int>(ovector[0]);
+            const auto pos2 = static_cast<unsigned int>(ovector[1]);
 
-    return "";
-}
+            match(pos1, pos2);
 
-std::string Regex::compile(std::string pattern)
-{
-    if (mData)
-        return "regular expression has already been compiled";
+            // jump to the end of the match for the next pcre_exec
+            pos = static_cast<int>(pos2);
+        }
 
-    auto* data = new Data(std::move(pattern));
-    auto res = data->compile();
-    if (res.empty())
-        mData.reset(data);
-    else
-        delete data;
-    return res;
+        return "";
+    }
 }
 
-std::string Regex::match(const std::string& str, const MatchFn& match) const
+std::shared_ptr<Regex> Regex::create(std::string pattern, std::string& err)
 {
-    if (!mData)
-        return "regular expression hat not been compiled yet";
-    return mData->match(str, match);
+    auto* regex = new PcreRegex(std::move(pattern));
+    err = regex->compile();
+    if (!err.empty()) {
+        delete regex;
+        return nullptr;
+    }
+    return std::shared_ptr<Regex>(regex);
 }
 
 #endif // HAVE_RULES

lib/regex.cpp

@@ -32,14 +32,12 @@
 class CPPCHECKLIB Regex
 {
 public:
-    std::string compile(std::string pattern);
+    virtual ~Regex() = default;
 
     using MatchFn = std::function<void (int start, int end)>;
-    std::string match(const std::string& str, const MatchFn& match)const;
+    virtual std::string match(const std::string& str, const MatchFn& matchFn) const = 0;
 
-private:
-    class Data;
-    std::shared_ptr<Data> mData;
+    static std::shared_ptr<Regex> create(std::string pattern, std::string& err);
 };
 
 #endif // HAVE_RULES

lib/regex.h

@@ -45,7 +45,9 @@
 #endif
 
 #ifdef HAVE_RULES
-#include "regex.h"
+#include <memory>
+
+class Regex;
 #endif
 
 struct Suppressions;
@@ -342,7 +344,7 @@ class CPPCHECKLIB WARN_UNUSED Settings {
         std::string id = "rule"; // default id
         std::string summary;
         Severity severity = Severity::style; // default severity
-        Regex regex;
+        std::shared_ptr<Regex> regex;
     };
 
     /**