refactor: improve error handling and pgn parsing (#167)

Author: Disservin

README.md

@@ -81,6 +81,11 @@ The 3 other big chess libraries that I know of in C++ are:
 - Texel tuner for HCE engines
   <https://github.com/GediminasMasaitis/texel-tuner>
 
+### Exceptions
+
+This library might throw exceptions in some cases, for example when the input is invalid or things are not as expected.
+To disable exceptions, define `CHESS_NO_EXCEPTIONS` before including the header.
+
 ### Benchmarks
 
 Tested on Ryzen 9 5950X:
@@ -109,10 +114,13 @@ depth 6  time 2165  nodes 872323796    nps 402734901 fen 4rrb1/1kp3b1/1p1p4/pP1P
 depth 6  time 6382  nodes 2678022813   nps 419555508 fen 1rkr3b/1ppn3p/3pB1n1/6q1/R2P4/4N1P1/1P5P/2KRQ1B1 b Ddb - 0 14
 ```
 
-#### Exceptions
+### PGN Parser
 
-This library might throw exceptions in some cases, for example when the input is invalid or things are not as expected.
-To disable exceptions, define `CHESS_NO_EXCEPTIONS` before including the header.
+Ran with `lichess_db_standard_rated_2017-03.pgn` on a Ryzen 9 5950X with Samsung 980 SSD.
+
+| Benchmark | Time    | Throughput    |
+| :---:   | :---: | :---: |
+| ./build/example | 28.927s   | 413.281 MB/s   |
 
 #### PERFT Comparison
 
@@ -121,6 +129,7 @@ To disable exceptions, define `CHESS_NO_EXCEPTIONS` before including the header.
 `chess-library` had no movelist preallocation, same as the other libraries.
 
 ```
+
 chess-library:
 depth 6  time 539   nodes 119060324    nps 220482081 fen rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1
 depth 5  time 538   nodes 193690690    nps 359351929 fen r3k2r/p1ppqpb1/bn2pnp1/3PN3/1p2P3/2N2Q1p/PPPBBPPP/R3K2R w KQkq - 0 1
@@ -144,6 +153,7 @@ depth 6  time 404   nodes 11030083     nps 27234772  fen 8/2p5/3p4/KP5r/1R3p1k/8
 depth 5  time 388   nodes 15833292     nps 40702550  fen r3k2r/Pppp1ppp/1b3nbN/nP6/BBP1P3/q4N2/Pp1P2PP/R2Q1RK1 w kq - 0 1
 depth 5  time 2909  nodes 89941194     nps 30907626  fen rnbq1k1r/pp1Pbppp/2p5/8/2B5/8/PPP1NnPP/RNBQK2R w KQ - 1 8
 depth 5  time 3403  nodes 164075551    nps 48200808  fen r4rk1/1pp1qppp/p1np1n2/2b1p1B1/2B1P1b1/P1NP1N2/1PP1QPPP/R4RK1 w - - 0 11
+
 ```
 
 ### Development Setup

docs/pages/pgn-parsing.md

@@ -68,7 +68,7 @@ Create the parser and parse the games:
 
 ```cpp
 pgn::StreamParser parser(file_stream);
-parser.readGames(visitor);
+auto error = parser.readGames(visitor);
 ```
 
 #### Putting it together
@@ -112,7 +112,12 @@ int main() {
 
     MyVisitor visitor;
     pgn::StreamParser parser(file_stream);
-    parser.readGames(visitor);
+    auto error = parser.readGames(visitor);
+
+    if (error) {
+        std::cerr << "Error parsing PGN: " << error.message() << std::endl;
+        return -1;
+    }
 
     return 0;
 }

example/main.cpp

@@ -38,7 +38,12 @@ int main(int argc, char const* argv[]) {
     const auto t0 = std::chrono::high_resolution_clock::now();
 
     pgn::StreamParser parser(file_stream);
-    parser.readGames(*vis);
+    auto error = parser.readGames(*vis);
+
+    if (error) {
+        std::cerr << "Error: " << error.message() << "\n";
+        return 1;
+    }
 
     const auto t1 = std::chrono::high_resolution_clock::now();
 

include/chess.hpp

@@ -25,7 +25,7 @@ THIS FILE IS AUTO GENERATED DO NOT CHANGE MANUALLY.
 
 Source: https://github.com/Disservin/chess-library
 
-VERSION: 0.8.3
+VERSION: 0.8.4
 */
 
 #ifndef CHESS_HPP
@@ -3935,11 +3935,49 @@ class Visitor {
     bool skip_ = false;
 };
 
-enum class StreamParserError {
-    None,
-    InvalidHeaderMissingClosingBracket,
-    InvalidHeaderMissingClosingQuote,
-    NotEnoughData,
+class StreamParserError {
+   public:
+    enum Code {
+        None,
+        ExceededMaxStringLength,
+        InvalidHeaderMissingClosingBracket,
+        InvalidHeaderMissingClosingQuote,
+        NotEnoughData
+    };
+
+    StreamParserError() : code_(None) {}
+
+    StreamParserError(Code code) : code_(code) {}
+
+    Code code() const { return code_; }
+
+    bool hasError() const { return code_ != None; }
+
+    std::string message() const {
+        switch (code_) {
+            case None:
+                return "No error";
+            case InvalidHeaderMissingClosingBracket:
+                return "Invalid header: missing closing bracket";
+            case InvalidHeaderMissingClosingQuote:
+                return "Invalid header: missing closing quote";
+            case NotEnoughData:
+                return "Not enough data";
+            default:
+                assert(false);
+                return "Unknown error";
+        }
+    }
+
+    bool operator==(Code code) const { return code_ == code; }
+    bool operator!=(Code code) const { return code_ != code; }
+    bool operator==(const StreamParserError &other) const { return code_ == other.code_; }
+    bool operator!=(const StreamParserError &other) const { return code_ != other.code_; }
+
+    operator bool() const { return code_ != None; }
+
+   private:
+    Code code_;
 };
 
 template <std::size_t BUFFER_SIZE =
@@ -3981,6 +4019,10 @@ class StreamParser {
 
             } else if (in_body) {
                 processBody();
+
+                if (error != StreamParserError::None) {
+                    return error;
+                }
             }
 
             if (!dont_advance_after_body) stream_buffer.advance();
@@ -3997,17 +4039,31 @@ class StreamParser {
    private:
     class LineBuffer {
        public:
-        LineBuffer() { buffer_.reserve(BUFFER_SIZE); }
-        bool empty() const noexcept { return buffer_.empty(); }
+        bool empty() const noexcept { return index_ == 0; }
 
-        void clear() noexcept { buffer_.clear(); }
+        void clear() noexcept { index_ = 0; }
 
-        std::string_view get() const noexcept { return std::string_view(buffer_.data(), buffer_.size()); }
+        std::string_view get() const noexcept { return std::string_view(buffer_.data(), index_); }
 
-        void operator+=(char c) { buffer_.push_back(c); }
+        bool add(char c) {
+            if (index_ >= N) {
+                return false;
+            }
+
+            buffer_[index_] = c;
+
+            ++index_;
+
+            return true;
+        }
 
        private:
-        std::vector<char> buffer_;
+        // PGN lines are limited to 255 characters
+        static constexpr int N = 255;
+
+        std::array<char, N> buffer_ = {};
+
+        std::size_t index_ = 0;
     };
 
     class StreamBuffer {
@@ -4125,7 +4181,7 @@ class StreamParser {
 
     void callVisitorMoveFunction() {
         if (!move.empty()) {
-            if (!visitor->skip()) visitor->move(move.get(), comment.get());
+            if (!visitor->skip()) visitor->move(move.get(), comment);
 
             move.clear();
             comment.clear();
@@ -4145,7 +4201,11 @@ class StreamParser {
                         if (is_space(*k)) {
                             break;
                         } else {
-                            header.first += *k;
+                            if (!header.first.add(*k)) {
+                                error = StreamParserError::ExceededMaxStringLength;
+                                return;
+                            }
+
                             stream_buffer.advance();
                         }
                     }
@@ -4179,7 +4239,12 @@ class StreamParser {
                             return;
                         } else {
                             backslash = false;
-                            header.second += *k;
+
+                            if (!header.second.add(*k)) {
+                                error = StreamParserError::ExceededMaxStringLength;
+                                return;
+                            }
+
                             stream_buffer.advance();
                         }
                     }
@@ -4253,7 +4318,7 @@ class StreamParser {
 
                 // the game has no moves, but a comment followed by a game termination
                 if (!visitor->skip()) {
-                    visitor->move("", comment.get());
+                    visitor->move("", comment);
 
                     comment.clear();
                 }
@@ -4393,8 +4458,10 @@ class StreamParser {
                 }
                 // castling
                 else {
-                    move += '0';
-                    move += '-';
+                    if (!move.add('0') || !move.add('-')) {
+                        error = StreamParserError::ExceededMaxStringLength;
+                        return;
+                    }
 
                     if (parseMove()) {
                         stream_buffer.advance();
@@ -4412,7 +4479,10 @@ class StreamParser {
                 break;
             }
 
-            move += *c;
+            if (!move.add(*c)) {
+                error = StreamParserError::ExceededMaxStringLength;
+                return true;
+            }
 
             stream_buffer.advance();
         }
@@ -4526,8 +4596,8 @@ class StreamParser {
     // one time allocations
     std::pair<LineBuffer, LineBuffer> header = {LineBuffer{}, LineBuffer{}};
 
-    LineBuffer move    = {};
-    LineBuffer comment = {};
+    LineBuffer move     = {};
+    std::string comment = {};
 
     // State
 

src/include.hpp

@@ -25,7 +25,7 @@ THIS FILE IS AUTO GENERATED DO NOT CHANGE MANUALLY.
 
 Source: https://github.com/Disservin/chess-library
 
-VERSION: 0.8.3
+VERSION: 0.8.4
 */
 
 #ifndef CHESS_HPP