WIP Lots of improvements#898
Draft
snazy wants to merge 59 commits into
Draft
Conversation
snazy
force-pushed
the
broad-review-fixes
branch
13 times, most recently
from
July 21, 2026 10:41
cdc5296 to
e3d0c26
Compare
Recognize map and filter macro fold shapes during planning and build result lists with a mutable evaluation-local buffer. This avoids repeated accumulator list copying while preserving generic fold fallback for non-standard accumulator references.
Plan undecorated checked top-level variables as direct activation lookups instead of absolute attributes. Select roots and decorated programs keep the attribute shape so qualifier planning, partial unknowns, and custom decorators retain their existing behavior.
Route unresolved varargs calls through a receiver-specific evaluator that avoids building the full argument array before receiver dispatch. Also pass the complete receiver tail to dynamic receivers and cover unchecked receiver varargs calls with multiple arguments.
Includes some Java 17 specific improvements, not cosmetic fluff.
This change adds the necessary base work for the CEL-Spec bump.
The original Bazel-driven build in CEL-Java to run conformance-tests was leveraging the `simple_test` convenience binary to run tests. That binary no longer exists for CEL-Spec 0.25.2 and was replaced with a Java client in the CEL-Spec version bump PR. This change goes further and removes the Bazel build entirely and replaces the client/server split, which was needed for the `simple_test` tool, with JUnit test-factory.
Update string-to-bool conversion to accept the CEL-Spec spellings while rejecting unsupported mixed-case values. Compare type values by CEL type name so well-known object type values match their built-in timestamp and duration type values. Allow null assignment to wrapper fields and leave wrapper, message, timestamp, and duration fields unset during protobuf construction. Support backtick-quoted field names in select expressions and message field initializers, then remove the now-passing conformance skips.
Handle CEL null values at the protobuf construction boundary for repeated and map fields whose value type is timestamp, duration, or a protobuf wrapper. Convert map message values to the actual target protobuf message type instead of always using google.protobuf.Value. Avoid hasField() for repeated wrapper fields when reading protobuf values, since repeated fields do not support presence checks.
The map Any null-retention conformance cases now pass after the protobuf map conversion changes. Remove the proto2 and proto3 skip entries so the JUnit conformance suite covers those cases.
Protobuf message construction could let bad field conversions escape as Java runtime exceptions, which broke disabled-check conformance cases expecting evaluation errors. Wrap field conversion and assignment failures from newValue() as CEL Err values, add provider coverage for invalid null assignments, and enable the remaining proto2/proto3 set_null conformance cases.
Include the CEL-Spec type_deduction suite in the JUnit conformance runner to cover checker result types for already-supported language features. Keep explicit skips for the remaining unsupported checker cases around optionals, flexible type parameter generality, wrapper promotion, and legacy nullable generic candidates.
The dynamic float protobuf range-assignment cases now pass under the JUnit conformance runner. Remove the stale proto2 and proto3 float range skip entries while keeping the int32 and uint32 range cases skipped because they still overflow into protobuf field values instead of returning range errors.
The legacy proto2 and proto3 standalone enum out-of-range assignment cases now pass under the JUnit conformance runner. Remove the stale skip entries for the too-large and too-negative enum assignment cases.
Int32Value and UInt32Value native conversions accepted values outside their protobuf wrapper ranges by truncating them to Java int. Add range checks for those wrapper conversions, cover the error paths in unit tests, and enable the int32 and uint32 dynamic range conformance cases.
Unsigned CEL uint values stored in negative Java long values were treated as JSON numbers because the conversion compared the signed long directly. Use unsigned comparison for uint-to-JSON conversion, add coverage for the uint64 max value, and enable the uint64 wrapper JSON conformance case.
Timestamp JSON conversion returned a wrapper message instead of google.protobuf.Value and omitted fractional seconds. FieldMask was also missing from the default well-known protobuf registration path, which kept its conformance case from checking. Return proper JSON Value instances for timestamp conversion, register FieldMask in the default protobuf type metadata, add narrow JSON Value conversion for FieldMask, Timestamp, and Empty message objects, and enable the corresponding wrapper JSON conformance cases.
The CEL-Spec type-deduction suite uses optional.none and optional.of in check-only cases. Those expressions could not type-check because the optional helper functions were not declared. Add an opt-in optional library with compile-time declarations for the optional helper functions, and use it in the conformance harness for optional expressions. This enables optional type deduction without adding runtime optional semantics.
The strong enum sections contain a mix of unsupported typed-enum semantics and cases that already work with the current int-backed enum behavior. Replace the broad section skips with targeted skips for the remaining typed-enum gaps, and convert conformance enum_value bindings to integer values so disabled-check conversion cases can run.
The nested protobuf message literal case now passes under the current conformance runner and evaluator. Remove the stale skip so the parse nesting coverage runs with the rest of the CEL-Spec simple suite.
The CEL-Spec encoder extension suite covers Base64 encode and decode helpers, including decode input without padding. Add an opt-in encoder extension library with base64.encode and base64.decode, wire it into the conformance harness for base64 expressions, and include direct core coverage for successful and invalid decode inputs.
The CEL-Spec bindings extension uses cel.bind to introduce a local variable in the result expression. CEL-Java parsed those expressions as ordinary calls, so the checker reported undeclared local variables. Add cel.bind as a standard macro, expanding it to a single-iteration comprehension with a dynamic accumulator initializer. Add direct macro coverage and include bindings_ext.textproto in the conformance suite.
The CEL-Spec math extension suite exercises numeric helpers such as min/max, rounding, finite checks, and bitwise operations. CEL-Java did not expose those extension functions, so the suite could not be enabled. Add an opt-in math extension library, wire it into the conformance harness for math expressions, and include direct coverage for representative numeric, rounding, and bitwise cases.
The CEL-Spec network extension suite requires net.IP and net.CIDR values with strict literal parsing, canonical formatting, receiver methods, and type-literal support. CEL-Java did not expose those extension functions or custom network types. Add an opt-in network extension library, support custom object type values with explicit traits, wire the library into the conformance harness, and cover representative IP and CIDR behavior with direct tests.
The CEL-Spec block suite uses test-only macros to model sequential local bindings and generated comprehension variable names. CEL-Java did not provide those macros, so the suite could not be parsed or executed in the pure Java conformance harness. Add opt-in parser macros for the block conformance cases, keep them out of the default macro set, wire them into the conformance parser only when needed, and enable block_ext with explicit skips for the optional-dependent cases.
The updated CEL-Spec includes macros that expose both key/index and value variables during comprehension evaluation. CEL-Java only modeled one iteration variable, which prevented exists/all/existsOne and transform macros from covering list indexes and map values. Add parser, checker, and interpreter support for the second comprehension variable, add transformList and transformMap macro expansion, preserve iter_var2 in AST pruning and exhaustive evaluation, and enable the macros2 conformance file with direct unit coverage.
Move CEL grammar generation into cel-core using CongoCC and remove the generated-antlr module, shadow jar dependency, and ANTLR runtime wiring. The generated parser now builds internal AST nodes with typed CEL expression conversion hooks, keeping parser construction close to the grammar while preserving the existing parser API and source-location handling. This simplifies dependency management for downstream consumers, removes the relocated ANTLR artifact from the build graph, and makes future grammar changes easier to review in the core module.
Add runtime optional values and wire optional constructors, receiver methods, lazy optMap/optFlatMap macros, and optional access operators. Parse optional select, index, list, map, and message syntax with CongoCC, then preserve the protobuf optional-entry AST fields through checking and interpretation. Enable optionals conformance coverage and remove the optional-dependent block-extension skips.
Struct-pruning was effectively doing nothing.
Convert MapT entries directly to protobuf MapEntry values instead of first materializing an intermediate Java map. Use the protobuf field descriptors for key and value conversion so scalar widths, unsigned values, enums, messages, and null handling retain their protobuf semantics.\n\nAdd regression coverage that rejects native-map materialization and exercises representative protobuf map value types.
Bind ordinary Jackson-discovered methods and fields to property-specific MethodHandles. Checked selectors can invoke the cached getter directly, while dynamic JacksonObjectT access performs one metadata lookup. Custom and inaccessible writers retain the BeanPropertyWriter fallback. OpenJDK 21.0.11, Linux amd64, JMH 1.37, 2 threads, 3 forks: Before: ``` preResolvedPropertyRead: 0.009 +/- 0.001 us/op, approximately 0 B/op propertyRead: 0.031 +/- 0.003 us/op, 32.012 +/- 0.027 B/op ``` After: ``` preResolvedPropertyRead: 0.009 +/- 0.001 -> 0.003 +/- 0.001 us/op (66.7% faster), approximately 0 B/op propertyRead: 0.031 +/- 0.003 -> 0.032 +/- 0.003 us/op (within error), about 32 B/op ```
Capture each Jackson-discovered BeanPropertyWriter in the cached FieldType. Checked selectors invoke Jackson 3s existing method-handle-backed accessor without a name lookup, while dynamic JacksonObjectT access performs one metadata lookup. OpenJDK 21.0.11, Linux amd64, JMH 1.37, 2 threads, 3 forks: Before: ``` preResolvedPropertyRead: 0.006 +/- 0.001 us/op, approximately 0 B/op propertyRead: 0.035 +/- 0.005 us/op, 32.016 +/- 0.035 B/op ``` After: ``` preResolvedPropertyRead: 0.006 +/- 0.001 -> 0.004 +/- 0.001 us/op (33.3% faster), approximately 0 B/op propertyRead: 0.035 +/- 0.005 -> 0.030 +/- 0.004 us/op (confidence intervals overlap), about 32 B/op ```
Bind validated MethodHandle getters for registered generated messages with singular bool, string, bytes, signed integer, float, and double fields. Dynamic messages, extensions, incompatible classes, and normalization-sensitive field categories retain descriptor access. OpenJDK 21.0.11, Linux amd64, JMH 1.37, 2 threads, 3 forks: Before: ``` generatedScalarFieldGetter: 0.023 +/- 0.001 us/op, approximately 0 B/op benchmarkResolverFieldQualifier: 0.077 +/- 0.007 us/op, 24.001 +/- 0.001 B/op ``` After: ``` generatedScalarFieldGetter: 0.023 +/- 0.001 -> 0.004 +/- 0.001 us/op (82.6% faster), approximately 0 B/op benchmarkResolverFieldQualifier: 0.077 +/- 0.007 -> 0.072 +/- 0.005 us/op (confidence intervals overlap), 24.001 B/op ```
Make TypeAdapter a bidirectional conversion boundary with exact-wrapper primitive input defaults, allocation-free built-in input overrides, strict primitive output helpers, and one compatibility fallback for custom Val implementations. Mark `Val.convertToNative()` deprecated for removal while retaining its implementations as the compatibility surface. Preserve wrapper-token behavior, CEL int/uint range semantics, and `TypeAdapter`'s functional-interface contract. Use each value container's or environment's `TypeAdapter` for outbound native conversions so conversion policy has a single internal seam. Remove `ObjectT`'s redundant `TypeAdapter` forwarding and keep the deprecated `Val` method only as the compatibility fallback in `TypeAdapterSupport`. Thread the environment adapter through conformance result encoding and use direct values in type-checked string extension tests. This removes deprecated Val conversion calls from operational test paths while retaining the per-Val conversion tests as compatibility coverage. Added focused native-to-Val, Val-to-native, and direct primitive getter benchmarks. Benchmark results: * Primitive boolean inbound: 3.451 → 0.438 ns/op * Primitive long inbound: 5.988 → 2.215 ns/op * Primitive double inbound: 4.120 → 2.215 ns/op * Primitive outbound adapter methods: approximately 0.51 ns/op, effectively zero allocation
Benchmarked overload dispatch paths before this change: Add focused JMH coverage for direct and evaluator calls at arities zero through four, receiver fallback with zero through four tail arguments, and bound generic trait success/fallback. Add semantic characterization for checked and unchecked binding, receiver fallback, error and unknown propagation, cost, call shape, and state tracking. Add fixed three/four/five-argument operation and evaluator while retaining the generic `FunctionOp` fallback. Preserve generic error/unknown ordering, trait checks, receiver fallback, call metadata, cost, decorators, and no-such-overload formatting. JMH baseline, using JDK 21.0.11, 3 forks, 5x200 ms warmup, 10x200 ms measurement, GC profiler: | Path | Generic before | Fixed after | Allocation before | Allocation after | | --- | ---: | ---: | ---: | ---: | | evaluator, arity 3 | `9.628 ns/op` | `2.096 ns/op` | `32 B/op` | approximately `0 B/op` | | `math.greatest`, arity 3 | `16.312 ns/op` | `3.073 ns/op` | `64 B/op` | approximately `0 B/op` | | Strings `indexOf` with offset | `15.601 ns/op` | `10.419 ns/op` | `56 B/op` | `24 B/op` | | evaluator, arity 4 | `12.524 ns/op` | `2.556 ns/op` | `32 B/op` | approximately `0 B/op` | | `math.greatest`, arity 4 | `20.520 ns/op` | `3.994 ns/op` | `64 B/op` | approximately `0 B/op` | | Strings `replace(..., n)` | `31.349 ns/op` | `24.956 ns/op` | `136 B/op` | `104 B/op` | | evaluator, arity 5 | `14.927 ns/op` | `2.868 ns/op` | `40 B/op` | approximately `0 B/op` | | `math.greatest`, arity 5 | `21.978 ns/op` | `4.450 ns/op` | `80 B/op` | approximately `0 B/op` |
Expand validated generated protobuf access to explicit presence, unsigned fields, numeric enums, wrappers, repeated fields, lazy maps, and PbObjectT-only ordinary messages. Reuse cached FieldType metadata in PbObjectT through allocation-free nested cache hits, invalidate entries when dynamic registrations are upgraded, and retain descriptor fallbacks for incompatible values and unsupported generated APIs. Keep ordinary-message getters off checked deep chains: the experiment was latency-neutral and increased allocation, while restricting them to PbObjectT improved object message access. Add CEL-facing semantic coverage and a dedicated FieldAccessBench. JMH 1.37, OpenJDK 21.0.11 Zulu, 3 forks, 5x200ms warmup, 10x200ms measurement, 1 thread, GC profiler (ns/op, B/op): * checked terminal int32: 59.64 +/- 6.33, 88 B -> 38.96 +/- 4.01, 72 B * PbObject generated int32: 30.81 +/- 2.79, 56 B -> 15.15 +/- 0.29, 40 B * PbObject dynamic int32: 30.67 +/- 2.87, 56 B -> 14.56 +/- 0.41, 40 B * raw presence present: 12.32 +/- 0.57, ~0 B -> 4.00 +/- 0.03, ~0 B * raw enum: 36.41 +/- 3.78, 64 B -> 4.49 +/- 0.09, 16 B * PbObject map selection: 149.71 +/- 9.22, 328 B -> 27.70 +/- 0.48, 72 B * PbObject nested message: 52.16 +/- 3.08, 32 B -> 26.33 +/- 3.43, 32 B Fallback remained neutral: DynamicMessage scalar was 17.83 +/- 0.08 -> 17.70 +/- 0.33 ns/op at ~0 B/op. Checked deep selection was 147.54 +/- 14.46 -> 159.82 +/- 14.24 ns/op at 120 B/op with overlapping intervals, which is why that path keeps descriptor message access.
snazy
force-pushed
the
broad-review-fixes
branch
from
July 21, 2026 16:09
4003d80 to
f468bdd
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.