Skip to content

xpath: Replace nom with hand-written parsing rules#39977

Merged
simonwuelker merged 3 commits intoservo:mainfrom
simonwuelker:xpath-parser
Oct 25, 2025
Merged

xpath: Replace nom with hand-written parsing rules#39977
simonwuelker merged 3 commits intoservo:mainfrom
simonwuelker:xpath-parser

Conversation

@simonwuelker
Copy link
Copy Markdown
Contributor

@simonwuelker simonwuelker commented Oct 18, 2025

The new parser is more verbose, but also more correct and easier to reason about. Apologies for the size of the change, I don't think there's an alternative to swapping out the entire parser at once.

Testing: Covered by existing tests, new tests also start to pass
Fixes #38552
Fixes #38553
Fixes #39596
Closes #39602
Part of #34527

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 18, 2025
@simonwuelker simonwuelker force-pushed the xpath-parser branch 3 times, most recently from 0eec400 to cce86fa Compare October 18, 2025 10:10
@jschwe jschwe added the T-ohos Do a try run on OpenHarmony label Oct 18, 2025
@github-actions github-actions bot removed the T-ohos Do a try run on OpenHarmony label Oct 18, 2025
@github-actions
Copy link
Copy Markdown

🔨 Triggering try run (#18617320802) for OpenHarmony

@github-actions
Copy link
Copy Markdown

🐰 Bencher Report

Branch39977/PR
TestbedHUAWEI Mate 60 Pro

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results
BenchmarkDataMeasure (units) x 1e3Latencymilliseconds (ms)MemoryBytesscoreMeasure (units)
release/E2E/file:///parse_from_string.html/📈 view plot
⚠️ NO THRESHOLD
1.95 units x 1e3
release/E2E/https://www.google.com/JS/gc-heap/admin📈 view plot
⚠️ NO THRESHOLD
26,752.00
release/E2E/https://www.google.com/JS/gc-heap/decommitted📈 view plot
⚠️ NO THRESHOLD
409,600.00
release/E2E/https://www.google.com/JS/gc-heap/unused📈 view plot
⚠️ NO THRESHOLD
139,832.00
release/E2E/https://www.google.com/JS/gc-heap/used📈 view plot
⚠️ NO THRESHOLD
472,392.00
release/E2E/https://www.google.com/JS/malloc-heap📈 view plot
⚠️ NO THRESHOLD
5,051,339.00
release/E2E/https://www.google.com/JS/non-heap📈 view plot
⚠️ NO THRESHOLD
262,144.00
release/E2E/https://www.google.com/LayoutThread/box-tree📈 view plot
⚠️ NO THRESHOLD
107,632.00
release/E2E/https://www.google.com/LayoutThread/display-list📈 view plot
⚠️ NO THRESHOLD
0.00
release/E2E/https://www.google.com/LayoutThread/font-context📈 view plot
⚠️ NO THRESHOLD
8,616.00
release/E2E/https://www.google.com/LayoutThread/fragment-tree📈 view plot
⚠️ NO THRESHOLD
112.00
release/E2E/https://www.google.com/LayoutThread/stacking-context-tree📈 view plot
⚠️ NO THRESHOLD
14,080.00
release/E2E/https://www.google.com/LayoutThread/stylist📈 view plot
⚠️ NO THRESHOLD
5,104.00
release/E2E/https://www.google.com/Load📈 view plot
⚠️ NO THRESHOLD
567.32 ms
release/E2E/https://www.google.com/Resident📈 view plot
⚠️ NO THRESHOLD
182,036,070.00
release/E2E/https://www.google.com/image-cache📈 view plot
⚠️ NO THRESHOLD
35,760.00
release/E2E/https://www.google.com/resident-smaps📈 view plot
⚠️ NO THRESHOLD
184,684,544.00
release/E2E/https://www.servo.org/Load📈 view plot
⚠️ NO THRESHOLD
827.92 ms
release/E2E/https://www.servo.org/Resident📈 view plot
⚠️ NO THRESHOLD
269,096,550.00
release/E2E/https://www.servo.org/resident-smaps📈 view plot
⚠️ NO THRESHOLD
270,905,344.00
release/Speedometer/Charts-observable-plot📈 view plot
⚠️ NO THRESHOLD
764.69 ms
release/Speedometer/Charts-observable-plot/Dotted📈 view plot
⚠️ NO THRESHOLD
94.99 ms
release/Speedometer/Charts-observable-plot/Dotted/Async📈 view plot
⚠️ NO THRESHOLD
10.16 ms
release/Speedometer/Charts-observable-plot/Dotted/Sync📈 view plot
⚠️ NO THRESHOLD
84.84 ms
release/Speedometer/Charts-observable-plot/Stacked by 20📈 view plot
⚠️ NO THRESHOLD
370.65 ms
release/Speedometer/Charts-observable-plot/Stacked by 20/Async📈 view plot
⚠️ NO THRESHOLD
17.11 ms
release/Speedometer/Charts-observable-plot/Stacked by 20/Sync📈 view plot
⚠️ NO THRESHOLD
353.55 ms
release/Speedometer/Charts-observable-plot/Stacked by 6📈 view plot
⚠️ NO THRESHOLD
299.05 ms
release/Speedometer/Charts-observable-plot/Stacked by 6/Async📈 view plot
⚠️ NO THRESHOLD
9.44 ms
release/Speedometer/Charts-observable-plot/Stacked by 6/Sync📈 view plot
⚠️ NO THRESHOLD
289.60 ms
release/Speedometer/Geomean📈 view plot
⚠️ NO THRESHOLD
679.05 ms
release/Speedometer/Iteration-0-Total📈 view plot
⚠️ NO THRESHOLD
863.10 ms
release/Speedometer/Iteration-1-Total📈 view plot
⚠️ NO THRESHOLD
844.34 ms
release/Speedometer/Iteration-2-Total📈 view plot
⚠️ NO THRESHOLD
845.15 ms
release/Speedometer/Iteration-3-Total📈 view plot
⚠️ NO THRESHOLD
850.32 ms
release/Speedometer/Iteration-4-Total📈 view plot
⚠️ NO THRESHOLD
851.62 ms
release/Speedometer/Iteration-5-Total📈 view plot
⚠️ NO THRESHOLD
986.19 ms
release/Speedometer/Iteration-6-Total📈 view plot
⚠️ NO THRESHOLD
843.81 ms
release/Speedometer/Iteration-7-Total📈 view plot
⚠️ NO THRESHOLD
1,100.45 ms
release/Speedometer/Iteration-8-Total📈 view plot
⚠️ NO THRESHOLD
1,117.74 ms
release/Speedometer/Iteration-9-Total📈 view plot
⚠️ NO THRESHOLD
1,132.62 ms
release/Speedometer/Score📈 view plot
⚠️ NO THRESHOLD
1.49 units
release/Speedometer/TodoMVC-Angular📈 view plot
⚠️ NO THRESHOLD
942.21 ms
release/Speedometer/TodoMVC-Angular/Adding100Items📈 view plot
⚠️ NO THRESHOLD
448.19 ms
release/Speedometer/TodoMVC-Angular/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
29.50 ms
release/Speedometer/TodoMVC-Angular/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
418.70 ms
release/Speedometer/TodoMVC-Angular/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
305.73 ms
release/Speedometer/TodoMVC-Angular/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
36.88 ms
release/Speedometer/TodoMVC-Angular/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
268.85 ms
release/Speedometer/TodoMVC-Angular/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
188.29 ms
release/Speedometer/TodoMVC-Angular/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
7.12 ms
release/Speedometer/TodoMVC-Angular/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
181.17 ms
release/Speedometer/TodoMVC-JavaScript-ES5📈 view plot
⚠️ NO THRESHOLD
1,494.28 ms
release/Speedometer/TodoMVC-JavaScript-ES5/Adding100Items📈 view plot
⚠️ NO THRESHOLD
1,218.99 ms
release/Speedometer/TodoMVC-JavaScript-ES5/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
50.81 ms
release/Speedometer/TodoMVC-JavaScript-ES5/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
1,168.18 ms
release/Speedometer/TodoMVC-JavaScript-ES5/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
171.07 ms
release/Speedometer/TodoMVC-JavaScript-ES5/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
33.12 ms
release/Speedometer/TodoMVC-JavaScript-ES5/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
137.95 ms
release/Speedometer/TodoMVC-JavaScript-ES5/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
104.22 ms
release/Speedometer/TodoMVC-JavaScript-ES5/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
7.53 ms
release/Speedometer/TodoMVC-JavaScript-ES5/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
96.68 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack📈 view plot
⚠️ NO THRESHOLD
2,152.54 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/Adding100Items📈 view plot
⚠️ NO THRESHOLD
1,747.98 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
36.08 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
1,711.90 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
251.37 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
36.22 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
215.15 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
153.20 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
7.88 ms
release/Speedometer/TodoMVC-JavaScript-ES6-Webpack/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
145.32 ms
release/Speedometer/TodoMVC-Preact📈 view plot
⚠️ NO THRESHOLD
159.67 ms
release/Speedometer/TodoMVC-Preact/Adding100Items📈 view plot
⚠️ NO THRESHOLD
80.50 ms
release/Speedometer/TodoMVC-Preact/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
72.30 ms
release/Speedometer/TodoMVC-Preact/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
8.20 ms
release/Speedometer/TodoMVC-Preact/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
62.77 ms
release/Speedometer/TodoMVC-Preact/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
49.99 ms
release/Speedometer/TodoMVC-Preact/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
12.78 ms
release/Speedometer/TodoMVC-Preact/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
16.41 ms
release/Speedometer/TodoMVC-Preact/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
10.10 ms
release/Speedometer/TodoMVC-Preact/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
6.31 ms
release/Speedometer/TodoMVC-React📈 view plot
⚠️ NO THRESHOLD
853.15 ms
release/Speedometer/TodoMVC-React-Redux📈 view plot
⚠️ NO THRESHOLD
1,041.43 ms
release/Speedometer/TodoMVC-React-Redux/Adding100Items📈 view plot
⚠️ NO THRESHOLD
338.70 ms
release/Speedometer/TodoMVC-React-Redux/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
33.99 ms
release/Speedometer/TodoMVC-React-Redux/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
304.71 ms
release/Speedometer/TodoMVC-React-Redux/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
456.30 ms
release/Speedometer/TodoMVC-React-Redux/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
35.10 ms
release/Speedometer/TodoMVC-React-Redux/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
421.20 ms
release/Speedometer/TodoMVC-React-Redux/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
246.42 ms
release/Speedometer/TodoMVC-React-Redux/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
8.59 ms
release/Speedometer/TodoMVC-React-Redux/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
237.83 ms
release/Speedometer/TodoMVC-React/Adding100Items📈 view plot
⚠️ NO THRESHOLD
301.93 ms
release/Speedometer/TodoMVC-React/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
38.31 ms
release/Speedometer/TodoMVC-React/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
263.63 ms
release/Speedometer/TodoMVC-React/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
353.56 ms
release/Speedometer/TodoMVC-React/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
37.61 ms
release/Speedometer/TodoMVC-React/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
315.96 ms
release/Speedometer/TodoMVC-React/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
197.65 ms
release/Speedometer/TodoMVC-React/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
7.97 ms
release/Speedometer/TodoMVC-React/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
189.69 ms
release/Speedometer/TodoMVC-Svelte📈 view plot
⚠️ NO THRESHOLD
140.29 ms
release/Speedometer/TodoMVC-Svelte/Adding100Items📈 view plot
⚠️ NO THRESHOLD
77.82 ms
release/Speedometer/TodoMVC-Svelte/Adding100Items/Async📈 view plot
⚠️ NO THRESHOLD
59.17 ms
release/Speedometer/TodoMVC-Svelte/Adding100Items/Sync📈 view plot
⚠️ NO THRESHOLD
18.64 ms
release/Speedometer/TodoMVC-Svelte/CompletingAllItems📈 view plot
⚠️ NO THRESHOLD
46.74 ms
release/Speedometer/TodoMVC-Svelte/CompletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
37.69 ms
release/Speedometer/TodoMVC-Svelte/CompletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
9.05 ms
release/Speedometer/TodoMVC-Svelte/DeletingAllItems📈 view plot
⚠️ NO THRESHOLD
15.73 ms
release/Speedometer/TodoMVC-Svelte/DeletingAllItems/Async📈 view plot
⚠️ NO THRESHOLD
10.99 ms
release/Speedometer/TodoMVC-Svelte/DeletingAllItems/Sync📈 view plot
⚠️ NO THRESHOLD
4.74 ms
🐰 View full continuous benchmarking report in Bencher

@github-actions
Copy link
Copy Markdown

✨ Try run (#18617320802) succeeded.

github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2025
…40103)

This change was split of from #39977
to make that PR easier to review.

The `Debug` impl for the error is not very useful yet, but
#39977 swaps out the error type
itself, so there's no point in improving it here.

Testing: I've manually tested that the error message appears. I don't
think we have tests for error messages.
Part of #34527

Signed-off-by: Simon Wülker <[email protected]>
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice rewrite!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 24, 2025
@simonwuelker
Copy link
Copy Markdown
Contributor Author

Thanks for the review!

@servo-highfive servo-highfive added S-needs-rebase There are merge conflict errors. S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Oct 24, 2025
@servo-highfive servo-highfive removed the S-needs-rebase There are merge conflict errors. label Oct 24, 2025
@simonwuelker simonwuelker added this pull request to the merge queue Oct 24, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2025
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 24, 2025
@servo-highfive servo-highfive added the S-tests-failed The changes caused existing tests to fail. label Oct 24, 2025
@simonwuelker simonwuelker added this pull request to the merge queue Oct 24, 2025
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 24, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 24, 2025
@servo-highfive servo-highfive removed the S-tests-failed The changes caused existing tests to fail. label Oct 25, 2025
Signed-off-by: Simon Wülker <[email protected]>
Signed-off-by: Simon Wülker <[email protected]>
@simonwuelker simonwuelker added this pull request to the merge queue Oct 25, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 25, 2025
Merged via the queue into servo:main with commit 7485978 Oct 25, 2025
30 checks passed
@simonwuelker simonwuelker deleted the xpath-parser branch October 25, 2025 03:54
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 25, 2025
@yezhizhen
Copy link
Copy Markdown
Member

Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

6 participants