fix: panic when using pointer to map as env #872
Conversation
Previously, passing a pointer to a map (e.g. `*map[string]any`) to `expr.Env()` would cause a panic because reflection methods were called on the pointer value instead of the underlying map. With this change the value is first deref'd in the config before any map operations are performed. Consequently, the `vm.Run()` method needs to dereference the env map. This ensures compatibility with the optimisations like `OpLoadFast` where the compiler determines the environment simply as `map[string]any`. Regression test added. Signed-off-by: Ville Vesilehto <[email protected]>
|
But why would |
|
|
||
| func (vm *VM) Run(program *Program, env any) (_ any, err error) { | ||
| if m, ok := env.(*map[string]any); ok && m != nil { | ||
| env = *m |
There was a problem hiding this comment.
I think Expr sloud allow to types as incoming env: struct, *struct and map[string]...
I guess this would mostly be a quality of life improvement. Tried to think of an example. Maybe you unmarshal the Also @yawnBright feel free to chime in with your use case. |
I think that's how I found this issue |
|
Let's say for now we do not support this feature. It should be struct, pointer to the struct, or a map. |
3 participants
Previously, passing a pointer to a map (e.g.
*map[string]any) toexpr.Env()would cause a panic because reflection methods were called on the pointer value instead of the underlying map. With this change the value is first deref'd in the config before any map operations are performed.Consequently, the
vm.Run()method needs to dereference the env map. This ensures compatibility with the optimisations likeOpLoadFastwhere the compiler determines the environment simply asmap[string]any.Regression test added.
Fixes #825.