Generate enums for operators and overloads #243
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hudlow
commented
Dec 11, 2025
packages/cel/src/func.ts
Outdated
| ): CelOverload<P, R> { | ||
| return new FuncOverload(parameters, result, impl); | ||
| return new FuncOverload( | ||
| crypto.randomUUID(), |
Collaborator
Author
There was a problem hiding this comment.
It seems like crypto is safe to use everywhere?
Member
There was a problem hiding this comment.
"Widely available" according to mdn. Safe to use. https://developer.mozilla.org/en-US/docs/Web/API/Crypto/randomUUID
packages/cel/src/type.ts
Outdated
| /** | ||
| * Returns true if the given value is a numeric CelType. | ||
| */ | ||
| export function isCelNumericType( |
Member
There was a problem hiding this comment.
I see that this is only used in tests can we move this to the test file?
Collaborator
Author
|
Mostly superseded by #244, but some automation is probably worth keeping. |
hudlow
changed the title
hudlow
force-pushed
the
hudlow/overload-flags
branch
from
483c511 to
2026e55
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Random overload IDsAfter staring at the schema I'm proposing that we generate UUIDs for
overload_idvalues at compile time.It basically seems like a bug to me that other implementations treat these as non-opaque values when their stated purpose is purely for cross-referencing values in a compiled expression payload. Using non-opaque IDs, let alone specific, stable non-opaque IDs does not seem to be part of the contract.And I believe it should be avoided because of the potential for unintentional collisions that could cause subtle-but-nasty bugs. The best way to avoid depending on the semantics or stability of values is to randomize them.Cross-type numeric comparisonsWhere we need to treat specific overloads specially, I have instead proposed a mechanism for standardized flags, and added
CelOverloadFlag.CROSS_TYPE_NUMERIC_COMPARISONas the inaugural flag. I've also added a syntheticisCrossTypeNumericComparisonproperty to overloads and a test to verify that the overloads — and only those overloads — that we expect to returntruefor this property do so.Stale generated files
The
operator_const.tsandoverload_const.tsfiles were previously generated... but generated from some hand-maintained YAML files? The generation mechanism and source YAML files have been lost in refactors.I've added code that generates these from
cel-goto thecel-specbuild, and now import these enumerations from that package.