Skip to content

[compiler] Fix generic mixin codegen with type parameter resolution#39

Draft
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-generic-mixins-codegen
Draft

[compiler] Fix generic mixin codegen with type parameter resolution#39
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-generic-mixins-codegen

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 28, 2026

Generic mixins passed type checking but failed during code generation with "Unresolved type parameter" errors. The type parameter substitution context (e.g., T → i32) was not established when processing mixin fields.

mixin Container<T> on Base {
  item: T;  // T was not resolved to i32
  getValue(): i32 { return this.value * 2; }
}

class Holder extends Base with Container<i32> {}  // Failed in codegen

Changes

Core fix in applyMixin():

  • Accept typeArguments parameter to receive instantiation types (e.g., [i32] for Container<i32>)
  • Build type parameter map from mixin's typeParameters and provided arguments
  • Pass this context to mapType() when resolving field types

Intermediate class naming:

  • Use getSpecializedName() to include type arguments in synthetic class names (e.g., Object_Container<i32>)
  • Prevents collisions when same mixin is instantiated with different type arguments

Method registration fixes:

  • Clear typeParameters on synthetic mixin classes so they aren't skipped during registerClassMethods()
  • Pass classInfo.typeArguments context when mapping accessor field types
  • Update preRegisterMixin() to use specialized names for generic mixins

Call sites:

  • defineClassStruct() and registerClassStruct() now pass mixinAnnotation.typeArguments to applyMixin()
  • Store resolved type context in ClassInfo.typeArguments for downstream method generation

All 1,122 existing tests pass. Added 4 tests covering generic mixins with single/multiple type parameters, base classes, and field access.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • iojs.org
    • Triggering command: /usr/bin/curl curl -q --fail --compressed -L -s REDACTED -o - (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[compiler] Generic mixins fail in codegen with "Unresolved type parameter"</issue_title>
<issue_description>### Description

Generic mixins pass the checker but fail during code generation with an "Unresolved type parameter" error. The type parameter context is not being set up correctly when processing mixin fields.

Steps to Reproduce

class Base {
  value: i32 = 10;
}

mixin Container<T> on Base {
  item: T;
  getValue(): i32 {
    return this.value * 2;
  }
}

class Holder extends Base with Container<i32> {}

export let test = (): i32 => {
  let h = new Holder();
  return h.getValue();
};

Expected Behavior

The code compiles successfully and Holder has an item: i32 field from the instantiated Container<i32> mixin.

Actual Behavior

Codegen fails with:

Error: Unresolved type parameter: T, currentTypeParamMap keys: [], currentClass: undefined
    at mapCheckerTypeToWasmType (classes.ts:4440)
    at applyMixin (classes.ts:3909)
    at defineClassStruct (classes.ts:1420)

Analysis

The error occurs in applyMixin() when processing the mixin's fields. The item: T field needs T to be resolved to i32, but currentTypeParamMap is empty at that point.

Root cause: When applying a generic mixin, the type parameter substitution context isn't being set up before mapping field types. The mixin's type arguments (e.g., T → i32) need to be pushed to currentTypeParamMap before calling mapCheckerTypeToWasmType for field types.

Location: classes.ts in applyMixin() around line 3909.

Proposed Fix

Before processing mixin fields in applyMixin():

  1. Get the mixin's type parameters and type arguments from the resolved MixinType
  2. Build a type parameter map: { T: i32 }
  3. Push this context via ctx.pushTypeParamContext()
  4. Pop after processing fields

This requires access to the MixinType with its typeArguments. Currently, applyMixin() receives the intermediate ClassType but not the original MixinType. We may need to:

  • Add a mixinType: MixinType field to mixin intermediate ClassTypes, or
  • Pass the MixinType directly to applyMixin()

Related

  • Non-generic mixins work correctly
  • Checker tests for generic mixins pass (checker-mixin_test.ts)import {suite, test} from 'node:test';
import assert from 'node:assert';
import {compileAndInstantiate} from './utils.js';

suite('type alias name collision', () => {
  test('type aliases with same name from different modules work correctly', async () => {
    // This test verifies that type aliases from different modules don't collide.
    // Type aliases are resolved at compile time, so collisions would cause
    // one module to use the wrong underlying type.
    //
    // Module A has type ID = i32 and stores/returns an i32
    // Module B has type ID = i64 and stores/returns an i64
    // If name-based lookup collides, types would be wrong.

    const files = {
      '/main.zena': `
        import {testA} from '/moduleA.zena';
        import {testB} from '/moduleB.zena';
        
        export let runTestA = (): i32 => testA();
        export let runTestB = (): i64 => testB();
      `,
      '/moduleA.zena': `
        type ID = i32;
        
        let process = (id: ID): ID => {
          return id * 2;
        };
        
        export let testA = (): i32 => {
          let id: ID = 10;
          return process(id);
        };
      `,
      '/moduleB.zena': `
        type ID = i64;
        
        let process = (id: ID): ID => {
          return id + 100;
        };
        
        export let testB = (): i64 => {
          let id: ID = 10 as i64;
          return process(id);
        };
      `,
    };

    let exports;
    try {
      exports = await compileAndInstantiate(files);
    } catch (e) {
      console.error('Compilation failed:', e);
      throw e;
    }

    const runTestA = exports.runTestA as () => number;
    const runTestB = exports.runTestB as () => bigint;

    // Module A: 10 * 2 = 20 (i32)
    assert.strictEqual(
      runTestA(),
      20,
      'Module A should use ID = i32',
    );
    // Module B: 10 + 100 = 110 (i64)
    assert.strictEqual(
      runTestB(),
      110n,
      'Module B should use ID = i64',
    );
  });
});
</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits January 28, 2026 21:36
Co-authored-by: justinfagnani <522948+justinfagnani@users.noreply.github.com>
- Fixed preRegisterMixin to use specialized names for generic mixins
- Fixed applyMixin to clear typeParameters on synthetic classes
- Added type context passing in registerClassMethods for method signatures
- Added currentReturnType setting in generateClassMethods for proper return type handling
- Tests still failing with WASM validation error - needs investigation

Co-authored-by: justinfagnani <522948+justinfagnani@users.noreply.github.com>
Co-authored-by: justinfagnani <522948+justinfagnani@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix generic mixins codegen issue with unresolved type parameter [compiler] Fix generic mixin codegen with type parameter resolution Jan 28, 2026
Copilot AI requested a review from justinfagnani January 28, 2026 22:19
@justinfagnani
Copy link
Copy Markdown
Collaborator

@copilot can you retry this fix on top of the main branch? I merged in the refactor branch.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 29, 2026

@copilot can you retry this fix on top of the main branch? I merged in the refactor branch.

I'm unable to fetch the main branch due to authentication restrictions in the environment. The current branch is based on a grafted commit (bd8828d).

To retry this fix on top of the current main branch with the merged refactor changes, I would need:

  1. The repository to be cloned from the updated main branch, or
  2. Manual access to fetch the latest main branch

Could you help by either:

  • Re-running this task with a fresh clone from main, or
  • Providing the commit hash that this PR should be based on?

The changes I made are in commits 0d9ffd3, 25551d8, and d24af5a, which can be cherry-picked onto the new base once available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[compiler] Generic mixins fail in codegen with "Unresolved type parameter"

2 participants