Repository navigation
Conversation
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! |
|
Unfortunately we are unable to take such a large PR. This is due to a lot of inflight changes that will conflict. We are going to take a more gradual approach by adding There is also the question of |
…nctions Prefer checked overload IDs while validating runtime arguments, then retain ordered arity fallbacks for parse-only, unknown-ID, and legacy registrations. Kind matching remains the default. The opt-in type mode validates concrete containers and solves independently scoped registration/implementation types with per-occurrence evidence for shared generic parameters. Share runtime acceptance between static and lazy calls. Filter actual values and implementation strictness before provider ambiguity, preserve operational statuses and CEL exceptions, and avoid unnecessary container observations. Keep nonoptional opaque metadata atomic and preserve nominal struct identity. Enforce per-name overload ID uniqueness in both activations, retain old provider subclasses, and disambiguate legacy descriptor/adapter calls through Nary arities. Test the fixed runtime/API matrix and deterministic independent overlap oracle. CEL reference implementations guide overload identity, scoped generic variables, and strict evaluation. Full runtime generic/container unification is a C++ extension behind enable_type_level_overload, not reference-language equivalence.
Bind standard, optional, and extension registrations to the checker-declared overload IDs, retaining dynamic lookup where checker/runtime overload shapes do not map one to one. Add the bool-to-int and optional unwrap declarations needed for those annotations. Return the selected OptionalValue from optional.or without unwrapping it, reject nonoptional opaque fallback values when selected, and retain present-receiver short circuiting. Cover direct or/orValue boundaries and evaluator error fallbacks in both matching modes.
c84e883 to
e9c7376
Compare
|
Thanks, @jcking. I understand the concern about the PR’s size and the other changes in progress. I’m happy to split the work into smaller PRs. We can first agree on their scope and order. I’ve rebased the branch onto master and force-pushed it. This includes your For the first smaller PR, we could follow your suggestion. If the type checker selects exactly one overload, use its Later PRs could add more overload IDs to standard-library, optional, and extension functions. I understand you may combine I also checked the current
Do you already have plans for any of these steps? I’d be happy to coordinate the scope and order of the smaller PRs with you. |
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.