Migrate all parsers from Boost.Spirit Qi to X3#7432
Merged
DennisOSRM merged 6 commits intomasterfrom Apr 7, 2026
Merged
Conversation
This change modernizes the parsing infrastructure across the entire codebase by migrating from Boost.Spirit Qi (V2) to the newer X3 library. This brings several benefits: - **Improved compile times**: X3's header-only architecture with better template metaprogramming reduces compilation overhead - **Modern C++ patterns**: Replaces Phoenix bindings with C++ lambdas and grammar class hierarchies with namespace-based composition - **AppleClang compatibility**: Side-steps warnings from recent AppleClang versions that have tighter rules around casts and type conversions in Phoenix expressions Changes across 16 files: - Server API parameter parsers (route, table, match, trip, nearest, tile) - URL parser (eliminated qi_iter_pos dependency) - ISO 8601 duration parser - CSV file parser - Opening hours parser (~35 rules, most complex migration) - Conditional restrictions parser All existing tests pass (server-tests: 16/16, extractor-tests, updater-tests: 5/5). Technical notes: - API grammars use x3::with<params_tag> context pattern instead of inherited attributes - Typed intermediate rules prevent variant<T,T> from same-type alternatives - Removed BOOST_SPIRIT_USE_PHOENIX_V3 and BOOST_PHOENIX_STL_TUPLE_H_ CMake defines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes a broad migration of the project’s parsing infrastructure from Boost.Spirit Qi/Phoenix to Boost.Spirit X3, simplifying grammars and removing Phoenix/CMake workarounds.
Changes:
- Migrates multiple parsers (server API parameters, URL parsing, opening_hours, conditional_restrictions, CSV, ISO 8601 duration) from Qi to X3.
- Refactors server API parameter grammars to use X3 context (
x3::with<params_tag>) for threading the parameters struct. - Removes Phoenix-related dependency defines from CMake.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/opening_hours.cpp | Rewrites the opening_hours grammar using X3 rules/symbol tables and X3 actions. |
| src/util/conditional_restrictions.cpp | Replaces Qi grammar with X3 rules for conditional restriction parsing. |
| src/updater/csv_source.cpp | Switches CSV value/key parsing expressions from Qi parsers to X3 parsers. |
| include/updater/csv_file_parser.hpp | Refactors CSVFilesParser to accept X3 parsers and parse via an injected X3 parse function. |
| src/server/api/url_parser.cpp | Migrates URL parsing grammar to X3 and replaces iter_pos-based prefix tracking with a computed length. |
| src/server/api/parameters_parser.cpp | Updates parameter parsing to X3 context-based parsing and swaps grammar entrypoints to root rules. |
| include/server/api/trip_parameter_grammar.hpp | Rewrites Trip API query grammar as X3 rule composition and context writes. |
| include/server/api/tile_parameter_grammar.hpp | Replaces Tile grammar with an X3 rule definition for TileParameters. |
| include/server/api/table_parameter_grammar.hpp | Rewrites Table API query grammar using X3 symbols, typed rules, and context writes. |
| include/server/api/route_parameters_grammar.hpp | Rewrites Route API query grammar using X3 symbols, typed rules, and context writes. |
| include/server/api/nearest_parameter_grammar.hpp | Rewrites Nearest API query grammar as X3 rules and context writes. |
| include/server/api/match_parameter_grammar.hpp | Rewrites Match API query grammar as X3 rules and context writes. |
| include/server/api/base_parameters_grammar.hpp | Provides shared X3 primitives/options parsing with params_tag context threading. |
| include/extractor/raster_source.hpp | Removes now-unneeded Qi includes. |
| include/extractor/extraction_helper_functions.hpp | Migrates ISO 8601 duration parsing helpers from Qi to X3. |
| CMakeLists.txt | Removes Phoenix-related Boost.Spirit/Phoenix workaround defines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+401
to
+403
| inline const auto year_end_value = x3::rule<struct ye_tag, unsigned>{"year_end"} = | ||
| ('-' >> year_val >> x3::omit[-('/' >> x3::uint_)]) | | ||
| (x3::lit('+') >> x3::attr(static_cast<unsigned>(-1))); |
There was a problem hiding this comment.
year_end_value uses x3::attr(static_cast<unsigned>(-1)) as the '+' sentinel, and later converts it to int for Monthday. Converting an out-of-range unsigned to int is implementation-defined and can yield an unintended large year instead of the intended -1 sentinel. Prefer making year_end_value an int (or directly synthesize oh::Monthday(-1) for '+') so the sentinel is represented safely and portably.
Suggested change
| inline const auto year_end_value = x3::rule<struct ye_tag, unsigned>{"year_end"} = | |
| ('-' >> year_val >> x3::omit[-('/' >> x3::uint_)]) | | |
| (x3::lit('+') >> x3::attr(static_cast<unsigned>(-1))); | |
| inline const auto year_end_value = x3::rule<struct ye_tag, int>{"year_end"} = | |
| ('-' >> year_val >> x3::omit[-('/' >> x3::uint_)]) | | |
| (x3::lit('+') >> x3::attr(-1)); |
When using a rule as a parser within an alternative in X3, the attribute must be explicitly forwarded with a semantic action. The date_to rule was not propagating the Monthday attribute from date_from, causing date ranges to have empty 'to' fields. This fixes all 14 opening_hours test failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use C++17 concatenated namespace syntax instead of nested namespace declarations, as recommended by modernize-concat-nested-namespaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TheMarex
approved these changes
Mar 27, 2026
Member
TheMarex
left a comment
There was a problem hiding this comment.
Overall looks good to me code wise, but I don't think X3 is an improvement in readability over Qi to be honest. You mention a compile time benchmark, but what would be really interesting here is actually a query time benchmark. scripts/osrm-runner.js should make that easy enough.
Collaborator
Author
|
Ran benchmarks on today's germany-latest with a fixed set of 1000 random queries. The numbers between master and this branch are the same. The differences look like noise in the microsecond range per query. |
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.
This PR modernizes the parsing infrastructure across the entire codebase by migrating from Boost.Spirit Qi (V2) to the newer X3 library.
Benefits
Changes
16 files migrated across the entire parser stack:
Compile time benchmark
To verify improvements in compile time, the following compilation runs were timed (single core, release build). They show a 6% improvement in single core compilation time, which seems like a good win.
Testing
✅ All existing tests pass:
Technical Details
x3::with<params_tag>context pattern instead of inherited attributesvariant<T,T>from same-type alternativesBOOST_SPIRIT_USE_PHOENIX_V3andBOOST_PHOENIX_STL_TUPLE_H_CMake definesTotal: 1018 insertions, 1010 deletions