Skip to content

fix: explicit slice coercion in MotionPhoto attribute comparison#58

Merged
mindeng merged 1 commit into
mindeng:mainfrom
Enduriel:slice-coercion
May 28, 2026
Merged

fix: explicit slice coercion in MotionPhoto attribute comparison#58
mindeng merged 1 commit into
mindeng:mainfrom
Enduriel:slice-coercion

Conversation

@Enduriel
Copy link
Copy Markdown
Contributor

Problem

cargo clippy/cargo build fails in src/jpeg.rs when rkyv is present
elsewhere in the dependency graph:

error[E0277]: can't compare `Option<&[u8]>` with `Option<&[u8; 11]>`
error[E0277]: can't compare `Option<&[u8]>` with `Option<&[u8; 9]>`

extract_attr_value() returns Option<&[u8]>, while the literals Some(b"MotionPhoto") and Some(b"video/mp4") are Option<&[u8; N]>. The comparison normally works because Rust coerces
the array reference to a slice — but that coercion only fires when the target type is unambiguous.

rkyv's blanket impl

impl<T: PartialEq<U>, U> PartialEq<ArchivedOption<T>> for Option<U>

adds another PartialEq candidate, so coercion no longer applies and the literal stays &[u8; N], for which no matching impl exists.

Fix

Coerce the literals to slices explicitly:

extract_attr_value(tag, b"Item:Semantic") == Some(&b"MotionPhoto"[..])
extract_attr_value(tag, b"Item:Mime")     == Some(&b"video/mp4"[..])

This makes the comparison type-correct on its own, independent of any other crate in the graph.

Notes

  • No behavior change; purely a type-annotation fix.
  • Reproducible by adding rkyv as a dependency to a crate that also uses this one.

Comparing `extract_attr_value()` (which returns `Option<&[u8]>`) against
byte-string literals relied on automatic unsizing coercion from
`&[u8; N]` to `&[u8]`. This coercion fails to apply when a crate like
rkyv is present in the dependency graph, since its blanket
`PartialEq<ArchivedOption<T>>` impl introduces an additional trait
resolution candidate and the coercion target is no longer unambiguous.

Force the slice type explicitly so the comparison compiles regardless of
other crates in the graph.
Copy link
Copy Markdown
Owner

@mindeng mindeng left a comment

Choose a reason for hiding this comment

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

LGTM. Clean fix — the explicit slice coercion resolves the rkyv blanket impl ambiguity without any behavior change. Tests (366 passed) and clippy are clean.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.84%. Comparing base (12de899) to head (b428e9b).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/jpeg.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
- Coverage   90.33%   89.84%   -0.49%     
==========================================
  Files          38       38              
  Lines        8212     8399     +187     
==========================================
+ Hits         7418     7546     +128     
- Misses        794      853      +59     
Flag Coverage Δ
nom-exif 89.84% <0.00%> (-0.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mindeng mindeng merged commit dad49e0 into mindeng:main May 28, 2026
3 of 5 checks passed
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.

2 participants