Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1
X3's binary parser is not well-designed. X3 has only the primitives for parsing a single chunk, e.g. byte/word/dword. This (massive) lack of feature essentially requires users to write something like
x3::repeat(n)[x3::byte_]. It is obvious that such pattern suffers from a huge performance loss because X3's composed parser parses each bytes of the input by repeatedly incrementing the iterator.The whole point of using binary parsers is performance and optimization. If we keep providing this poorly designed feature, there would be more users relying on this inefficient pattern in the future.
In my 15+ years of experience using Spirit.Qi and X3, I've never used Spirit's binary parsers because of its lack of fundamental features. I always used a fully featured binary parser framework, such as MessagePack, Protocol Buffers, and Apache Thrift in real-world applications. I believe this is objectively the correct choice for serious business projects. Text parsers and binary parsers are completely different domains and we should accept the fact that a single parser combinator library just can't support both of them. X3 and even X4's interface is not suited for binary parsers.
GitHub full-text search shows very limited real-world use case: https://github.com/search?q=%22x3%3A%3Abyte_%22+language%3AC%2B%2B+-path%3Ax3%2Fbinary+-org%3Athink-cell+-path%3Amsgpack+-path%3Amqtt+-path%3Aivp&type=code
Additionally, we aim to reduce the Boost dependencies to zero in #1, and we don't like the Boost.Endian dependency introduced by the binary parser implementation. We can't offer human resource to port Boost.Endian in a constexpr-friendly and standard-compliant manner right now.