Make specialization name encoding collision-safe

[1sgtpepper]

Part of #479 (approved); split from #604.

Summary

Preserve literal source-name bytes and partial-template name positions across specialization passes. Partial function reuse is keyed by the exact source function and concrete bindings rather than rendered symbol names.

Changes

• Add verified poly.name_pattern metadata containing literal chunks around the current template parameters; absent metadata leaves the entire symbol name opaque.
• Preserve post-insertion template and function names when symbol-table uniquing changes a preferred generated name.
• Make felt specialization names distinguish field names with a byte-length prefix.
• Add positive, negative, collision, cache-reuse, wildcard, and type-inference coverage.

Testing

• GitHub Actions: all 9 checks pass, including GCC, Clang, debug/release, docs, formatting, changelog, and test-result checks.
• nix develop --command bash -c "cmake --build build --target llzk-opt -j1"
• Focused llzk-opt/FileCheck and -verify-diagnostics pipelines for flattening, type-variable inference, wildcard specialization, malformed poly.name_pattern, and the existing felt-field diagnostic.
• check-llzk-tests.sh upstream/main
• scripts/generate-test-checks.py stdout dry-run: 57 split outputs validated. Two splits expose an existing generator limitation with inline #felt.field/#felt<const> attributes; all committed complete output checks pass.

Documentation and changelog

The poly.template TableGen documentation describes the metadata contract, and the required unreleased changelog entry is included.

Allow edits from maintainers is enabled.

AI assistance

• No AI tools contributed to this PR.
• AI tools contributed to this PR.

Tools used: Codex

How the tools contributed: Implementation.

How I verified the contribution: Reviewed the final diff against agreed upon maintainer direction; ran the exact-head build, focused FileCheck and diagnostic pipelines, generator dry-runs, a surface audit, and confirmed all 9 GitHub Actions checks passed.

[tim-hoffman]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: b9466f6fa2

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

[iangneal]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: f3368d77b2

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

[iangneal]

This is a duplicate of some code you have in another PR, so refer to my comments there: https://github.com/project-llzk/llzk-lib/pull/634/changes#r3646683046

[tim-hoffman]

@1sgtpepper there's a real bug here as witnessed by test/Transforms/Flattening/instantiate_name_encoding.llzk but I think this solution is a bit too adhoc and can result in some problems.

Multiple passes can perform some kind of partial instantiation that adds the placeholder character: specialize-wildcard-arrays, inline-structs, infer-tvar, and of course flatten (where it's expected that placeholders would be replaced). The problem with fromRawName and the other uses of escapeSpecialCharacters is that they may be called when a string is not actually a "raw name" because an earlier pass created this name from BuildShortTypeString but the later pass has no way of knowing that. So, a placeholder added by infer-tvar can be escaped during flatten when it was expected that flatten would now replace it with the proper instantiation.

One foolproof solution would be a verification on all input IR to ensure the PLACEHOLDER is not present.

[tim-hoffman]

see earlier comment

[iangneal]

@1sgtpepper there's a real bug here as witnessed by test/Transforms/Flattening/instantiate_name_encoding.llzk but I think this solution is a bit too adhoc and can result in some problems.

Multiple passes can perform some kind of partial instantiation that adds the placeholder character: specialize-wildcard-arrays, inline-structs, infer-tvar, and of course flatten (where it's expected that placeholders would be replaced). The problem with fromRawName and the other uses of escapeSpecialCharacters is that they may be called when a string is not actually a "raw name" because an earlier pass created this name from BuildShortTypeString but the later pass has no way of knowing that. So, a placeholder added by infer-tvar can be escaped during flatten when it was expected that flatten would now replace it with the proper instantiation.

One foolproof solution would be a verification on all input IR to ensure the PLACEHOLDER is not present.

This feedback also applies to #634 which has a similar function.

[1sgtpepper]

Should PLACEHOLDER be rejected before transforms run, including in included modules?

[iangneal]

Should PLACEHOLDER be rejected before transforms run, including in included modules?

Probably so, but we'll get @tim-hoffman's input as well.

[tim-hoffman]

Should PLACEHOLDER be rejected before transforms run, including in included modules?

Probably so, but we'll get @tim-hoffman's input as well.

I'm putting together some test cases to exercise the specific problems I mentioned.

[1sgtpepper]

Ill hold off on #634 until this is cleared so I can reconcile the two.

[tim-hoffman]

@1sgtpepper #646 adds some tests that would end up with mangled/reordered names due to changes in this PR. Hopefully that clears up the issue I was describing.

In the "foolproof" solution I mentioned before, modules included via IncludeOp::inlineAndErase() would also need to be sanitized. However, I hadn't really considered protecting the surface of IR generated via the c/c++ "build" API functions from applications using this repo as a library. That's more challenging. I am open to any ideas you have on how to handle this, even so far as replacing the PLACEHOLDER based implementation with some other way to track positions in symbol names that will be replaced with attribute values in the future.

[1sgtpepper]

Thanks for #646 been looking into this and seems more messy than I initially thought, I'll share my thoughts soon since this also affects my adjacent prs substantially(it seems)

[1sgtpepper]

Thanks—#646 convinced me that the spelling itself is the wrong place to retain slot state.

My proposal is to store literal name chunks on poly.template, with gaps matching the remaining parameters. Passes refine those gaps; sym_name remains opaque.

Two questions:

1. Should this be an inherent ODS attribute, or is a verified generic ArrayAttr appropriate?
2. Can absence mean a literal source/legacy name, with no automatic migration?

I'd keep the rest out of this PR.

[1sgtpepper]

Leaning toward an optional inherent ODS name_pattern backed by ArrayAttr, since later passes rely on it.

When absent, the entire sym_name is opaque, regardless of whether parameters remain; slots are never inferred from marker bytes.

I’d keep the scope to the existing partial-template refinement paths, plus a pass-local exact cache replacing rendered-name reuse for partial functions.

[tim-hoffman]

@1sgtpepper I'd lean toward a verified generic ArrayAttr rather than adding it to the ODS since it's not truly a language feature but rather an artifact needed only for transformation passes. It's unclear to me how this would work when multiple versions of the same template are created; is the symbol name also being updated to avoid conflicts?

[1sgtpepper]

A verified generic ArrayAttr works. Each generated partial-template clone gets a rendered sym_name; SymbolTable uniquifies collisions, and references use the inserted name. Flattening reuses partial functions by source function and exact bindings.

Is it okay for a missing attribute to mean the entire name is opaque, with no legacy slot reconstruction?

[tim-hoffman]

A verified generic ArrayAttr works. Each generated partial-template clone gets a rendered sym_name; SymbolTable uniquifies collisions, and references use the inserted name. Flattening reuses partial functions by source function and exact bindings.

Is it okay for a missing attribute to mean the entire name is opaque, with no legacy slot reconstruction?

Yes. That should be fine.