Skip to content
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

Add EvalContext argument for Expression.GetType and ParamMarker.GetUserVar #53533

Closed
YangKeao opened this issue May 24, 2024 · 0 comments · Fixed by #53534
Closed

Add EvalContext argument for Expression.GetType and ParamMarker.GetUserVar #53533

YangKeao opened this issue May 24, 2024 · 0 comments · Fixed by #53534
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@YangKeao
Copy link
Member

YangKeao commented May 24, 2024

Enhancement

Ref #53485 (comment). As the actual type and internal datum depend on the context, we should explicitly pass the context through the argument, rather than store it in the ParamMarker. It'll be a big change.

It'll introduce three modification:

  1. Remove the ctx in ParamMarker.
  2. Add EvalContext argument for Expression.GetType.
  3. Add EvalContext argument for String (replace String with StringWithCtx)

The most concerning issue is that the .String() method cannot accept a parameter. We should analyze the usage of it carefully. Sometimes, returning a simple ? is enough, but it's not always good to do so. If not, we can Eval it, or provide another function to get the debug output with context 🤔 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant