cgen: fix generic multi-return interface cast with stale checker types - #27610
Conversation
Two issues caused wrong concrete type to be used in interface cast when multiple generic instantiations existed in one compilation unit: 1. Type resolution caches (unwrap_generic_cache, resolved_scope_var_type_cache) persisted across generic specializations in post_process_generic_fns_for_files, causing the second specialization to use cached results from the first. 2. return_stmt used checker-mutated ret_expr_types directly for multi-return struct field generation. In a generic context, the checker mutates AST types to the last processed concrete type, so all specializations would use the same (wrong) concrete type. Fix: - fn.v: clear_type_resolution_caches() between specializations - cgen.v return_stmt: resolve Ident via resolved_scope_var_type_uncached and non-Ident via resolved_expr_type in generic contexts Add test: generic_multi_return_interface_cast_test.v
|
@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: 58c1d70e18
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
When a generic multi-return returns a mut generic parameter as an
interface (e.g. fn get[T](mut x T) (Getter, int) { return x, 0 }),
resolved_scope_var_type_uncached resolved x to the lowered pointer
type (&Concrete). return_stmt had already emitted * for auto-deref
vars, so expr_with_cast used the pointer-to-interface helper while
the argument was already dereferenced, producing invalid C.
Fix by dereferencing the resolved type when is_auto_deref_var().
Additionally, the * auto-deref wrapped the entire interface cast call
instead of being inside it; use a synthetic PrefixExpr{.op: .mul} so
* is emitted inside the cast expression.
tests: add multi-return mut param interface cast regression
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d51a26a55b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ace cast
When a returned auto-deref var is a mut generic parameter whose concrete
type T is itself a pointer (e.g. fn f[T](mut x T) (I, int) { return x, 0 }
with T = &Impl), the synthetic PrefixExpr{op: .mul} from the previous
fix hits the special handling for mut generic pointer params in expr(),
which emits an additional dereference. The interface caster then receives
**x even though resolved_ret_type was only dereferenced to &Impl,
producing invalid C.
Fix by adding an is_auto_deref_synthetic flag to Gen. Set it before
creating the synthetic PrefixExpr so the extra * logic in the generic
prefix handler is suppressed.
tests: add pointer generic param multi-return interface cast regression
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Add test_generic_as_cast_then_multi_return_interface to cover the pattern: raw := interface_value as T; return raw, ... (multi-return with interface cast). Both generic instantiations in the same file trigger the codegen path that was broken before the fix. Co-authored-by: Jengro <avey777@outlook.com>
… both value and &Interface paths
|
@GGRei Since the last codex review, only testing code and merging upstream code have been added |
3 participants
fixed: #27637
Description
When a generic method returns a multi-return value where one value needs to be auto-cast to an interface type, the generated C code used the wrong concrete type for the interface cast function when multiple generic instantiations existed in the same compilation unit.
For example, a generic method
get()onContainer[T]returning(Interface, int)would generateI_Impl2_to_Interface(HEAP(Impl2, ...))for BOTHContainer[Impl1]andContainer[Impl2]instantiations — the first instantiation would incorrectly use the second concrete type's cast function.Root Cause
Two issues combined:
Stale type resolution caches (
fn.v):unwrap_generic_cacheandresolved_scope_var_type_cachepersisted across generic specializations inpost_process_generic_fns_for_files. Since the cache keys are based on source position (identical across instantiations), the second specialization would reuse cached results from the first.Checker-mutated AST types in
return_stmt(cgen.v): The V checker processes each generic function instantiation and mutates AST node types in-place, leaving the last processed concrete type's types in the AST.return_stmtused these mutatedret_expr_typesdirectly when generating multi-return struct fields, causing all specializations to use the same (wrong) concrete type.Changes
vlib/v/gen/c/fn.vg.clear_type_resolution_caches()after settingg.cur_concrete_typesfor each specialization inpost_process_generic_fns_for_files, ensuring each instantiation resolves types with its own concrete types.vlib/v/gen/c/cgen.vreturn_stmt, when generating multi-return struct literals in a generic context, resolve the expression type correctly:Identexpressions: useresolved_scope_var_type_uncached(preserves original generic type viageneric_typfield)SelectorExpr, etc.): useresolved_expr_type(resolves through receiver/field type chain)vlib/v/tests/generic_multi_return_interface_cast_test.v(new)x+100vsx+200). If the wrong interface cast function is used, the return value differs from the expected value, causing an assertion failure.Testing
vlib/v/tests/generic_multi_return_interface_cast_test.v— new test, passesvlib/v/tests/interfaces/interface_generic_pool_cast_multi_type_test.v— existing test, still passesvlib/v/parser/(9 tests) — all passvlib/v/checker/— passesvlib/v/slow_tests/inout/(91 compiler error tests) — 0 errorsvlib/v/tests/comptime/(169 tests) — all pass