Repository navigation
expr.ConstExpr panics if function was defined with expr.Function instead of env #809
Description
Short description: expr.ConstExpr requires that the given function was defined in the environment and does not take into account those functions defined with expr.Function.
Why it matters: This prevents usage of structs as environment in many cases, since it's impractical to define all the functions in the environment struct so we resort to add functionality with expr.Function.
Workaround: It is possible to directly define the signature in the configuration if we directly set the value for the function in the expr/conf.(*Config).ConstFns.
Limitations: a ConstExpr function can have a single signature because for each function name, we can only assign one value in ConstFns. Maybe worth creating a Feature Request issue?
Example (Go Playground)
package main
import (
"fmt"
"github.com/expr-lang/expr"
)
type MyEnv struct {
Value string
}
func identity(x any) any {
return x
}
func main() {
code := `[identity(1), identity(Value)]`
options := []expr.Option{
expr.Env(MyEnv{}),
expr.Function(
"identity",
func(params ...any) (any, error) {
return identity(params[0]), nil
},
identity,
),
expr.ConstExpr("identity"), // Mark identity func as constant expression.
}
program, err := expr.Compile(code, options...)
if err != nil {
panic(err)
}
env := MyEnv{
Value: "thing",
}
output, err := expr.Run(program, env)
if err != nil {
panic(err)
}
fmt.Println(output)
}Workaround for single-signature (Go Playground)
package main
import (
"fmt"
"reflect"
"github.com/expr-lang/expr"
"github.com/expr-lang/expr/conf"
)
type MyEnv struct {
Value string
}
func identity(x any) any {
return x
}
func main() {
code := `[identity(1), identity(Value)]`
options := []expr.Option{
expr.Env(MyEnv{}),
expr.Function(
"identity",
func(params ...any) (any, error) {
return identity(params[0]), nil
},
identity,
),
func(c *conf.Config) {
c.ConstFns["identity"] = reflect.ValueOf(identity) // Mark identity func as constant expression.
},
}
program, err := expr.Compile(code, options...)
if err != nil {
panic(err)
}
env := MyEnv{
Value: "thing",
}
output, err := expr.Run(program, env)
if err != nil {
panic(err)
}
fmt.Println(output)
}
Activity
antonmedv commentedon Jun 27, 2025
Yes, i think we need to fix it.
diegommm commentedon Jun 27, 2025
I can fix this if you want, but I think I would need to make some non-trivial changes, so I will tell you the plan before doing any work to make sure it's ok.
Global idea
Plan
ConstFnsfield of conf.Config because it implies that a single signature can be used for a function.Constto builtin.Function since we already have all the information that we need here.constExprvisitor of the optimizer use the functions from theFunctionsfield ofconf.Config. This field will be used to: (1) check if the function is marked asConst; and (2) then optimize for any of the signatures.Handling deprecation of field
ConstFnsBecause this field is exported then it's part of the public API of the package. This means that in order to keep backward-compatibility it's not possible to just delete the field because someone could be depending on it. My plan to keep backward-compatibility is:
Deprecated: <message>in the last line of the function comment (see source). Additionally, if you want to use a symbol marked as deprecated then vet will complain.constExprvisitor of the optimizer will use to know if a function is ConstExpr will be: (appears in theConstFnsmap) OR (has the newConstfield set to true inFunctions).ConstFnswill not be used though, we will instead check the slice of signatures inFunctions.Future
Since builtins are also defined as Function, then a user can also do things like
expr.ConstExpr("toJSON"). This would allowtoJSON({"name": "John", "age": 30})to be evaluated at compile time, which some people can find useful to avoid marshaling all the time the same thing every time it is evaluated.In order to have this working, all we would need to do is to reuse the same logic that we use
Functionsin theBuiltinsfields ofconf.Config.antonmedv commentedon Jun 30, 2025
But how functions from env will work?
I think this is correct solution. I like it.
Well, it is exposed but "public" API is via
expr.ConstExpr()and other helper functions. So I believe it is save to modify the conf.Config object directly.diegommm commentedon Jun 30, 2025
Currently, we call
expr.ConstExpr(name)and it checks the environment. I think it would be good to keep this function as it looks from the outside but internally it would do the following:Functions, then just set theConstfield to true.Functionsbut exists in the environment, then first callexpr.Functionfilling out the details for it, and setting theConstfield to true.Nice! So we can make the change without the other complication. I would just add it to the release notes just to make extra sure that people are warned.
diegommm commentedon Jul 16, 2025
Hey @antonmedv, I was wondering if you were able to check my last message, I just wanted to confirm it looks ok to you before doing any work.
antonmedv commentedon Jul 17, 2025
I’m on vacation) will try to check it tonight
fix: support ConstExpr with functions defined via expr.Function