checker: fix static method used as map/filter argument (#27328)#27331
Conversation
A static method used as a first-class value, e.g. `arr.map(Type.from)`, is parsed as an `ast.EnumVal`, since `Type.from` is syntactically identical to an enum value (`Color.red`). The parser only rewrites it into a function `ast.Ident` when the static method is already registered in the function table, which is not the case for cross-module references (imported modules are parsed after the call site) nor for forward references within a file. In those cases the `EnumVal` reached the backend, which emitted the static method's name as a bare value without calling it (and markused did not keep the method, so it was never generated) -> `undeclared identifier` / `unknown enum`. Resolve the value in the checker: in `array_builtin_method_call`, once the callback argument of `map`/`filter`/`any`/`all`/`count` has been resolved to a function type, rewrite the `ast.EnumVal` into a function `ast.Ident`, so the backends call it and markused keeps the static method, exactly like any other function value.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a465e6b96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if node.kind in [.map, .filter, .any, .all, .count] && node.args.len > 0 { | ||
| arg_expr := node.args[0].expr | ||
| if arg_expr is ast.EnumVal && c.table.sym(arg_expr.typ).kind == .function { | ||
| static_fn_name := '${arg_expr.enum_name}__static__${arg_expr.val}' |
There was a problem hiding this comment.
When the callback is written through a supported type alias, e.g. type MyAlias = MyStruct; nums.map(MyAlias.new), enum_val_as_static_fn() can resolve the EnumVal via the final symbol (MyStruct__static__new) and set its type to a function, but this reconstruction only looks for MyAlias__static__new. That misses the real registered function, so the expression is left as an EnumVal; markused still will not keep the static method and cgen emits a non-existent alias-named function value. Reuse the resolved function/fkey path rather than rebuilding it from enum_name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 035ae39. The alias-aware lookup that enum_val_as_static_fn already used (which checks the final/unaliased symbol) is now factored into a shared static_method_of_enum_val helper and reused at the rewrite site, taking the resolved function's real name. Added an aliased-static-method case to the regression test.
Address review feedback: when the callback is a static method reached through a type alias (`type Alias = Struct; arr.map(Alias.new)`), rebuilding the function name from the EnumVal's `enum_name` produced `Alias__static__new`, which is not a registered function, so the value was left as an EnumVal (markused dropped the method, cgen emitted an alias-named, non-existent function). Factor the alias-aware lookup used by `enum_val_as_static_fn` into a shared `static_method_of_enum_val` helper (it also checks the final/unaliased symbol) and reuse it at the rewrite site, taking the resolved function's real name.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Fixes #27328
A static method used as a first-class value — e.g.
arr.map(mtime.MTime.from)— is parsed as anast.EnumVal, becauseType.fromis syntactically identical to an enum value such asColor.red. The parser only rewrites it into a functionast.Identwhen the static method is already registered in the function table. That is not the case for:In those cases the
ast.EnumValreaches the backend, which emits the static method's name as a bare value without calling it (andmarkusednever keeps the method, so it is not generated):Fix
Resolve it in the checker. In
array_builtin_method_call, once the callback argument ofmap/filter/any/all/counthas been resolved to a function type, rewrite theast.EnumValinto a functionast.Ident. Then all existing function-value handling applies — the backends call it andmarkusedkeeps the static method, exactly like any other function value.The rewrite only triggers when the value resolves to a function type, so genuine enum values (
nums.map(Color.green)) are left untouched.Test
Added
vlib/v/tests/fns/static_method_as_map_arg_test.v, which covers the forward-reference path (map(Foo.from), the explicit-call form, andfilter). It fails on master and passes with this change.Verified no regressions:
enums/(40/40),fns/(162/162), the fullcompiler_errors_test.vharness, and the existing static-method tests all pass;nums.map(Color.green)still produces enum values.