Skip to content

node-api: clarify enum value ABI stability #59085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+24 −6
Open

node-api: clarify enum value ABI stability #59085

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions doc/api/n-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,24 @@ must use Node-API exclusively by restricting itself to using
and by checking, for all external libraries that it uses, that the external
library makes ABI stability guarantees similar to Node-API.

### Enum values in ABI stability
Copy link
Member

Choose a reason for hiding this comment

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

Should we also augment the https://github.com/nodejs/node/blob/main/doc/contributing/adding-new-napi-api.md document along with it?


All enum data types defined in Node-API should be considered as a fixed size
integer value. Bit flag enum types should be explicitly documented, and they
Copy link
Member

Choose a reason for hiding this comment

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

Can we be more specific and say that we expect enums to be int32_t values?

Suggested change
integer value. Bit flag enum types should be explicitly documented, and they
`int32_t` value. Bit flag enum types should be explicitly documented, and they

work with bit operators like bit-OR (`|`) as a bit value. Unless otherwise
documented, an enum type should be considered to be extensible.

Copy link
Member

Choose a reason for hiding this comment

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

Should we mention that we must never remove or reuse previous enum values. Instead, we must always only add new enum values with the new integer values.

For an enum type returned from an Node-API function, or as an out parameter of
Copy link
Member

Choose a reason for hiding this comment

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

a Node

Copy link
Member

Choose a reason for hiding this comment

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

or provided as an out ...

an Node-API function, the value is an integer value and an addon should
Copy link
Member

Choose a reason for hiding this comment

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

a Node-API

handle unknown values. For example, an addon should have a `default` branch
when checking `napi_status` in switch statements as new status codes can be
introduced in newer Node.js versions.
Comment on lines +133 to +135
Copy link
Member

Choose a reason for hiding this comment

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

For example, when checking napi_status in switch statements, an addon should include a default branch, as new status codes may be introduced in newer Node.js versions


Copy link
Member

Choose a reason for hiding this comment

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

While I see the justification for the napi_status value, I would like to better understand the application for the out- parameters. Are there examples in some system APIs where developers were adding new values to the output enum values. Are there any such examples in GCC libraries or any other system that cares about the ABI stability?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is rare because usually kernel APIs do not return a value like type enum. But there are still cases like CLD_CONTINUED used in siginfo_t::si_code, as an out-param in waitid, can be added over time. It is more common to return an int flag and new bit flags are added over time, e.g. IFF_LOWER_UP in ioctl.

It is more like a design choice that the kernel does not return types/flags information after the user code handed over them, avoiding compatibility issue.

However, Node-API needs to incorporate new ECMAScript features, and we must expose new (rare) JavaScript types (symbol as an example, retrospectively). Even napi_valuetype can not be determined to be frozen and napi_typeof must work with new types. As JavaScript land can pass new value types to a Node-API function freely (without respective Node-API versions), a Node-API function must defensively reject unknown types, in accordance to the ABI guarantee.

For an enum type used in an in-parameter, extended values are guarded with
Node-API version of which version they are introduced. The result of passing
an unknown integer value to Node-API functions is undefined unless otherwise
documented.
Comment on lines +137 to +140
Copy link
Member

Choose a reason for hiding this comment

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

For an enum type used in an in-parameter, extended values are version-guarded based on the Node-API version in which they were introduced. The result of passing an unknown integer value to Node-API functions is undefined unless otherwise documented.

Comment on lines +139 to +140
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid the undefined bahaviour and rather say that it is an error.

Suggested change
an unknown integer value to Node-API functions is undefined unless otherwise
documented.
an unknown integer value to Node-API functions is an error. The Node-API
functions are expected to return napi_invalid_arg in such cases.

Copy link
Member

Choose a reason for hiding this comment

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

Just a side note: these constraints generally look good to me but they could likely benefit from some concrete examples illustrating the correct structure.


## Building

Unlike modules written in JavaScript, developing and deploying Node.js
Expand Down Expand Up @@ -2203,7 +2221,7 @@ typedef enum {
} napi_key_filter;
```

Property filter bits. They can be or'ed to build a composite filter.
Property filter bit flag. This works with bit operators to build a composite filter.

#### `napi_key_conversion`

Expand Down Expand Up @@ -4419,11 +4437,11 @@ typedef enum {
} napi_property_attributes;
```

`napi_property_attributes` are flags used to control the behavior of properties
set on a JavaScript object. Other than `napi_static` they correspond to the
attributes listed in [Section 6.1.7.1][]
of the [ECMAScript Language Specification][].
They can be one or more of the following bitflags:
`napi_property_attributes` are bit flags used to control the behavior of
properties set on a JavaScript object. This works with bit operators. Other
than `napi_static` they correspond to the attributes listed in
[Section 6.1.7.1][] of the [ECMAScript Language Specification][].
They can be one or more of the following bit flags:

* `napi_default`: No explicit attributes are set on the property. By default, a
property is read only, not enumerable and not configurable.
Expand Down
Loading