checker: accept inline fn literal for fn-type alias param in generic fn (#28181) - #28191
checker: accept inline fn literal for fn-type alias param in generic fn (#28181)#28191waterWang wants to merge 54 commits into
Conversation
|
@codex review |
![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: e960ed8d01
ℹ️ 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".
| } else if clean.starts_with('shared ') { | ||
| clean = clean[7..].trim_space() |
There was a problem hiding this comment.
When a specialized generic call compares callbacks whose parameter texts differ only by shared (for example, fn(shared State) versus fn(State)), this branch makes the signatures equal. A shared parameter has distinct calling semantics: callers pass shared storage explicitly and codegen uses the shared-parameter metadata to handle its synchronized wrapper. Accepting the non-shared callback here can therefore suppress the intended type error and leave codegen calling it with an incompatible representation; retain and compare the shared mode rather than dropping it.
Useful? React with 👍 / 👎.
|
Replayed this cleanly onto current Focused results after adding a final newline to the new test file locally:
There is one mechanical blocker in the submitted commit: |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23e60b5bf7
ℹ️ 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 clean.starts_with('mut ') { | ||
| clean = '&' + clean[4..].trim_space() | ||
| } | ||
| return clean.replace(' ', '') |
There was a problem hiding this comment.
When a specialized generic expects a callback parameter such as chan foo.Item, an actual callback taking the unrelated type chanfoo.Item is treated as compatible because removing every space maps both spellings to chanfoo.Item. This can bypass the monomorphization-time type check when the generic body could not be validated concretely earlier, allowing an incompatible callback representation into codegen; normalize formatting around function syntax without deleting meaningful spaces in compound types.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00fd0e6: signature normalization now removes whitespace only around type punctuation while retaining identifier-token boundaries such as chan foo.Item; unit coverage distinguishes it from chanfoo.Item.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00fd0e6a5a
ℹ️ 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 clean.starts_with('mut ') { | ||
| clean = '&' + clean[4..].trim_space() |
There was a problem hiding this comment.
When a generic callback parameter mut T is specialized with T = &Item, this blindly normalizes the resulting mut &Item to &&Item. The canonical type logic in fn_compatible_param_type deliberately adds a pointer only when the parameter is not already a pointer (vlib/v3/types/checker_tail.v:15444-15451), so mut &Item has the effective parameter type &Item, not &&Item. As a result, deferred specialization validation can accept an actual fn(&&Item) callback for an expected fn(mut &Item), allowing an incompatible callback signature through; preserve an existing leading & when normalizing mut.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5c4f0711c
ℹ️ 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 !clean.starts_with('&') { | ||
| clean = '&' + clean |
There was a problem hiding this comment.
In a deferred generic specialization involving voidptr, byteptr, or charptr, this guard canonicalizes mut voidptr to &voidptr, causing fn_type_texts_signature_compatible('fn(&voidptr)', 'fn(mut voidptr)') to return true even though the former takes a double pointer. These builtins are already represented as types.Pointer, and fn_compatible_param_type intentionally avoids adding another pointer to them, so this can admit an ABI-incompatible callback. The fresh evidence beyond the earlier mut &Item issue is that the revised guard recognizes only a leading & and still overlooks named pointer builtins.
Useful? React with 👍 / 👎.
|
Fixed the exact-head pointer-builtin finding in
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821479d148
ℹ️ 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".
| for i in 0 .. actual_params.len { | ||
| if normalize_fn_param_text(actual_params[i]) != normalize_fn_param_text(expected_params[i]) { | ||
| return false |
There was a problem hiding this comment.
When an inline callback has a shared parameter, this comparison still cannot validate its mode. Fresh evidence in the current head is that lift_fn_literal reconstructs the callback signature from param_type.name() at fn.v:10122-10131, while parsing shared T strips the modifier at checker_tail_stmt.v:13719-13720; consequently fn (shared value State) reaches this check as fn(State). A non-shared alias can therefore accept the shared callback through the earlier text-equality path, while a matching fn(shared State) alias is rejected, so the literal's shared metadata must be retained or inspected separately during compatibility checking.
Useful? React with 👍 / 👎.
|
Fixed the exact-head shared callback review finding by retaining shared mode while lifting function-literal parameters, preserving shared-bearing specialized signatures, and comparing canonical signatures without discarding parameter names or mut-pointer semantics. Added a generic-context regression proving a shared alias accepts the matching literal and a non-shared alias rejects it. Validation: targeted shared-mode regression passed; full type_checker_errors_test.v passed; fn_test.v passed; compiler_errors passed (1619 passed, 1 skipped); formatting and git diff --check passed. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71300c92b
ℹ️ 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".
| is_mut = true | ||
| clean = clean[4..].trim_space() | ||
| } | ||
| clean = generic_fn_type_param_payload(clean) |
There was a problem hiding this comment.
const_ callback parameter metadata
When a generic call passes an inline callback such as fn (event &C.native_event) to an alias declared as fn (const_event &C.native_event), this payload extraction removes const_event and makes both signatures normalize to &C.native_event. That prefix is not merely a parameter name: c_abi_fn_param_type in vlib/v3/types/checker_tail_stmt.v uses it to emit a const pointer, so accepting the non-const callback produces an incompatible C function-pointer assignment and can fail -cstrict builds. Preserve this C-ABI qualifier while discarding ordinary parameter names.
Useful? React with 👍 / 👎.
|
Fixed the latest exact-head finding in 7e62b77. Function-signature normalization now preserves the const_ pointer-parameter qualifier while discarding ordinary parameter names. Generic receiver validation also compares the source fn literal C ABI signature against the expected alias before fn-literal lifting can erase that metadata. Regression coverage verifies matching const callback literals are accepted, non-const literals are rejected in generic specialization, and normalized signature comparisons distinguish the qualifier. Validation:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e62b7794c
ℹ️ 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 != .fn_literal { | ||
| return true | ||
| } | ||
| expected_abi := t.tc.c_abi_fn_ptr_type_for_type_text(expected_type) or { return true } |
There was a problem hiding this comment.
When the expected alias is an ordinary fn (event &C.native_event) but the inline literal uses fn (const_event &C.native_event), this lookup finds no expected C-ABI metadata and returns true without inspecting the literal. The fresh evidence in the current head is this one-sided early return: lift_fn_literal subsequently drops the literal's const_ parameter name, so the textual fallback also treats the signatures as identical and can emit an incompatible const-taking function-pointer assignment under -cstrict. Inspect both signatures even when only the actual callback carries ABI metadata.
Useful? React with 👍 / 👎.
| is_c_abi_const = true | ||
| clean = tail | ||
| } else { | ||
| clean = generic_fn_type_param_payload(clean) |
There was a problem hiding this comment.
When a callback alias contains another function type with named parameters, such as fn (cb fn (value Item)), the inline literal is stringified as fn (fn (Item)), but this only removes the outer parameter name. Because generic_top_level_space_index deliberately ignores the space inside the nested fn, the expected side remains fn(value Item) and the otherwise identical signatures compare unequal, reintroducing a spurious cannot use error during generic specialization. Recursively normalize parameters of nested function types as well.
Useful? React with 👍 / 👎.
|
Fixed both latest exact-head findings in 84b49f9:
Regression coverage includes the reverse const mismatch and a generic nested callback alias whose inner parameter is named. Validation:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84b49f92ca
ℹ️ 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".
| fn_params, _ := fn_type_text_parts(qualified) or { []string{}, '' } | ||
| fn_has_shared_param := fn_params.any(it.trim_space().starts_with('shared ')) |
There was a problem hiding this comment.
When a specialized signature contains a nested callback such as fn (cb fn (shared value State)), the sole outer parameter starts with cb, so this check does not detect the nested shared mode. The subsequent parse/name round trip stores it as fn(fn(State)) because types.FnType has no shared-parameter metadata, which can make matching nested callbacks fail validation or incompatible callbacks use the wrong shared representation. Recursively inspect nested function parameters and return types before allowing the round trip.
Useful? React with 👍 / 👎.
| if (actual.starts_with('fn(') || actual.starts_with('fn (')) && (expected.starts_with('fn(') | ||
| || expected.starts_with('fn (')) && fn_type_texts_signature_compatible(actual, expected) { | ||
| return true |
There was a problem hiding this comment.
When validate_specialized_fn_field_call checks an inline callback passed through a function-valued field, it falls back to this helper after resolved_receiver_arg_compatible returns false for a C-ABI mismatch. For example, with type Event = C.native_event, an expected fn (event &Event) has a const C pointer while an actual fn (mut event Event) has a mutable pointer, yet this normalization maps both parameters to &Event and returns true. That overrides the explicit ABI rejection and can emit an incompatible function-pointer call in specialized generic code; this fallback must also honor the C-ABI comparison.
Useful? React with 👍 / 👎.
|
Fixed both latest exact-head findings in bda24ad:
Regression coverage includes matching and incompatible nested shared callbacks plus a generic function-field call with an incompatible mutable C-alias callback. Validation:
|
|
Fixed callback modes in struct literals in 25bcd56:
Validation:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25bcd56243
ℹ️ 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 container_compatible := t.fn_literal_container_modes_compatible(arg_id, expected_type) { | ||
| return container_compatible |
There was a problem hiding this comment.
When specialization validates a container literal, this early return replaces the ordinary actual_type versus expected_type check with only the recursive mode scan. In a generic body such as fn invoke[T](s T) { s.take(Foo{}) }, specializing T to a receiver whose take method expects int now accepts the Foo{} argument because the struct branch returns true regardless of its expected outer type, allowing invalid code to reach C generation. Run the mode scan in addition to, rather than instead of, the aggregate's normal type compatibility check.
Useful? React with 👍 / 👎.
| if field.kind != .field_init || field.children_count == 0 { | ||
| continue | ||
| } | ||
| field_type := t.lookup_struct_field_type(struct_type, field.value) or { continue } |
There was a problem hiding this comment.
When a callback field is initialized positionally, as in Options{fn (const_event &C.native_event) {}}, its field_init has an empty field.value; this lookup therefore fails and continue skips ABI-mode validation. A positional const-qualified literal can consequently initialize a plain callback field during generic specialization, despite producing an incompatible C function pointer. Resolve empty field names by their declaration index before validating the value.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
| if has_source_only_mode { | ||
| return qualified |
There was a problem hiding this comment.
When an imported generic function declares a callback using a module-local type, such as module m declaring fn apply[T](cb fn (const_event &Event)), this early return preserves the source-only mode before the parse/name round trip that would qualify Event as m.Event. Because qualify_specialized_signature_type_text leaves ordinary module-local names bare, validation from a caller with its own Event resolves the expected ABI against the caller type and rejects a matching fn (const_event &m.Event). Qualify declaration-local names while retaining the source mode.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
Fixed the latest Codex findings in
Tests: focused callback container group; focused declaration-module alias group; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad2192f214
ℹ️ 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".
| } | ||
| actual_value := actual_abi or { return false } | ||
| expected_value := expected_abi or { return false } | ||
| return actual_value == expected_value |
There was a problem hiding this comment.
When a specialized generic body passes a matching bare callback literal to an optional parameter such as handler ?ConstHandler, V's normal T-to-?T argument promotion should accept it. Here the literal produces fn_ptr:... while the expected type produces ?(fn_ptr:...), so this equality returns false before the optional compatibility branch at line 12490 can run, emitting a spurious cannot use error. Compare the literal against the option's base callback ABI before applying the wrapper.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
| } | ||
| node := t.a.nodes[int(arg_id)] | ||
| match node.kind { | ||
| .paren, .cast_expr, .expr_stmt { |
There was a problem hiding this comment.
When a specialized generic call passes an explicitly boxed callback such as accept(Any(fn (const_event &C.native_event) {})), descending through this .cast_expr exposes the inner ABI-qualified literal even though the argument's type is now the empty interface Any. The subsequent check finds an actual ABI but no expected ABI and rejects the call, despite the existing empty-interface behavior demonstrated in vlib/v/tests/casts/cast_to_empty_interface_test.v:35-44. Stop ABI comparison at casts whose target is not callback-shaped.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
Fixed the latest Codex wrapper findings in
Tests (each with isolated PR-specific TMPDIR/VTMP): focused wrapper group; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a69b01f918
ℹ️ 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".
| for clean.starts_with('?') || clean.starts_with('!') { | ||
| clean = clean[1..].trim_space() | ||
| } | ||
| if t.is_fn_pointer_type_name(t.normalize_type_alias(clean)) { |
There was a problem hiding this comment.
When a generic specialization passes a cast such as Handlers([fn (const_event &C.native_event) {}]), where type Handlers = []PlainHandler, normalizing the cast target produces an array type, so this direct-function-only predicate returns false. The collector then stops at the cast, the container validator never sees the array literal, and the equal outer alias types allow the const-qualified callback into a plain function-pointer array. Unwrap casts whose targets contain callbacks and validate their operands against the corresponding container element types.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
Fixed the callback-container cast finding in
Tests (isolated PR-specific TMPDIR/VTMP): focused six-case wrapper/cast group; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6c8820e9a
ℹ️ 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 == .postfix && node.children_count == 1 { | ||
| return t.fn_literal_container_modes_compatible(t.a.child(&node, 0), expected_type) | ||
| } |
There was a problem hiding this comment.
When a specialized generic receiver call passes a parenthesized container such as r.apply(([fn (const_event &C.native_event) {}])) to []PlainHandler, the argument remains a .paren node here, so this helper returns none. The source collector unwraps the parentheses but then stops at the array literal, leaving no ABI signature to compare; the erased semantic array type is therefore accepted despite containing an incompatible C function pointer. Unwrap .paren/.expr_stmt before dispatching to the container branches.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
| element_expected := if source_expected.starts_with('[]') { | ||
| source_expected[2..] | ||
| } else if t.is_fixed_array_type(source_expected) { | ||
| fixed_array_elem_type(source_expected) | ||
| } else if normalized_expected.starts_with('[]') { |
There was a problem hiding this comment.
When a specialized generic receiver method expects ?[]PlainHandler and receives a bare array literal containing fn (const_event &C.native_event) {}, both source_expected and normalized_expected still start with ?, so none of these array element-type branches match and the helper returns none. The outer optional-promotion check later accepts the erased []fn(&C.native_event) type as the option's base, allowing the incompatible callback through; unwrap ? (and analogous promotable wrappers) before deriving the container element type.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
Fixed both exact-head callback-container findings in
Validation passed: focused V3 type-checker regression file; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ca1c64e67
ℹ️ 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".
| map_expected) { | ||
| return false | ||
| } | ||
| i++ |
There was a problem hiding this comment.
For a map-update literal such as consume[int]({...base}), the flat map AST reserves a key/value pair for the spread, as confirmed by lower_map_init_to_runtime setting start_i = 2; advancing only once leaves the placeholder value to be processed as the next key, so the following incomplete-pair check returns false and rejects an otherwise valid generic call. Fresh evidence beyond the earlier spread report is that the new special case still disagrees with every existing map walker, which advances by two for a spread entry.
Useful? React with 👍 / 👎.
|
Fixed the exact-head map-spread finding in Validation passed: exact V3 check fixture; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c0522760
ℹ️ 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".
| abi_expected_type := if expected_type.starts_with('?') { | ||
| expected_type[1..] |
There was a problem hiding this comment.
When a specialized generic call passes a bare callback literal to a result-wrapped parameter such as handler !ConstHandler, normal callback-to-!fn promotion should accept it (and is already exercised by vlib/v3/tests/fn_value_decl_type_test.v), but this strips only ?. The literal therefore yields fn_ptr:... while the expected signature remains !(fn_ptr:...), causing rejection before the result-promotion branch in resolved_receiver_arg_compatible can run. Unwrap ! here just as the container validator does.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
| for normalized_expected.starts_with('?') || normalized_expected.starts_with('!') { | ||
| normalized_expected = normalized_expected[1..].trim_space() | ||
| } | ||
| if node.kind in [.array_literal, .array_init] { |
There was a problem hiding this comment.
When a specialized generic call receives if flag { [fn (const_event &C.native_event) {}] } else { []PlainHandler{} } for a []PlainHandler parameter, the outer node is .if_expr, so this literal-only container dispatch returns none. The source collector traverses the branches but stops at each array literal, leaving no ABI signature to compare; the erased semantic array type is then accepted despite containing an incompatible function pointer. Recurse through callback-valued if/match branches before dispatching on the aggregate literal.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
Fixed both exact-head wrapped-callback findings in
Validation passed: four exact V3 good/bad fixtures; |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02bcbd39a6
ℹ️ 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 != .fn_literal { | ||
| return |
There was a problem hiding this comment.
In a specialized generic receiver call, passing a named callback rather than an inline literal bypasses the new ABI validation because this collector immediately returns for .ident nodes. For example, a local variable or named function initialized/declared as fn (const_event &C.native_event) can be passed to a receiver parameter expecting plain fn (event &C.native_event); the resolved FnType has erased const_, so the later type comparison accepts it and emits an incompatible non-const function pointer. Preserve the source signature on callback-valued identifiers or trace them to their declaration before returning.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e739bf11e9
ℹ️ 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".
| else {} | ||
| } | ||
| if node.kind != .fn_literal { | ||
| return |
There was a problem hiding this comment.
When a specialized generic call passes a method value such as Handler{}.handle, where handle takes const_event &C.native_event, the argument is a .selector rather than an .ident. This collector therefore falls through and returns without recording its source signature, even though the checker already recognizes selector method values; the erased semantic function type can then match a plain callback parameter and emit an incompatible C function pointer. Handle method-value selectors by resolving their declaration and retaining its source parameter modes.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
| } else if normalized_expected.starts_with('[]') { | ||
| array_expected = normalized_expected | ||
| normalized_expected[2..] |
There was a problem hiding this comment.
When the expected parameter is an alias such as type ConstHandlers = []fn (const_event &C.native_event), source_expected is not syntactically an array, so this branch derives its element from normalized_expected. That normalization uses the semantic FnType, which has discarded the const_ parameter name, causing a matching const-qualified literal to be compared against a plain callback and rejected (while a plain literal can be accepted). Expand the alias using its source ABI metadata before recursively validating the elements.
AGENTS.md reference: AGENTS.md:L320-L322
Useful? React with 👍 / 👎.
3 participants
Fixes #28181
Background
An inline
fnliteral passed to a vlib method that takes a fn-type alias as the parameter is rejected by the V3 type checker with a spurious error when the call site is inside a generic function:The same call outside a generic function compiles fine.
Root cause
Generic function bodies are validated a second time during monomorphization (
receiver_call_validate_args→resolved_receiver_arg_compatibleinvlib/v3/transform/fn.v). That path compares the argument type to the parameter type as strings:fn (&Request, &ResponseWriter)(nomutmode on the parameter,fn (with a space)fn(&Request, mut ResponseWriter)(mutparameter mode, no space)actual != expectedand no fn-signature comparison existed, so the pair was rejected even though both spellings denote the identical function type.Fix
Added
fn_type_texts_signature_compatible(plus thenormalize_fn_param_texthelper) and wired it into bothresolved_receiver_arg_compatibleandfn_field_arg_compatible:fn_type_text_partsmut T→&T(amutparameter denotes reference passing, and a fn literal stringifies it as&T),sharedstripped, interior whitespace removedTest
vlib/v/tests/generics/generic_fn_literal_matches_fn_type_alias_issue_28181_test.v— regression test that reproduces #28181 exactly (inline fn literal as a fn-type alias argument insidefn startup[T]()). Verified failing without the fix (type checker found 1 error(s): cannot use \fn (&Request, &ResponseWriter)` as argument 2 to `r.register`; expected `Handler``) and passing with it. All existing generics/fn-type tests continue to pass.