Skip to content

util: add types.isFloat16Array() #57879

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -3102,6 +3102,23 @@ types.isExternal(new String('foo')); // returns false
For further information on `napi_create_external`, refer to
[`napi_create_external()`][].

### `util.types.isFloat16Array(value)`
Copy link
Member

Choose a reason for hiding this comment

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

This can be done fully on user land , why are we exposing it?

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be inconsistent to not expose the 16 bit version next to all others?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure consistency should be enough to expand the API surface. Is there any other reasoning for adding this other than consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the scope of this PR, the aim is on having this particular function on node:internal/util/types. It is re-exported as node:util/types as is.

Copy link
Member

Choose a reason for hiding this comment

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

It can't be done in userland easily, though, because you'd have to extract the Symbol.toStringTag getter. Consistency is a pretty strong justification for extending any API surface, on its own merits.


<!-- YAML
added: REPLACEME
-->

* `value` {any}
* Returns: {boolean}

Returns `true` if the value is a built-in {Float16Array} instance.

```js
util.types.isFloat16Array(new ArrayBuffer()); // Returns false
util.types.isFloat16Array(new Float16Array()); // Returns true
util.types.isFloat16Array(new Float32Array()); // Returns false
```

### `util.types.isFloat32Array(value)`

<!-- YAML
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/util/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ function isInt32Array(value) {
return TypedArrayPrototypeGetSymbolToStringTag(value) === 'Int32Array';
}

function isFloat16Array(value) {
return TypedArrayPrototypeGetSymbolToStringTag(value) === 'Float16Array';
}

