swig, generator: unify accessor annotations and add custom-access groundwork#3315
Conversation
Rename the member-level annotation prefix from !accessors: to !access:
to decouple it from the !generate-accessors struct-level annotation.
Until now, the !accessors:<value> member annotation was consumed only
by the accessor generator, and its name reflected that — it read as a
natural companion to the struct-level !generate-accessors annotation.
That one-to-one relationship is about to change. An upcoming
!generate-python struct-level annotation will also need to read the
same member annotations, both to drive the generated SWIG layer and to
validate consistency between the C and Python sides. The annotation is
no longer the private concern of the accessor generator.
Keeping the name !accessors: would make that relationship confusing:
- The visual similarity to !generate-accessors suggests the member
annotation is owned by accessor generation, when in fact multiple
generators will consume it.
- Readers (and future contributors adding more generators) would
reasonably — but wrongly — assume the annotation is accessor
specific and only relevant when !generate-accessors is set.
Renaming to !access: breaks that false association. The new name also
reads more naturally as English:
char *name; // !access:readonly
char *addr; // !access:none
vs.
char *name; // !accessors:readonly
char *addr; // !accessors:none
This patch is a pure rename with no behavioural change:
- generate-accessors.py: update has_annotation() lookups and the
docstring examples
- private.h: rename all 47 member annotations
- generate-accessors.md: update every example, table row and note;
re-pad the affected table rows to preserve column alignment
Generator output (accessors.h / accessors.c / accessors.ld) is byte
identical before and after this commit.
Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Opus 4.7 <[email protected]>
Replace the single-mode !access:<value> annotation with an orthogonal
two-axis spec:
!access:read=<mode>,write=<mode>
where read and write are independent and each takes one of:
generated (generator emits the accessor), custom (hand-written
accessor in the public API, generator emits nothing), or none (no
accessor exists). The same spec is accepted at the struct level via
!generate-accessors:read=<mode>,write=<mode>, with per-axis
inheritance from the struct-level default to member-level overrides.
The new model lets downstream consumers (the Python-binding generator,
the nvme.i consistency check) distinguish "no accessor" from
"hand-written accessor", and cleanly expresses mixed cases such as
"generated getter + hand-written setter" that the old one-dimensional
annotation could not name. This is a clean break: the old :none /
:readonly / :writeonly / :readwrite tokens are no longer recognised.
See libnvme/tools/generator/generate-accessors.md for the full syntax,
inheritance rules, and examples.
All 47 !access: annotations in private.h are updated accordingly. The
regenerated accessors.{h,c,ld} are byte-identical to the pre-change
baseline.
Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Opus 4.7 <[email protected]>
| struct libnvme_stat stat[2]; /* gendisk I/O stat */ | ||
| unsigned int curr_idx; /* current index into the stat[] */ | ||
| bool diffstat; // !access:none | ||
| bool diffstat; // !access:read=none,write=none |
There was a problem hiding this comment.
Is this change necessary? It looks like a no-op. The same comment on diffstat below
There was a problem hiding this comment.
Good catch — it does look like a no-op at first glance.
The behavior isn’t defined only at the member level though — it comes from the struct-level default, and member annotations act as overrides.
In this case:
struct libnvme_path { // !generate-accessors
implies the default:
read=generated,write=generated
So:
bool diffstat; // !access:read=none,write=none
is explicitly overriding that default to disable both directions.
That said, I agree this adds noise when we’re just “turning things off”. One thing we can improve is choosing struct-level defaults that better match the majority of members, which would reduce the need for these overrides.
| int queue_depth; // !access:read=custom,write=none | ||
| long multipath_failover_count; // !access:read=custom,write=none | ||
| long command_retry_count; // !access:read=custom,write=none | ||
| long command_error_count; // !access:read=custom,write=none |
There was a problem hiding this comment.
Just wondering if the default should always be none when not listed, e.g.
// !access:read=custom
or would this be confusing? I'd like to reduce the 'noise' level if possible
There was a problem hiding this comment.
The current behavior is that struct-level annotations define defaults, and member-level annotations override per axis.
So something like:
// !access:read=custom
would inherit the struct-level default for write, not imply write=none.
That’s why we currently need to spell out:
// !access:read=custom,write=none
I do agree with the goal of reducing noise though. The best way to do that is probably to pick struct-level defaults that match the common case, so we only annotate exceptions at the member level.
|
If it helps, we could also consider making I can make that change if you'd like. |
2 participants
Summary
This PR contains two small, incremental patches that prepare the codebase for upcoming work on Python/SWIG binding generation.
The goal is to make accessor semantics explicit and machine-readable so downstream tooling can reason about read/write behavior and custom implementations.
Patch 1: Rename annotation
!accessors:*→!access:*Patch 2: Explicit access model + custom support
Introduce a unified annotation model:
!access:read=<mode>,write=<mode>!generate-accessors:read=<mode>,write=<mode>Modes:
generated,custom,noneStruct-level annotations define defaults; member-level overrides per axis
Add support in the generator
Update documentation
Annotate existing fields that already rely on custom accessors
Rationale
The previous annotation scheme could not clearly express:
This becomes important for:
These changes establish a clear and extensible model without altering existing runtime behavior.
Notes
Feedback welcome before building on top of this.