fix(linalg): point length-contract panics at the distance function - #8864
Open
LuciferYang wants to merge 1 commit into
Open
fix(linalg): point length-contract panics at the distance function#8864LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
LuciferYang
force-pushed
the
fix/track-caller-length-asserts
branch
from
August 29, 2026 15:18
f5ed9a7 to
17d1f73
Compare
LuciferYang
force-pushed
the
fix/track-caller-length-asserts
branch
from
August 29, 2026 16:18
17d1f73 to
8ba6428
Compare
LuciferYang
force-pushed
the
fix/track-caller-length-asserts
branch
from
August 29, 2026 16:28
8ba6428 to
e5b2b4f
Compare
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The revision accurately narrows the documented scope to the l2 and dot families and makes the regression’s panic-payload expectation explicit. Both helpers remain covered centrally without changing accepted inputs, panic messages, or public APIs.
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
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.
What this changes
assert_equal_lengthsandassert_batch_layoutinrust/lance-linalg/src/distance.rsget#[track_caller], so a length-contract panic reports the distance function that was called instead of the helper. Closes #8863.Before, every such panic reported
distance.rs:31. That helper has 16 call sites acrossl2.rs,dot.rs,dot_u8.rsandl2_u8.rs, andassert_batch_layouthas 4 more, so the panic told you the two lengths but not which metric raised them. These are the always-on asserts promoted in #8593, #8594 and #8639 precisely so they fire in release, which is where a backtrace is least likely to be available.The attribute moves the location one frame out, to the distance function. Going further, to the user's own call site, would mean putting it on every public distance function, so I left that alone.
Test plan
rust/lance-linalg/tests/panic_location.rsis new. It captures the panic location and message through a hook and asserts, fordot_u8and forl2_distance_batch, that the file isdot_u8.rsandl2.rsrather thandistance.rs, and that the message is the length-contract one rather than any other panic in the same file. It has to be an integration binary rather than a unit test, because replacing the global hook races with any other test that panics in the same process, anddistance/dot_f16.rsalready swaps the hook inside the lib test binary.Measured, one attribute at a time, since each is independently load-bearing:
removing it from
assert_equal_lengths:expected dot_u8.rs, got rust/lance-linalg/src/distance.rs:34removing it from
assert_batch_layout:expected l2.rs, got rust/lance-linalg/src/distance.rs:52cargo test -p lance-linalg: 389 passed and 1 ignored in the lib, 1 passed in the new binarycargo fmt --all -- --checkandcargo clippy -p lance-linalg --all-targets -- -D warnings: cleanTwo limits worth stating. The location moves one frame, to the distance function, not to the caller's own line: for the batch path that is the trait-impl override body in
l2.rs, not thel2_distance_batchthe caller wrote. Reaching the caller would mean putting the attribute on every public wrapper and trait method, and rustc's MIR inliner declines#[track_caller]callees, so that is a separate cost to weigh. And because the newtests/directory is the crate's first,--testsnow builds an integration target where it previously resolved to the lib alone;qemu-pre-haswellpasses--liband so will not run this test, which is correct, since that job exists to catch AVX2 leaking into the x86-64-v2 baseline and panic locations are not SIMD-dependent.Scope note: these two helpers are called from the l2 and dot families only.
cosineasserts its length contract inline withdebug_assert_eq!andhamminghas no production length assert, so neither is covered here.