Skip to content

nix_alloc_primop: args: fix constness and use all arguments - #16519

Open
yorickvP wants to merge 2 commits into
NixOS:masterfrom
yorickvP:fix-primop-api
Open

yorickvP wants to merge 2 commits into
NixOS:masterfrom
yorickvP:fix-primop-api

Conversation

@yorickvP

Copy link
Copy Markdown
Contributor

Motivation

The type for args in nix_alloc_primop was const char **, meaning a mutable array of const strings. We actually don't mutate the array, so it's fine to make it const instead. This makes the generated rust api less awkward.

Also, the first argument name was copied over instead of the correct one.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@yorickvP
yorickvP requested a review from edolstra as a code owner September 25, 2026 15:02
@github-actions github-actions Bot added the c api Nix as a C library with a stable interface label Sep 25, 2026
Comment thread src/libexpr-c/nix_api_value.cc Outdated
Previously, it would be possible to specify an args list with a
different length than the specified arity. RegisterPrimOp would then
raise the arity and the function would want superfluous arguments.

This commit turns this into an error, and adds documentation and
tests.

Assisted-by: Claude Code - Claude Opus 5.5
@yorickvP
yorickvP requested a review from xokdvium October 2, 2026 13:11

const char * const tooFew[] = {"a", nullptr};
ASSERT_EQ(nullptr, nix_alloc_primop(ctx, primop_square, 2, "tooFew", tooFew, nullptr, nullptr));
ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I never quite understood what use the error codes have an how they are converted (or rather how they design is supposed to work...). At least my understanding was that NIX_ERROR was supposed to represent evaluation errors (and this is more akin EINVAL which we don't have).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a blocker, because that part of the C API seems very undercooked in general. But I'm just wondering if I'm misunderstanding or if we really don't have any way to represent "invalid argument" errors.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c api Nix as a C library with a stable interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants