Skip to content

Reject ZST#57

Merged
apfitzge merged 1 commit intoanza-xyz:mainfrom
apfitzge:reject-zsts-explicitly
Mar 11, 2026
Merged

Reject ZST#57
apfitzge merged 1 commit intoanza-xyz:mainfrom
apfitzge:reject-zsts-explicitly

Conversation

@apfitzge
Copy link
Contributor

@apfitzge apfitzge commented Mar 10, 2026

Problem

  • ZST will currently cause a panic

Changes

  • Just reject them with an compile-time error instead of panicking

@apfitzge apfitzge requested a review from OliverNChalk March 10, 2026 22:01
OliverNChalk
OliverNChalk previously approved these changes Mar 10, 2026
@apfitzge
Copy link
Contributor Author

@OliverNChalk rebased to deal with conflict on added test (no changes, accepted both)

src/spsc.rs Outdated
Comment on lines +412 to +414
if core::mem::size_of::<T>() == 0 {
return Err(Error::InvalidBufferSize);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if core::mem::size_of::<T>() == 0 {
return Err(Error::InvalidBufferSize);
}
const {
assert!(
core::mem::size_of::<T>() > 0,
"zero-sized types are not supported"
)
}

checked locally and this produces a compiler error for the test_zero_sized_types_are_rejected test below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this just panics instead - which would have alrady happened with a divide by zero. Would rather just return an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

No this fails to compile instead of failing at runtime. catches the issue much earlier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah you're right i missed the wrapping const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done and removed tests.

@apfitzge apfitzge force-pushed the reject-zsts-explicitly branch from e5f08ef to a87356a Compare March 11, 2026 14:54
@apfitzge apfitzge merged commit 3d9122b into anza-xyz:main Mar 11, 2026
2 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