Adapt to Contour's best code quality & maintenance practice.

Signed-off-by: Christian Parpart <[email protected]>

Author: christianparpart

.github/workflows/build.yml

@@ -410,6 +410,8 @@ jobs:
         run: cmake --build build/ -- -j3
       - name: "test: crispy"
         run: ./build/src/crispy/crispy_test
+      - name: "test: regex_dfa"
+        run: ./build/src/regex_dfa/regex_dfa_test
       - name: "test: vtparser"
         run: ./build/src/vtparser/vtparser_test
       - name: "test: libterminal"
@@ -421,6 +423,7 @@ jobs:
           name: contour-ubuntu2204-tests
           path: |
             build/src/crispy/crispy_test
+            build/src/regex_dfa/regex_dfa_test
             build/src/vtparser/vtparser_test
             build/src/vtbackend/vtbackend_test
             build/src/vtbackend/bench-headless

src/regex_dfa/Alphabet.h

@@ -25,14 +25,14 @@ class Alphabet
     using set_type = std::set<Symbol>;
     using iterator = set_type::iterator;
 
-    size_t size() const noexcept { return alphabet_.size(); }
+    [[nodiscard]] size_t size() const noexcept { return alphabet_.size(); }
 
     void insert(Symbol ch);
 
-    std::string to_string() const;
+    [[nodiscard]] std::string to_string() const;
 
-    const iterator begin() const { return alphabet_.begin(); }
-    const iterator end() const { return alphabet_.end(); }
+    [[nodiscard]] iterator begin() const { return alphabet_.begin(); }
+    [[nodiscard]] iterator end() const { return alphabet_.end(); }
 
   private:
     set_type alphabet_;

src/regex_dfa/CMakeLists.txt

@@ -21,7 +21,9 @@ target_include_directories(regex_dfa PUBLIC ${PROJECT_SOURCE_DIR}/src ${CMAKE_SO
 target_link_libraries(regex_dfa PUBLIC fmt::fmt-header-only)
 
 # ----------------------------------------------------------------------------
-if(TESTS)
+option(REGEX_DFA_TESTING "Enables building of unittests for regex_dfa library [default: ON]" ON)
+if(REGEX_DFA_TESTING)
+    enable_testing()
     add_executable(regex_dfa_test
         regex_dfa_test.cpp
         DFABuilder_test.cpp
@@ -33,9 +35,9 @@ if(TESTS)
         State_test.cpp
         Symbols_test.cpp
         util/iterator_test.cpp
-        util/testing.cpp
     )
 
     target_link_libraries(regex_dfa_test PUBLIC regex_dfa)
+    target_link_libraries(regex_dfa_test PUBLIC Catch2::Catch2)
     target_link_libraries(regex_dfa_test PUBLIC fmt::fmt-header-only)
-endif(TESTS)
+endif(REGEX_DFA_TESTING)

src/regex_dfa/Compiler.cpp

@@ -134,7 +134,7 @@ DFA Compiler::compileMinimalDFA()
 
 LexerDef Compiler::compile()
 {
-    return generateTables(compileMinimalDFA(), containsBeginOfLine_, move(names_));
+    return generateTables(compileMinimalDFA(), containsBeginOfLine_, std::move(names_));
 }
 
 LexerDef Compiler::compileMulti(OvershadowMap* overshadows)

src/regex_dfa/Compiler.h

@@ -48,9 +48,9 @@ class Compiler
      */
     void declareAll(RuleList rules);
 
-    const RuleList& rules() const noexcept { return rules_; }
-    const TagNameMap& names() const noexcept { return names_; }
-    size_t size() const;
+    [[nodiscard]] const RuleList& rules() const noexcept { return rules_; }
+    [[nodiscard]] const TagNameMap& names() const noexcept { return names_; }
+    [[nodiscard]] size_t size() const;
 
     /**
      * Compiles all previousely parsed rules into a DFA.
@@ -84,9 +84,9 @@ class Compiler
     static LexerDef generateTables(const DFA& dfa, bool requiresBeginOfLine, const TagNameMap& names);
     static LexerDef generateTables(const MultiDFA& dfa, bool requiresBeginOfLine, const TagNameMap& names);
 
-    const std::map<std::string, NFA>& automata() const { return fa_; }
+    [[nodiscard]] const std::map<std::string, NFA>& automata() const { return fa_; }
 
-    bool containsBeginOfLine() const noexcept { return containsBeginOfLine_; }
+    [[nodiscard]] bool containsBeginOfLine() const noexcept { return containsBeginOfLine_; }
 
   private:
     /**

src/regex_dfa/DFA.cpp

@@ -37,7 +37,7 @@ Alphabet DFA::alphabet() const
 {
     Alphabet alphabet;
     for (const State& state: states_)
-        for (const pair<Symbol, StateId>& t: state.transitions)
+        for (pair<Symbol, StateId> const t: state.transitions)
             alphabet.insert(t.first);
 
     return alphabet;
@@ -118,12 +118,12 @@ void DFA::prepareStateIds(StateId baseId, StateId q0)
     AcceptMap remapped;
     for (auto& a: acceptTags_)
         remapped[transformId(a.first)] = a.second;
-    acceptTags_ = move(remapped);
+    acceptTags_ = std::move(remapped);
 
     BacktrackingMap backtracking;
     for (const auto& bt: backtrackStates_)
         backtracking[transformId(bt.first)] = transformId(bt.second);
-    backtrackStates_ = move(backtracking);
+    backtrackStates_ = std::move(backtracking);
 
     initialState_ = q0;
 }

src/regex_dfa/DFA.h

@@ -57,16 +57,16 @@ class DFA
     }
 
     //! Retrieves the alphabet of this finite automaton.
-    Alphabet alphabet() const;
+    [[nodiscard]] Alphabet alphabet() const;
 
     //! Retrieves the initial state.
-    StateId initialState() const { return initialState_; }
+    [[nodiscard]] StateId initialState() const { return initialState_; }
 
     //! Retrieves the list of available states.
-    const StateVec& states() const { return states_; }
-    StateVec& states() { return states_; }
+    [[nodiscard]] const StateVec& states() const { return states_; }
+    [[nodiscard]] StateVec& states() { return states_; }
 
-    StateIdVec stateIds() const
+    [[nodiscard]] StateIdVec stateIds() const
     {
         StateIdVec v;
         v.reserve(states_.size());
@@ -76,7 +76,7 @@ class DFA
     }
 
     //! Retrieves the list of accepting states.
-    std::vector<StateId> acceptStates() const;
+    [[nodiscard]] std::vector<StateId> acceptStates() const;
 
     /**
      * Traverses all states and edges in this NFA and calls @p visitor for each state & edge.
@@ -89,39 +89,39 @@ class DFA
 
     void setInitialState(StateId state);
 
-    const TransitionMap& stateTransitions(StateId id) const
+    [[nodiscard]] const TransitionMap& stateTransitions(StateId id) const
     {
         return states_[static_cast<size_t>(id)].transitions;
     }
 
     // {{{ backtracking (for lookahead)
     void setBacktrack(StateId from, StateId to) { backtrackStates_[from] = to; }
 
-    std::optional<StateId> backtrack(StateId acceptState) const
+    [[nodiscard]] std::optional<StateId> backtrack(StateId acceptState) const
     {
         if (auto i = backtrackStates_.find(acceptState); i != backtrackStates_.end())
             return i->second;
 
         return std::nullopt;
     }
 
-    const BacktrackingMap& backtracking() const noexcept { return backtrackStates_; }
+    [[nodiscard]] const BacktrackingMap& backtracking() const noexcept { return backtrackStates_; }
     // }}}
 
     //! Flags given state as accepting-state with given Tag @p acceptTag.
     void setAccept(StateId state, Tag acceptTag) { acceptTags_[state] = acceptTag; }
 
-    bool isAccepting(StateId s) const { return acceptTags_.find(s) != acceptTags_.end(); }
+    [[nodiscard]] bool isAccepting(StateId s) const { return acceptTags_.find(s) != acceptTags_.end(); }
 
-    std::optional<Tag> acceptTag(StateId s) const
+    [[nodiscard]] std::optional<Tag> acceptTag(StateId s) const
     {
         if (auto i = acceptTags_.find(s); i != acceptTags_.end())
             return i->second;
 
         return std::nullopt;
     }
 
-    std::optional<StateId> delta(StateId state, Symbol symbol) const
+    [[nodiscard]] std::optional<StateId> delta(StateId state, Symbol symbol) const
     {
         const auto& T = states_[state].transitions;
         if (auto i = T.find(symbol); i != T.end())
@@ -133,7 +133,7 @@ class DFA
     void setTransition(StateId from, Symbol symbol, StateId to);
     void removeTransition(StateId from, Symbol symbol);
 
-    StateIdVec nonAcceptStates() const
+    [[nodiscard]] StateIdVec nonAcceptStates() const
     {
         StateIdVec result;
         result.reserve(
@@ -146,9 +146,9 @@ class DFA
         return result;
     }
 
-    bool isAcceptor(Tag t) const
+    [[nodiscard]] bool isAcceptor(Tag t) const
     {
-        for (const std::pair<StateId, Tag>& p: acceptTags_)
+        for (std::pair<StateId, Tag> p: acceptTags_)
             if (p.second == t)
                 return true;
 

src/regex_dfa/DFABuilder.h

@@ -32,19 +32,19 @@ class DFABuilder
      * @param overshadows if not nullptr, it will be used to store semantic information about
      *                    which rule tags have been overshadowed by which.
      */
-    DFA construct(OvershadowMap* overshadows = nullptr);
+    [[nodiscard]] DFA construct(OvershadowMap* overshadows = nullptr);
 
   private:
     struct TransitionTable;
 
-    DFA constructDFA(const std::vector<StateIdVec>& Q,
-                     const TransitionTable& T,
-                     OvershadowMap* overshadows) const;
+    [[nodiscard]] DFA constructDFA(const std::vector<StateIdVec>& Q,
+                                   const TransitionTable& T,
+                                   OvershadowMap* overshadows) const;
 
     /**
      * Finds @p t in @p Q and returns its offset (aka configuration number) or -1 if not found.
      */
-    static std::optional<StateId> configurationNumber(const std::vector<StateIdVec>& Q, const StateIdVec& t);
+    [[nodiscard]] static std::optional<StateId> configurationNumber(const std::vector<StateIdVec>& Q, const StateIdVec& t);
 
     /**
      * Determines the tag to use for the deterministic state representing @p q from non-deterministic FA @p
@@ -54,7 +54,7 @@ class DFABuilder
      *
      * @returns the determined tag or std::nullopt if none
      */
-    std::optional<Tag> determineTag(const StateIdVec& q, std::map<Tag, Tag>* overshadows) const;
+    [[nodiscard]] std::optional<Tag> determineTag(const StateIdVec& q, std::map<Tag, Tag>* overshadows) const;
 
   private:
     const NFA nfa_;

src/regex_dfa/DFABuilder_test.cpp

@@ -10,14 +10,14 @@
 #include <regex_dfa/DFABuilder.h>
 #include <regex_dfa/MultiDFA.h>
 
+#include <catch2/catch.hpp>
+
 #include <memory>
 #include <sstream>
 
-#include <klex/util/testing.h>
-
 using namespace regex_dfa;
 
-TEST(regex_DFABuilder, shadowing)
+TEST_CASE("regex_DFABuilder.shadowing")
 {
     Compiler cc;
     cc.parse(std::make_unique<std::stringstream>(R"(
@@ -27,7 +27,7 @@ TEST(regex_DFABuilder, shadowing)
     // rule 2 is overshadowed by rule 1
     Compiler::OvershadowMap overshadows;
     DFA dfa = cc.compileDFA(&overshadows);
-    ASSERT_EQ(1, overshadows.size());
-    EXPECT_EQ(2, overshadows[0].first);  // overshadowee
-    EXPECT_EQ(1, overshadows[0].second); // overshadower
+    REQUIRE(1 == overshadows.size());
+    CHECK(2 == overshadows[0].first);  // overshadowee
+    CHECK(1 == overshadows[0].second); // overshadower
 }

src/regex_dfa/DFAMinimizer.cpp

@@ -101,9 +101,9 @@ DFAMinimizer::PartitionVec DFAMinimizer::split(const StateIdVec& S) const
         {
             DEBUG("split: {} on character '{}' into {} sets", to_string(S), (char) c, t_i.size());
             PartitionVec result;
-            for (const pair<int, StateIdVec>& t: t_i)
+            for (auto&& t: t_i)
             {
-                result.emplace_back(move(t.second));
+                result.emplace_back(std::move(t.second));
                 DEBUG(" partition {}: {}", t.first, t.second);
             }
             return result;
@@ -125,7 +125,7 @@ DFAMinimizer::PartitionVec DFAMinimizer::split(const StateIdVec& S) const
                 main.emplace_back(s);
 
             if (!main.empty())
-                result.emplace_back(move(main));
+                result.emplace_back(std::move(main));
         }
     }
 
@@ -253,9 +253,9 @@ DFA DFAMinimizer::constructFromPartitions(const PartitionVec& P) const
     for (const StateIdVec& p: P)
     {
         const StateId s = *p.begin();
-        for (const pair<Symbol, StateId>& transition: dfa_.stateTransitions(s))
+        for (pair<Symbol, StateId> const transition: dfa_.stateTransitions(s))
         {
-            const int t_i = partitionId(transition.second);
+            auto const t_i = partitionId(transition.second);
             DEBUG("map p{} --({})--> p{}", p_i, prettySymbol(transition.first), t_i);
             dfamin.setTransition(p_i, transition.first, t_i);
         }

src/regex_dfa/DFAMinimizer.h

@@ -35,18 +35,18 @@ class DFAMinimizer
     using PartitionVec = std::list<StateIdVec>;
 
     void constructPartitions();
-    StateIdVec nonAcceptStates() const;
-    bool containsInitialState(const StateIdVec& S) const;
-    bool isMultiInitialState(StateId s) const;
-    PartitionVec::iterator findGroup(StateId s);
-    int partitionId(StateId s) const;
-    PartitionVec split(const StateIdVec& S) const;
-    DFA constructFromPartitions(const PartitionVec& P) const;
-    std::optional<StateId> containsBacktrackState(const StateIdVec& Q) const;
+    [[nodiscard]] StateIdVec nonAcceptStates() const;
+    [[nodiscard]] bool containsInitialState(const StateIdVec& S) const;
+    [[nodiscard]] bool isMultiInitialState(StateId s) const;
+    [[nodiscard]] PartitionVec::iterator findGroup(StateId s);
+    [[nodiscard]] int partitionId(StateId s) const;
+    [[nodiscard]] PartitionVec split(const StateIdVec& S) const;
+    [[nodiscard]] DFA constructFromPartitions(const PartitionVec& P) const;
+    [[nodiscard]] std::optional<StateId> containsBacktrackState(const StateIdVec& Q) const;
 
     static void dumpGroups(const PartitionVec& T);
 
-    StateId targetStateId(StateId oldId) const
+    [[nodiscard]] StateId targetStateId(StateId oldId) const
     {
         auto i = targetStateIdMap_.find(oldId);
         assert(i != targetStateIdMap_.end());

src/regex_dfa/DotVisitor.h

@@ -17,7 +17,7 @@ namespace regex_dfa
 class DotVisitor
 {
   public:
-    virtual ~DotVisitor() {}
+    virtual ~DotVisitor() = default;
 
     virtual void start(StateId initialState) = 0;
     virtual void visitNode(StateId number, bool start, bool accept) = 0;

src/regex_dfa/DotWriter.cpp

@@ -65,7 +65,7 @@ void DotWriter::visitNode(StateId number, bool start, bool accept)
     }
 }
 
-void DotWriter::visitEdge(StateId from, StateId to, Symbol s)
+void DotWriter::visitEdge(StateId /*from*/, StateId to, Symbol s)
 {
     transitionGroups_[to].push_back(s);
 }

src/regex_dfa/DotWriter_test.cpp

@@ -9,12 +9,12 @@
 
 #include <sstream>
 
-#include <klex/util/testing.h>
+#include <catch2/catch.hpp>
 
 using namespace std;
 using namespace regex_dfa;
 
-TEST(regex_DotWriter, simple)
+TEST_CASE("regex_DotWriter.simple")
 {
     stringstream sstr;
     DotWriter dw(sstr, "n");
@@ -33,12 +33,11 @@ TEST(regex_DotWriter, simple)
     dw.endVisitEdge(1, 1);
     dw.end();
 
-    log(sstr.str());
-    ASSERT_TRUE(!sstr.str().empty());
+    REQUIRE(!sstr.str().empty());
     // just make sure it processes
 }
 
-TEST(regex_DotWriter, multidfa_simple)
+TEST_CASE("regex_DotWriter.multidfa_simple")
 {
     stringstream sstr;
     const MultiDFA::InitialStateMap mis { { "foo", 1 }, { "bar", 2 } };
@@ -63,7 +62,6 @@ TEST(regex_DotWriter, multidfa_simple)
 
     dw.end();
 
-    log(sstr.str());
-    ASSERT_TRUE(!sstr.str().empty());
+    REQUIRE(!sstr.str().empty());
     // just make sure it processes
 }