Skip to content

ParseWrapper and TokenStreamWrapper can't work with internal buffering - #231

Open
sunshowers wants to merge 1 commit into
jj-stack/allow-constructing-and-cloning-parsewrapper-and-tokenstreamwrapper-nrtluwxufrom
jj-stack/parsewrapper-and-tokenstreamwrapper-don-t-work-with-internal-buffering-nznpslrr
Open

ParseWrapper and TokenStreamWrapper can't work with internal buffering#231
sunshowers wants to merge 1 commit into
jj-stack/allow-constructing-and-cloning-parsewrapper-and-tokenstreamwrapper-nrtluwxufrom
jj-stack/parsewrapper-and-tokenstreamwrapper-don-t-work-with-internal-buffering-nznpslrr

Conversation

@sunshowers

@sunshowers sunshowers commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

ParseWrapper and TokenStreamWrapper receive their tokens through a side channel that only works while our deserializer is in charge. This doesn't really work with scenarios in which serde performs internal buffering -- it calls deserialize_any to buffer the value as a Content first, and only later hands that Content to the wrapper's Deserialize impl. Our deserialize_bytes is never called, so the side channel never fires and the wrapper sees a plain string or number.

In general, this can't be fixed. Spans can't be serialized, so they can't be roundtripped through Content. (We could try doing some kind of after-the-kind fixup but that seems dubious.) The alternative is to gracefully degrade by using Span::call_site(), but I'd rather put the burden on macro authors to fix their types instead.

Currently, any attempt to use these wrappers in such a scenario results in the somewhat unhelpful error message:

invalid type: string "foo", expected TokenStream

Improve this error message to indicate that the macro needs to be fixed, and document the limitation on both wrappers. This error reaches the user as a spanned diagnostic (rather than a panic) due to the span attribution in the *Access impls done earlier in this series.

Note that this error message isn't seen with #[serde(untagged)] because it swallows each variant's error. But the underlying principle remains the same.

`ParseWrapper` and `TokenStreamWrapper` receive their tokens through a side channel that only works while our deserializer is in charge. This doesn't really work with scenarios in which serde performs internal buffering. Any attempt to use these wrappers in such a scenario results in the somewhat unhelpful error message:

    invalid type: string "foo", expected TokenStream

Improve this error message to indicate that the macro needs to be fixed.

A different option is to gracefully degrade with internal buffering, but I'd rather put the burden on macro authors to improve their diagnostics.

Note that this error message isn't seen with `#[serde(untagged)]` because it swallows each variant's error. But the underlying principle remains the same.
@sunshowers sunshowers changed the title ParseWrapper and TokenStreamWrapper don't work with internal buffering ParseWrapper and TokenStreamWrapper can't work with internal buffering Sep 2, 2026
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