Skip to content
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

Feature request: Buffer.readUint(BE|LE) with BigInt support #21662

Closed
jfedyczak opened this issue Jul 4, 2018 · 10 comments
Closed

Feature request: Buffer.readUint(BE|LE) with BigInt support #21662

jfedyczak opened this issue Jul 4, 2018 · 10 comments
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js.

Comments

@jfedyczak
Copy link

buf.readUIntLE(offset, byteLength) with byteLength > 6 to return BigInt

@addaleax addaleax added buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js. labels Jul 4, 2018
@addaleax
Copy link
Member

addaleax commented Jul 4, 2018

/cc @devsnek

@targos
Copy link
Member

targos commented Jul 4, 2018

Ref: #19691

@devsnek
Copy link
Member

devsnek commented Jul 4, 2018

i would be more comfortable with another method to give bigints (readBigUintLE(offset, byteLength))

@jfedyczak
Copy link
Author

@devsnek Separate methods for BigInt sound perfectly reasonable.

@mnpenner
Copy link

Oh man... I was just about to submit a new issue, when I thought I'd do one more search. Coulda got in big trouble 😝 I'll paste it here instead:

I see there was an attempt to add these methods a few years ago, but now that we have native BigInt support, perhaps it's time to revisit this?

All the existing methods should probably be updated to accept BigInts, as it appears presently they do not:

❯ node
> buf = Buffer.alloc(16)
<Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00>
> buf.writeUInt32BE(10n,0)
TypeError: Cannot convert a BigInt value to a number
    at writeU_Int32BE (internal/buffer.js:624:11)
    at Buffer.writeUInt32BE (internal/buffer.js:638:10)

And then some new 64-bit variants would be nice ({read,write}[U]Int64{BE,LE}). I think they'd only take BigInts as we can't safely represent 64-bit integers with regular Numbers.

And then lastly, I think buf.writeUIntBE and buf.writeUIntLE should allow byteLengths larger than 6 when given a BigInt.


Separate methods for the BigInt versions might make more sense, now that you guys mention it. It would create less confusion around why the 64-bit versions don't accept Numbers.

@no2chem
Copy link
Contributor

no2chem commented Sep 12, 2018

I've implemented this using N-API: https://github.com/no2chem/bigint-buffer - but I feel that this should be a core part of the node Buffer API (and BigInt API).

@rockaBe
Copy link

rockaBe commented Jan 14, 2019

Any news on this feature?

@mnpenner
Copy link

64-bit int support has arrived. I think we're still missing arbitrary precision support.

@ottokruse
Copy link

ottokruse commented Aug 28, 2020

Amazing that JS does support BigInt('0b1010101010101011010101010101'), who TF puts binary in string form like that, but not the obvious use case of reading a BigInt directly from a Buffer.

This code (BE) is quite some faster than BigInt(`0x${someBuffer.toString('hex')}`) but still a native operator would be easier and faster

someBuffer.reduce((val, int) => (val << BigInt(8)) | BigInt(int), BigInt(0));

@richardlau
Copy link
Member

I believe this was implemented via #19691 and released in Node.js 12.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

9 participants