Add diagnostic helps for TiffValue proxy implementations#321
Merged
Conversation
We have a fundamental choice here: We could add more proxy implementations for `Vec` and `Box` to follow `&'_ T`, for some types that indirections is clear. Vec and arrays are also both very natural ways of expressing a repeated value. However, passing an allocated value to the interface forces us to drop it inside. We might be able to utilize the allocation for an in-place byteswap in a future increment but not with the current design. So, overall, I think that inherently `Drop`-y types should be avoided in these implementations. Instead, we can use the amazing diagnostic tools in the compiler to just help the users navigate the type system. We can still provide the impl for arrays. For context, see: https://users.rust-lang.org/t/what-type-is-u16/137395 Now they see: | 38 | .write_tag(tiff::tags::Tag::GeoKeyDirectoryTag, &geokey); | ^^^^^^ the trait `TiffValue` is not implemented for `&Vec<u16>` | = note: the trait is implemented for primitive types (`u8`, `i16`, `f32`, etc.) = note: the trait is implemented for shared references to values = note: the trait is implemented for slices, pass them by reference (e.g. `&[u8]`) = note: the trait is implemented for arrays if it is implemented for a slice = note: values in a `Vec` or `Box` should be dereferenced, e.g. `&vec[..]` = help: the following other types implement trait `TiffValue`: Previously the compiler would both strip the `&` in its first message (due to our forwarding impl) and not list any way of getting a reference-to-slice.
This gives us the diagnostic items. As a side effect we will have `expect` for lints from 1.81 to apply in later PRs. It's time I think.
197g
force-pushed
the
forward-tiff-value-for-vec-and-array
branch
from
6409c85 to
27bfcf5
Compare
197g
changed the title
197g
changed the title
fintelia
approved these changes
Jan 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We have a fundamental choice here: We could add more proxy implementations for
VecandBoxto follow&'_ T, for some types that indirections is clear. Vec and arrays are also both very natural ways of expressing a repeated value. However, passing an allocated value to the interface forces us to drop it inside. We might be able to utilize the allocation for an in-place byteswap in a future increment but not with the current design.So, overall, I think that inherently
Drop-y types should be avoided in these implementations. Instead, we can use the amazing diagnostic tools in the compiler to just help the users navigate the type system. We can still provide the impl for arrays.For context, see: https://users.rust-lang.org/t/what-type-is-u16/137395
Now they see:
Previously the compiler would both strip the
&in its first message (due to our forwarding impl) and not list any way of getting a reference-to-slice nor make it obvious that this would resolve the situation. (There is a small 'try to dereference' but that is among a noise of type-system squabble and doesn't actually explain how this would affect the type and help).