Skip to content

perform span attributions in serde *Access impls - #229

Open
sunshowers wants to merge 1 commit into
jj-stack/delete-internalerror-unknown-okrnyrzqfrom
jj-stack/perform-span-attributions-in-serde-access-impls-sotzxrvr
Open

perform span attributions in serde *Access impls#229
sunshowers wants to merge 1 commit into
jj-stack/delete-internalerror-unknown-okrnyrzqfrom
jj-stack/perform-span-attributions-in-serde-access-impls-sotzxrvr

Conversation

@sunshowers

@sunshowers sunshowers commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This lets us always provide some kind of span in diagnostics.

We rely on the property that in the middle of deserializing a value, serde's upcalls back to us always go through the Access traits. This means that as long as we cover the methods:

  • next_key_seed (covered by next_value_seed)
  • next_value_seed
  • next_element_seed
  • variant_seed
  • newtype_variant_seed
  • tuple_variant
  • struct_variant

as well as the top level, we can always convert a Unspanned error into the corresponding Spanned error with the tightest available span.

This fixes a number of panics and incorrect spans, as shown by the UI tests.

This lets us always provide _some_ kind of span in diagnostics.

We rely on the property that in the middle of deserializing a value, serde hands control back to us only through the `Access` traits. This means that as long as we cover the methods:

* `next_key_seed` (covered by `next_value_seed`)
* `next_value_seed`
* `next_element_seed`
* `variant_seed`
* `newtype_variant_seed`
* `tuple_variant`
* `struct_variant`

as well as the top level, we can always convert a `NoData` error into the corresponding `Normal` error with the tightest available span.

This fixes a number of panics and incorrect spans, as shown by the UI tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant