Skip to content

chore: tidy up a few compile-time assertions - #9158

Merged
connortsui20 merged 1 commit into
developfrom
claude/modernize-assert-syntax-v34sdq
Aug 3, 2026
Merged

connortsui20 merged 1 commit into
developfrom
claude/modernize-assert-syntax-v34sdq

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 3, 2026 •

Copy link
Copy Markdown
Member

Uses const { assert!(..) } for the function-body assertion in build_views to match the existing const-block sites, removes a redundant block around the single assert! in vortex-error, and corrects a doc comment that claimed DType is 16 bytes while the assertion below it checked for 24.

Three small cleanups, no behavior change:

- Use `const { assert!(..) }` for the fn-body assertion in `build_views`, so it
  matches the existing const-block sites in `optimizer::rules` and the AVX2 take
  kernel. Const blocks are the right form in expression position; item-scope
  assertions have no equivalent spelling.
- Drop the redundant block around the single `assert!` in `vortex-error`.
- Fix a stale doc comment: it claimed `DType` is 16 bytes while the assertion
  below it checked for 24.

Deliberately left alone: the `assert_eq_size!`/`assert_eq_align!` uses in
`varbinview::view` and the `[(); size_of::<T>()]` size checks in `dtype_impl`.
Both report the *actual* size when they fail ("found one with a size of 32",
"source type: `BinaryView` (256 bits)"), which is the useful part of a
struct-grew-unexpectedly guard. A const `assert!` can only report that the
condition was false, since const eval cannot format values into a panic
message. Likewise `assert_impl_all!` in `vortex-duckdb`, which asserts trait
implementations that `assert!` cannot express.

Signed-off-by: Connor <connor@spiraldb.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVLiXup87rwTYbMxtrnkCv
@connortsui20 connortsui20 added the changelog/chore A trivial change label Aug 3, 2026 — with Claude
@codspeed

codspeed Bot commented Aug 3, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 12.27%

⚡ 1 improved benchmark
✅ 1841 untouched benchmarks
⏩ 44 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation decompress[u64, (10000, 256)] 62.1 µs 55.3 µs +12.27%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/modernize-assert-syntax-v34sdq (6c24b87) with develop (2de0319)

Open in CodSpeed

Footnotes

  1. 44 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@connortsui20
connortsui20 enabled auto-merge (squash) August 3, 2026 20:25
@connortsui20
connortsui20 requested review from a10y and robert3005 August 3, 2026 20:26
@connortsui20
connortsui20 disabled auto-merge August 3, 2026 21:02
@connortsui20
connortsui20 enabled auto-merge (squash) August 3, 2026 21:02
@connortsui20
connortsui20 merged commit fae9da1 into develop Aug 3, 2026
84 of 85 checks passed
@connortsui20
connortsui20 deleted the claude/modernize-assert-syntax-v34sdq branch August 3, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants