markused: fix generic fn instantiation dedup key collision for identical module names by Jengro777 · Pull Request #27399 · vlang/v

Jengro777 · 2026-06-09T08:23:39Z

When a V project has two or more modules that share the same name (e.g., both declare module common in different directory paths like m0a/common/ and m0b/common/), and both define struct types with identical short names, the compiler silently drops some generic function instantiations. The C compilation then fails with "implicit declaration" / "undefined symbol" errors because the generated C code calls functions that were never emitted.

This occurs because the markused walker's fn_generic_types_key function uses type_to_str(typ) (the V type name) to build deduplication keys. When two types from different same-named modules share the same short name, type_to_str can return identical strings, causing the walker to treat subsequent generic instantiations as duplicates and skip them.

Root Cause

In vlib/v/markused/walker.v, the fn_generic_types_key function:

fn (w &Walker) fn_generic_types_key(fkey string, concrete_types []ast.Type) string {
    mut parts := []string{cap: concrete_types.len}
    for typ in concrete_types {
        parts << w.table.type_to_str(typ)  // ← uses V type name
    }
    return '${fkey}:${parts.join('|')}'
}

type_to_str returns the V-level type name string. Types from modules with the same declared name but different import paths could produce the same key string, causing the dedup check used_fn_generic_type_keys[key] to incorrectly skip needed instantiations.

Fix

Use typ.str() — the underlying Type ID integer — instead of type_to_str:

-   parts << w.table.type_to_str(typ)
+   parts << typ.str()

Type IDs are unique across all type symbols, guaranteeing that every concrete type produces a distinct key regardless of overlapping module paths or short names.

Reproduction

A test with 6 module pairs (12 modules total, all named common, each defining the same 3 struct type names, each calling the same generic API function) consistently fails before this fix:

Before	After
30/36 C functions generated	36/36 C functions generated
6 "undefined symbol" link errors	Clean compile, outputs "ok"

Tests

Reproduction case compiles and runs successfully
examples/hello_world.v compiles and runs
Relevant compiler test suites pass

Changed Files

vlib/v/markused/walker.v — 1 line change

Fixes #27398.

…ate module names When multiple modules share the same name (e.g. both declared 'module common' in different directories) and define structs with identical short names, the walker's fn_generic_types_key function used type_to_str() for deduplication. Since type_to_str returns the V type name, types from different modules with the same short name produced identical keys, causing subsequent generic instantiations to be silently dropped. Fix by using typ.str() (the underlying Type ID integer) instead of type_to_str, guaranteeing unique keys per concrete type regardless of module path. Fixes vlang#27398.

chatgpt-codex-connector · 2026-06-09T08:23:43Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

Jengro777 · 2026-06-09T11:56:34Z

This can solve the problem, but it may make the generated c unreadable

GGRei · 2026-06-09T17:01:44Z

About the generated C readability concern and from what I can see, this key is an internal markused tracking/dedup key for generic instantiations. It does not feed into the generated C symbol text. The C names are still produced by the normal Cgen naming path, so this change should not make generated C names unreadable. ^

For the fix itself, maybe a slightly clearer version would be to use u32(typ).str() instead of typ.str(), since the goal here is to use the packed ast.Type identity, not the debug string formatting. :)

Also, if possible, adding a small regression test?

Nice catch for the bug/fix.

Jengro777 · 2026-06-09T21:59:20Z

About the generated C readability concern and from what I can see, this key is an internal markused tracking/dedup key for generic instantiations. It does not feed into the generated C symbol text. The C names are still produced by the normal Cgen naming path, so this change should not make generated C names unreadable. ^

For the fix itself, maybe a slightly clearer version would be to use u32(typ).str() instead of typ.str(), since the goal here is to use the packed ast.Type identity, not the debug string formatting. :)

Also, if possible, adding a small regression test?

Nice catch for the bug/fix.

thankyou
I'll fix it, but adding tests doesn't seem appropriate to me because this is a hash collision issue and it behaves differently on different machines

GGRei · 2026-06-09T22:36:20Z

Yes but the failure may vary between machines in terms of which specialization is missing but the expected behavior is not machine-dependent. No required generic specialization should be silently dropped.

So the test does not need to check the exact missing symbol. It only needs to build a reduced project with the same pattern and assert that it compiles/runs.

Of course, this is only my personal opinion, so feel free to handle it however you think is best :)

Jengro777 · 2026-06-10T00:01:06Z

Yes but the failure may vary between machines in terms of which specialization is missing but the expected behavior is not machine-dependent. No required generic specialization should be silently dropped.

So the test does not need to check the exact missing symbol. It only needs to build a reduced project with the same pattern and assert that it compiles/runs.

Of course, this is only my personal opinion, so feel free to handle it however you think is best :)

thank you
I need to think about this, because even on my machine, I have to run it multiple times to consistently reproduce the bug. Maybe we could create a bigger example for testing, But it's not suitable to put it in the test file

…ate module names When multiple modules share the same name (e.g., both declare 'module common' in different directories) and define structs with identical short names, the walker's fn_generic_types_key used type_to_str() for deduplication. Type names from different same-named modules could produce identical keys, causing subsequent generic instantiations to be silently dropped. Fix by using u32(typ).str() (the raw Type ID) instead of type_to_str, guaranteeing unique keys per concrete type regardless of module path. Adds project_issue_27398 with 100 module pairs to reliably reproduce the bug. Fixes vlang#27398.

Jengro777 · 2026-06-10T04:44:52Z

In order to reproduce the bug on every machine as much as possible, the test files are deliberately written here

Please review

GGRei · 2026-06-10T05:58:04Z

The test fixture feels a bit large.

Since projects_that_should_compile_test.v already creates temporary projects in other tests, maybe this case could generate the duplicate-module project under os.vtmp_dir() with a deterministic loop and a fixed pair count, for example 20/24/32 pairs, then run it with -skip-unused run flag and assert ok.

That would keep the compile/run regression strong, while avoiding ~5k LOC of mechanical fixture files.

If CI proves the pair count is too low, it can be raised by changing one constant instead of adding more files?

24 pairs seems like a good compromise, I tested it locally.

@vexe

Replace the large static test fixture project_issue_27398/ (202 files, ~5k LOC) with a standalone test that generates the duplicate-module project at runtime using os.write_file in os.vtmp_dir(). - Remove project_issue_27398/ static fixture directory - Remove the associated test from projects_that_should_compile_test.v - Add generic_fn_dedup_collision_issue_27398_test.v: generates 24 module pairs, compiles with -skip-unused run and asserts output is 'ok' - Remove the old test function from projects_that_should_compile_test.v The test uses os.chdir + 'v .' to avoid an existing V bug with absolute path module resolution, and uses @vexe so it always tests the compiler being built. Refs vlang#27398.

Jengro777 · 2026-06-10T06:20:40Z

The test fixture feels a bit large.

Since projects_that_should_compile_test.v already creates temporary projects in other tests, maybe this case could generate the duplicate-module project under os.vtmp_dir() with a deterministic loop and a fixed pair count, for example 20/24/32 pairs, then run it with -skip-unused run flag and assert ok.

That would keep the compile/run regression strong, while avoiding ~5k LOC of mechanical fixture files.

If CI proves the pair count is too low, it can be raised by changing one constant instead of adding more files?

24 pairs seems like a good compromise, I tested it locally.

Thank you for reviewing
I have now switched to a different testing method
The implementation method of V2 is completely different, so this problem should not occur

GGRei · 2026-06-11T03:47:19Z

@codex review

chatgpt-codex-connector · 2026-06-11T03:47:29Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

GGRei · 2026-06-11T04:11:01Z

@Jengro777 can you try launching codex review please? :)

Jengro777 · 2026-06-11T04:42:28Z

@Jengro777 can you try launching codex review please? :)

I used Codex Review locally and it worked fine

Jengro777 · 2026-06-11T05:10:09Z

https://github.com/Jengro777/v/tasks/727da2a5-de44-433d-9718-555574646b21

… test

Jengro777 · 2026-06-11T05:47:33Z

@codex review

chatgpt-codex-connector · 2026-06-11T05:49:37Z

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ 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".

Jengro777 · 2026-06-12T23:22:34Z

The error has nothing to do with PR

Jengro777 · 2026-06-16T00:05:32Z

@codex review

chatgpt-codex-connector · 2026-06-16T00:07:11Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: e23467b3b1

ℹ️ 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".

Jengro777 · 2026-06-16T11:09:57Z

Request a review

Jengro777 · 2026-06-18T02:02:02Z

@medvednikov

Sorry to bother you, can this PR be prioritized for review?
This bug causes us to modify the source code every time we execute V up in our production environment in order to compile properly

Jengro777 added 2 commits June 10, 2026 14:17

Merge remote-tracking branch 'V_REPO/master' into generic_fn

3ba5f47

Jengro777 added 2 commits June 11, 2026 13:42

Merge remote-tracking branch 'V_REPO/master' into generic_fn

b43efe0

tests: guard working directory restore with defer in generic_fn dedup…

300e78a

… test

Merge remote-tracking branch 'V_REPO/master' into generic_fn

e23467b

Uh oh!

Conversation

Jengro777 commented Jun 9, 2026

Root Cause

Fix

Reproduction

Tests

Changed Files

Uh oh!

chatgpt-codex-connector Bot commented Jun 9, 2026

Uh oh!

Jengro777 commented Jun 9, 2026

Uh oh!

GGRei commented Jun 9, 2026

Uh oh!

Jengro777 commented Jun 9, 2026

Uh oh!

GGRei commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jengro777 commented Jun 10, 2026

Uh oh!

Jengro777 commented Jun 10, 2026

Uh oh!

GGRei commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jengro777 commented Jun 10, 2026

Uh oh!

GGRei commented Jun 11, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jun 11, 2026

Uh oh!

GGRei commented Jun 11, 2026

Uh oh!

Jengro777 commented Jun 11, 2026

Uh oh!

Jengro777 commented Jun 11, 2026

Uh oh!

Jengro777 commented Jun 11, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jun 11, 2026

Uh oh!

Jengro777 commented Jun 12, 2026

Uh oh!

Jengro777 commented Jun 16, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jun 16, 2026

Uh oh!

Jengro777 commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jengro777 commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GGRei commented Jun 9, 2026 •

edited

Loading

GGRei commented Jun 10, 2026 •

edited

Loading

Jengro777 commented Jun 16, 2026 •

edited

Loading