Conversation
…nctions Align cel-cpp function resolution with cel-go and cel-java by introducing: 1. Plan-time overload filtering using overload_id from checked expressions 2. Runtime type-level argument matching for container elements and TypeParam, guarded by enable_type_level_overload 3. Parse-only and legacy fallback paths using arity/kind-based matching Key changes: - Add overload_id to FunctionDescriptor with Type-based constructors - Add FindStaticOverloadById/FindLazyOverloadById to FunctionRegistry - Add FindFunctionOverloadById(name, overload_id) to ActivationInterface - Add enable_type_level_overload flag to RuntimeOptions - Prefer checked overload_id candidates while still validating runtime arguments Compatibility: - Kind-level matching remains the default runtime behavior - Functions without overload_id fall back to arity-based matching
Annotate standard library, optional type, and extension function registrations with their corresponding overload_id constants. This enables plan-time overload resolution via overload_id lookup from the checker reference_map, replacing arity-only candidate selection for checked expressions where overload_id data is available. - runtime/standard/: standard functions pass StandardOverloadIds constants through RegisterHelper and FunctionAdapter interfaces. - runtime/optional_types.cc: optional functions use overload_id constants derived from checker/optional.cc declarations. - extensions/: extension registrations for strings, math, sets, lists, regex, encoders, formatting, and comprehensions_v2 are annotated with their checker-declared overload_id constants. - common/standard_definitions.h: add missing kBoolToInt constant. - checker/standard_library.cc: add missing bool-to-int overload decl. - checker/optional.cc: add unwrap/unwrapOpt declarations. Functions where the checker declaration count does not match the runtime registration count (e.g. container membership operators, list.sort) intentionally omit overload_id to preserve runtime dynamic matching.
ff56407 to
c84e883
Compare
|
Hi @jnthntatum, cc @jcking — I’ve rebased this PR onto the latest master and force-pushed the updated commits. The changes remain separated into two commits:
All tests pass locally with The Cloud Build check currently reports “Needs /gcbrun from a collaborator,” and the workflow security scan is awaiting maintainer approval. Could a collaborator help trigger these checks? Following your guidance in #1530, this implementation uses plan-time overload selection and registry metadata without changing the function invocation interface. I’d appreciate feedback on whether the current approach and commit boundaries are suitable for review and Piper synchronization, or whether smaller dependent PRs would be preferable. Two compatibility points I’d particularly like to confirm:
I’m happy to coordinate the integration steps here or in #1484. Thanks for your time! |
Fixes #1484.
This is a follow-up to #1530, rewritten around the approach suggested in that discussion.
The previous version tried to pass overload information through the runtime function invocation path. That made the overridable function interface harder to keep compatible. This version takes a different route:
FlatExprBuilderuses the checker’soverload_idinformation to narrow the candidate set ahead of time, and the runtime registry records which overload ID each implementation handles.This PR is a bit large because the registry, planner, runtime dispatch, standard function registrations, and tests need to move together to keep the tree green. If this is too large to review or sync comfortably, I am happy to discuss how to split it into smaller reviewable pieces.
What changed
This PR adds
overload_id-aware function overload resolution for checked expressions:FunctionDescriptorcan carry anoverload_idand fullcel::Typeinformation.FunctionRegistrycan look up static and lazy overloads by overload ID.ActivationInterfacecan look up activation-provided overloads by overload ID.FlatExprBuilderreads overload IDs from the checked expressionreference_mapand prefers the overloads selected by the checker.This PR also annotates standard library, optional types, and extension runtime function registrations with the overload IDs declared by the checker.
Why
Today, checked expressions can contain more precise overload information than the runtime normally uses during dispatch. For example, the checker may distinguish overloads such as
list<int>andlist<string>, while the default runtime matching path only sees both arguments ascel::Kind::kList.This change uses a two-part approach:
overload_iddata narrows the candidate set to the overloads selected by the checker.enable_type_level_overloadcan enable type-level verification for cases where precise container element/key/value types matter.The
overload_idpath also helps with empty containers, where runtime values may not carry enough element/key/value type information to disambiguate overloads on their own.Compatibility
The main compatibility note is
FunctionDescriptor:FunctionDescriptor::types()now returnsconst std::vector<cel::Type>&.FunctionDescriptor::kinds()returns the previous kind-level view:const std::vector<cel::Kind>&.Apart from that API adjustment, the runtime behavior is intended to stay compatible by default:
overload_id-based candidate filtering is used for checked expressions whenreference_mapoverload information is available.RuntimeOptions::enable_type_level_overloaddefaults tofalse.RuntimeOptions::enable_type_level_overloadis set totrue.