fix: remove xpressive dependency - #566
Conversation
|
Boost dependency footprint vs Header-inclusion weights (graph files pulling each direct dependency in):
Transitive Boost modules: 68 → 67 (-1)
|
|
Compiler-warning counts vs
|
16db299 to
dedb6c5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
dedb6c5 to
2ad1055
Compare
2ad1055 to
30d31bf
Compare
|
I'm not convinced about the trade-off here. We already depend on Boost.Regex, so why not just replace Xpressive with Regex? Or the other way around? |
|
@jeremy-murphy IMO doubling down with
IMO there are several reasons not to go the Boost.Regex route here: Boost.Regex would break the writer's header-only-ness. We don't actually need a regex engine here. The pattern is:
Naive
Overall I do think dropping dependencies is urgent, we're bleeding users over our transitive dependency weight, but happy to chat about it. |
jeremy-murphy
left a comment
There was a problem hiding this comment.
Just quibbling about nothing important. :) (And I might be wrong, I'm tired.)
| if (dot_id_is_digit(s[i])) | ||
| while (i < s.size() && dot_id_is_digit(s[i])) | ||
| ++i; |
There was a problem hiding this comment.
Looks like it tests i and s[i] unnecessarily at the start?
| if (dot_id_is_digit(s[i])) | |
| while (i < s.size() && dot_id_is_digit(s[i])) | |
| ++i; | |
| while (dot_id_is_digit(s[i]) && ++i < s.size()); |
#496
Before submitting
developbranch.Type of change
Does this PR introduce a breaking change?
What this PR does
Motivation
Too many warnings come from xpressive and its dependencies proto and fusion.
Context:
DOT is the plain-text graph language Graphviz uses, the format that
write_graphvizproduces andread_graphvizparses (for example,digraph { a -> b [label="hello world"] }). In that text, every name and value is a token the grammar calls an ID: node names likea, attribute names likelabel, and attribute values like"hello world". The grammar allows an ID in four forms:foo,node_1,_x).42,-3.14,.5."hello world","has \"quotes\"". This form can hold any characters, with\"for an embedded quote.<...>), not relevant here.The first two forms may be written bare. Anything that is not a valid bare name or numeral (it contains a space, punctuation, or a quote, starts with a digit and then has letters like
9lives, has two dots like1.2.3, or is empty) must use the quoted form, or the parser breaks. Deciding which case a value falls into is exactly the job ofescape_dot_string.Testing
Naive -O2 benchmark on a small corpus (https://godbolt.org/z/4MP8T95h8):
escape_dot_string runs once per written value (node id + every vertex/edge/graph attribute), so this constant factor just multiplies with graph size: on a large, attribute-heavy graph it's the same 50–90× penalty applied many times over. I haven't measured that end-to-end, but it can't be a good sign.
Checklist
b2in thetest/directory).