ABI future-proofing: pin enum width, fixed-width uint64_t sizes#106
Merged
Conversation
Two layout-neutral hardening changes to the plugin C ABI, both protecting third-party plugins built on non-default toolchains. Genuinely irreversible once such plugins ship, so done before wide adoption. Enum width pinning: every by-value C-ABI enum (PJ_primitive_type_t, PJ_data_source_state_t, the message-level / message-box-type / message-box- buttons enums, PJ_toolbox_message_level_t) gains a `..._FORCE_INT32 = 0x7FFFFFFF` enumerator. The existing sizeof==4 static_asserts only protect this project's build; the enumerator is what forces a stable 4-byte width into a plugin compiled with -fshort-enums, where these enums (max value 0xFF) would otherwise shrink to 1 byte and silently misalign every struct/param that embeds them. Adds the missing PJ_message_box_buttons_t sentinel. Fixed-width sizes: replaces size_t with uint64_t in every C-ABI struct and function-pointer signature (plugin_data_api.h). size_t is what made the ABI implicitly 64-bit-only; uint64_t makes the width explicit and platform- independent. Both are layout-identical on the supported 64-bit target: enum sizeof stays 4, size_t and uint64_t are the same width, no vtable offset or size moves. All layout sentinels and 62/62 tests pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prior commit widened the C ABI (plugin_data_api.h) from size_t to
uint64_t, but the C++ host vtable implementations and SDK host-views that
fill those slots still used size_t. On macOS uint64_t is `unsigned long
long` while size_t is `unsigned long` — same width, distinct types — so the
trampolines fail to bind to the slots and the out-param pointers mismatch
(size_t* vs uint64_t*). Linux (glibc) and Windows (MSVC) alias both to one
type, so only macOS CI caught it.
Convert every ABI-boundary size_t to uint64_t, keeping size_t for purely
internal in-memory indexing:
- pj_datastore host trampolines: append_record/append_bound_record
field_count, push_owned size, lazy-fetch out_size, set_retention_budget,
list_topics capacity/out_count, get_bytes out_size, entry_count return.
- SDK host-views (plugin_data_api.hpp): list_topics/get_string_list counts,
entryCount return, setRetentionBudget param, both LazyBox trampolines;
object_bytes.hpp get_bytes; settings_store_host slots.
- testing helpers + 4 built test trampolines bound to the same slots.
Value conversions size_t<->uint64_t are same-width and warning-free on
64-bit; only the pointer and function-pointer types required fixing.
Internal storage (RetentionBudget.max_memory_bytes, encoding/buffer
indexing) stays size_t.
Build (Debug+ASAN, -Werror -Wconversion) + 62/62 tests pass; clang-format clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Final follow-up from the code-review / sustainability pass. The query binary-search + SPDX/CI hygiene already landed in #104; this PR carries the ABI future-proofing changes — the ones that become irreversible once third-party plugins compile against the headers.
Changes (1 commit, layout-neutral on 64-bit)
1 · Enum width pinning. Every by-value C-ABI enum gains a
..._FORCE_INT32 = 0x7FFFFFFFenumerator:PJ_primitive_type_t,PJ_data_source_state_t,PJ_data_source_message_level_t,PJ_message_box_type_t,PJ_message_box_buttons_t,PJ_toolbox_message_level_t. The pre-existingsizeof(enum)==4static_asserts only protect this build; the enumerator is what forces a stable 4-byte width into a plugin compiled with-fshort-enums(default on some ARM/embedded toolchains), where these enums (max value0xFF) would otherwise shrink to 1 byte and silently misalign every struct/param that embeds them —PJ_scalar_value_t.type,PJ_field_info_t.type,ensure_field, … Adds the missingPJ_message_box_buttons_tsentinel.2 · Fixed-width sizes. Replaces
size_twithuint64_tin every C-ABI struct and function-pointer signature inplugin_data_api.h.size_tis what made the ABI implicitly 64-bit-only;uint64_tmakes the width explicit and platform-independent.Why now
Both are impossible to fix after a non-conformant plugin ships — the wrong enum/size width gets baked into its frozen structs. Cheap and safe to do before wide adoption.
Compatibility
Layout-identical on the supported 64-bit target: enum
sizeofstays 4,size_t≡uint64_t(both 8 bytes), no vtable offset or size moves. All ABI layout sentinels and the test suite pass unchanged.Verification
-Wall -Wextra -Werror+ ASAN (./build.sh --debug)../test.sh→ 62/62 pass, includingabi_layout_sentinels_test(compile-time layout guards).Release
Recommend a MINOR bump. It is binary-compatible (→ PATCH by that measure), but adding enumerators could trip a downstream plugin that compiles our C enums with
-Werror -Wswitchand an exhaustiveswitchlackingdefault:(a source-compat break). Version files are not touched here — say the word and I'll bumpconanfile.py+CMakeLists.txt(PJ_PACKAGE_VERSION).Not included (follow-up)
The third Tier-0 irreversible — a wire/ObjectStore format-version anchor — is intentionally deferred: its mechanism (in-band version field across ~16 builtin codecs vs. a structured
schema_id/version onObjectTopicDescriptor+ a "breaking change ⇒ new type id" policy) is a permanent design choice worth deciding explicitly.🤖 Generated with Claude Code