Consider making the fields of `Spanned` public

[Kixunil]

I think Spanned should be just a POD with all its fields public or at least the value field should be. Not being public makes matching extra annoying.

My use case: I have a toml that allows setting one of two fields only if another is either not set or set to false (they are contradictory) so I deserialize both fields and then I match on the tuple of them emitting an error if an invalid combination is present (and convert to a proper enum a valid one is). I'd love to make the error message look as nice as Rust error messages - having "foo specified here" and "bar specified here" messages pointing at the places where they are specified. I need span for this so I'm refactoring the match.

[epage]

The only relevant issue I found in the old toml-rs repo was https://github.com/toml-rs/toml-rs/issues/436

I generally lean towards types being all pub or all private. My main concern over exposing Spanneds fields is if we'd want to use an actual range type so we can clarify the bounds via the type system.

I wonder what the rest of the ecosystem does.

• syn is its own thing
• ariadne uses range
• combine uses start and end fields

[Kixunil]

To add to your list codespan-reporting uses range. Which I personally don't like much - semantically encoding "this is a span not some arbitrary range" is nice.

In my own crate I wrote struct Span { start: usize, end: usize } and conversions between toml and codespan-reporting types.

[dtolnay]

I opened #664 to try to make progress by exposing a way to construct Spanned, even if the fields aren't public.

I would be happy for us to consider Spanned::new(lo..hi, value) or Spanned::new(value).with_range(lo..hi), the latter not precluding construction with a better span type in the future if we settle on one.

[epage]

for that use case, would #634 be more relevant? We have a prototype at #635 that is just waiting on feedback

[dtolnay]

As I understand it, #635 adds a mechanism by which authors of a Deserializer impl can look at a particular deserialize_struct call and tell whether Spanned<T> is the thing being deserialized, and if so, feed it with the right range for Spanned<T> to get deserialized successfully.

That is not the missing piece in https://github.com/dtolnay/serde-untagged/issues/5. Serde-untagged implements a particularly clever Visitor, which the caller uses for implementing Deserialize. There is no Deserializer impl at play in that crate.

The only way that I see SpannedDeserializer could come into play is if serde-untagged did either of these 2 things:

1. If it deserialized the entire input into Spanned<Buffer> first. Then allowed the user's functions to deserialize Spanned<T> from serde-untagged's buffer: .map(|map| map.deserialize::<Spanned<TomlDetailedDependency>>()). For this it would need SpannedDeserializer in implementing the Buffer type's Deserializer impl.

2. If it had a Deserializer adapter that wrapped the incoming Deserializer provided by the caller to .deserialize(deserializer), and intercepted the deserialize_struct call there without buffering, and behaved differently based on whether it is or isn't Spanned that the caller wants to deserialize.

But:

1. The point of that crate is to have no buffering, due to https://github.com/serde-rs/serde/issues/1183.

2. This would not be useful because without buffering, it would only be able to support at most 1 Spanned case in your untagged enum. You would not be able to have 2 or more variants which are both Spanned, which is what the issue in serde-untagged is in need of, I think.