-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(cdk): support encoding Tokens as numbers #2534
Conversation
Is it possible to isolate the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nifty little feature! Would be nice if we had (maybe we do, I haven't checked!) checks in JSII runtimes that they don't mess up the magic numbers in the language-native domain, just to be sure.
Numbers can now be encoded into a set of (hugely negative) numbers, and the encoding can be reversed. This allows APIs that take numbers to take lazy values and intrinsics that evaluate to numbers. This change only introduces the capability, it does not use it in any of the construct libraries yet. Fixes #1455.
9559b41
to
8acae0e
Compare
/** | ||
* Return a special Double value that encodes the given integer | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention in the docstring that this is used to encode an index to the token map and therefore it's okay that we don't support negative numbers. Also, mention the limit on the number of number tokens we can support.
* | ||
* We put the following marker into the top 16 bits (exponent and sign), and | ||
* use the mantissa part to encode the token index. To save some bit twiddling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"encode the index to the token in the token map"
* Create a unique number representation for this Token and return it | ||
*/ | ||
public registerNumber(token: Token): number { | ||
const tokenIndex = this.tokenCounter++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error if we reached the limit.
Numbers can now be encoded into a set of (hugely negative) numbers,
and the encoding can be reversed.
This allows APIs that take numbers to take lazy values and intrinsics
that evaluate to numbers.
This change only introduces the capability, it does not use it in any
of the construct libraries yet.
Fixes #1455.
Pull Request Checklist
design
folderBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.