function isFloat32Array(value) {
return TypedArrayPrototypeGetSymbolToStringTag(value) === 'Float32Array';
}
Expand Down Expand Up @@ -65,6 +69,7 @@ module.exports = {
isInt8Array,
isInt16Array,
isInt32Array,
isFloat16Array,
isFloat32Array,
isFloat64Array,
isBigInt64Array,
Expand Down
22 changes: 21 additions & 1 deletion test/parallel/test-util-types.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Flags: --experimental-vm-modules --expose-internals --allow-natives-syntax
// Flags: --experimental-vm-modules --expose-internals --allow-natives-syntax --js-float16array
// TODO(LiviaMedeiros): once `Float16Array` is unflagged in v8, remove `--js-float16array` above
'use strict';
Copy link
Member

Choose a reason for hiding this comment

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

I also think we shouldn't land this until this becomes stable.

Copy link
Member

Choose a reason for hiding this comment

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

I think doing it the other way around is actually good so that the new method is available as soon as it becomes stable. It will likely not be used before that anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is already stable, it is stage 4 – and if you meant the V8 implementation, that's really not necessary IMO, precisely because we can trust it will follow the spec.

const common = require('../common');
const assert = require('assert');
Expand All @@ -9,6 +10,9 @@ const { JSStream } = internalBinding('js_stream');

const external = (new JSStream())._externalStream;

// TODO(LiviaMedeiros): once linter recognizes `Float16Array`, remove next line
const { Float16Array } = globalThis;

for (const [ value, _method ] of [
[ external, 'isExternal' ],
[ new Date() ],
Expand Down Expand Up @@ -38,6 +42,7 @@ for (const [ value, _method ] of [
[ new Int8Array() ],
[ new Int16Array() ],
[ new Int32Array() ],
[ new Float16Array() ],
[ new Float32Array() ],
[ new Float64Array() ],
[ new BigInt64Array() ],
Expand Down Expand Up @@ -102,6 +107,9 @@ for (const [ value, _method ] of [
assert(!types.isInt32Array({ [Symbol.toStringTag]: 'Int32Array' }));
assert(types.isInt32Array(vm.runInNewContext('new Int32Array')));

assert(!types.isFloat16Array({ [Symbol.toStringTag]: 'Float16Array' }));
assert(types.isFloat16Array(vm.runInNewContext('new Float16Array')));

assert(!types.isFloat32Array({ [Symbol.toStringTag]: 'Float32Array' }));
assert(types.isFloat32Array(vm.runInNewContext('new Float32Array')));

Expand All @@ -127,6 +135,7 @@ for (const [ value, _method ] of [
const int8Array = new Int8Array(arrayBuffer);
const int16Array = new Int16Array(arrayBuffer);
const int32Array = new Int32Array(arrayBuffer);
const float16Array = new Float16Array(arrayBuffer);
const float32Array = new Float32Array(arrayBuffer);
const float64Array = new Float64Array(arrayBuffer);
const bigInt64Array = new BigInt64Array(arrayBuffer);
Expand All @@ -141,6 +150,7 @@ for (const [ value, _method ] of [
const fakeInt8Array = { __proto__: Int8Array.prototype };
const fakeInt16Array = { __proto__: Int16Array.prototype };
const fakeInt32Array = { __proto__: Int32Array.prototype };
const fakeFloat16Array = { __proto__: Float16Array.prototype };
const fakeFloat32Array = { __proto__: Float32Array.prototype };
const fakeFloat64Array = { __proto__: Float64Array.prototype };
const fakeBigInt64Array = { __proto__: BigInt64Array.prototype };
Expand All @@ -164,6 +174,10 @@ for (const [ value, _method ] of [
Object.setPrototypeOf(new Int16Array(arrayBuffer), Int16Array.prototype);
const stealthyInt32Array =
Object.setPrototypeOf(new Int32Array(arrayBuffer), Int32Array.prototype);
const stealthyFloat16Array =
Object.setPrototypeOf(
new Float16Array(arrayBuffer), Float16Array.prototype
);
const stealthyFloat32Array =
Object.setPrototypeOf(
new Float32Array(arrayBuffer), Float32Array.prototype
Expand Down Expand Up @@ -191,6 +205,7 @@ for (const [ value, _method ] of [
int8Array, fakeInt8Array, stealthyInt8Array,
int16Array, fakeInt16Array, stealthyInt16Array,
int32Array, fakeInt32Array, stealthyInt32Array,
float16Array, fakeFloat16Array, stealthyFloat16Array,
float32Array, fakeFloat32Array, stealthyFloat32Array,
float64Array, fakeFloat64Array, stealthyFloat64Array,
bigInt64Array, fakeBigInt64Array, stealthyBigInt64Array,
Expand All @@ -208,6 +223,7 @@ for (const [ value, _method ] of [
int8Array, stealthyInt8Array,
int16Array, stealthyInt16Array,
int32Array, stealthyInt32Array,
float16Array, stealthyFloat16Array,
float32Array, stealthyFloat32Array,
float64Array, stealthyFloat64Array,
bigInt64Array, stealthyBigInt64Array,
Expand All @@ -222,6 +238,7 @@ for (const [ value, _method ] of [
int8Array, stealthyInt8Array,
int16Array, stealthyInt16Array,
int32Array, stealthyInt32Array,
float16Array, stealthyFloat16Array,
float32Array, stealthyFloat32Array,
float64Array, stealthyFloat64Array,
bigInt64Array, stealthyBigInt64Array,
Expand All @@ -248,6 +265,9 @@ for (const [ value, _method ] of [
isInt32Array: [
int32Array, stealthyInt32Array,
],
isFloat16Array: [
float16Array, stealthyFloat16Array,
],
isFloat32Array: [
float32Array, stealthyFloat32Array,
],
Expand Down
2 changes: 1 addition & 1 deletion tools/doc/type-parser.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const jsGlobalTypes = [
'TypeError', 'URIError', 'WeakMap', 'WeakSet',

'TypedArray',
'Float32Array', 'Float64Array',
'Float16Array', 'Float32Array', 'Float64Array',
'Int8Array', 'Int16Array', 'Int32Array',
'Uint8Array', 'Uint8ClampedArray', 'Uint16Array', 'Uint32Array',
];
Expand Down
Loading