bbakernoaa / bbakernoaa/AMIO

Evaluate adopting `restrict` on the public C API pointer parameters

Open
#13 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
1
Forks
3
Avg merge
2d 30m
Merged PRs (30d)
5

Description

Raised by @DWesl in review of #9:

> On the other hand, `restrict` is C99 with no C++ equivalent, even if most compilers have a home-rolled version due to how useful it is for optimization.

`restrict` is currently used nowhere in AMIO. Deferring from #9 because applying it only to the two new functions would leave the majority of the C API inconsistent.

## Scope

`include/amio/amio.h` declares 15 entry points. Those taking two or more pointer parameters that could in principle alias:

- `amio_init` — `manifest_path`, `out_core`
- `amio_open_dataset` — `config_path`, `out_dataset`
- `amio_write` — `var_name`, `host_data`, `shape`, `out_io`
- `amio_read` — `var_name`, `bbox`, `out_view`
- `amio_get_var_attribute_text` — `var_name`, `attr_name`, `out_buf`, `out_len` (added in #9)
- `amio_get_var_attribute_double` — `var_name`, `attr_name`, `out_value` (added in #9)

`amio_view_data` (`out_data`, `out_size`) is a marginal case. The remaining entry points take a single handle and don't qualify.

## Prerequisites

**1. Portability macro.** `restrict` is a C99 keyword with no C++ equivalent, and `amio.h` is compiled both ways by `tests/header_isolation`: `amio_header_c99_isolation` (`-std=c99 -pedantic -Werror`, empty include path) and `amio_header_cxx_inclusion` (`-std=c++20`). Requires something like:

```c
#if defined(__cplusplus)
# if defined(__GNUC__) || defined(__clang__) || defined(_MSC_VER)
# define AMIO_RESTRICT __restrict
# else
# define AMIO_RESTRICT
# endif
#elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
# define AMIO_RESTRICT restrict
#else
# define AMIO_RESTRICT
#endif
```

Natural home is `amio_export.h`, alongside `AMIO_API`. Both isolation tests must be confirmed still passing, in particular that `__restrict` doesn't trip `-pedantic` in the C++ compile.

2. Documentation. restrict makes it undefined behavior for a caller to pass overlapping pointers. That is a new obligation on consumers of a published ABI (currently 0.2.0 in #9), so every affected Doxygen block needs an explicit non-overlap precondition. Fortran consumers via `iso_c_binding` are unaffected.

### Open question: is it worth it?
The stated motivation is optimization, but these are coarse-grained entry points that perform a handle-table lookup and dispatch once per call. There's no inner loop for the aliasing information to unlock. The realistic benefit is documenting intent rather than generated code.

Worth deciding this before doing the work. Options:

1. Apply `AMIO_RESTRICT` across all qualifying entry points.
2. Skip the qualifier and instead state the non-overlap precondition in the Doxygen blocks only.
3. Close as not worthwhile.

If we go with (1), it should land as a single PR covering the whole C API so new and existing declarations stay consistent.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading include/amio/amio.h and amio_export.h, then inspect tests/header_isolation for the C99 and C++20 compilation requirements. Resolve whether the API should use AMIO_RESTRICT, document non-overlap preconditions only, or skip both; if adopted, cover all qualifying declarations and affected Doxygen blocks, with both isolation tests still passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, cpp
Domain
api, backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.