Skip to content

[Repo Assist] Fix custom attribute encoding for generative type providers#463

Draft
github-actions[bot] wants to merge 2 commits intomasterfrom
repo-assist/fix-custom-attr-encoding-87a0d53629082154
Draft

[Repo Assist] Fix custom attribute encoding for generative type providers#463
github-actions[bot] wants to merge 2 commits intomasterfrom
repo-assist/fix-custom-attr-encoding-87a0d53629082154

Conversation

@github-actions
Copy link
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Two related fixes to custom attribute encoding in the generative type provider assembly compiler (AssemblyCompiler / defineCustomAttrs).

Fix 1: Implement array support in encodeCustomAttrElemTypeForObject

Before: Passing an obj[] (or typed array like string[]) as an argument to an Object-typed custom attribute constructor parameter would throw:

failwith "TODO: can't yet emit arrays in attrs"
```

**After:** Infers the element type from the runtime array type and encodes `SZARRAY + element type byte` per ECMA-335 §II.23.3. Supported element types: `string`, `bool`, `char`, `sbyte`/`byte`, `int16`/`uint16`, `int32`/`uint32`, `int64`/`uint64`, `single`, `double`, and `obj` (encoded as `OBJECT`).

## Fix 2: Apply `transValue` to all custom attribute argument values

**Before:** `transValue` (which translates `System.Type` → `ILType`) was defined inside `defineCustomAttrs` but never used — the function was dead code. Constructor arguments and named properties/fields were passed through untranslated.

**After:** `transValue` is moved before its first use and applied to:
- All constructor fixed arguments (`constructorArgs`)
- Named property values (`namedProps`)
- Named field values (`namedFields`)

This ensures that if a custom attribute argument contains a `System.Type` value (e.g., `typeof(SomeProvidedType)`), it is translated to the target `ILType` and the correct type name is encoded in the binary attribute blob.

## Root cause

Both issues affect **generative type providers** only. Erased providers reflect attributes directly without going through ECMA-335 binary encoding.

## Test Status

All existing tests pass: **104 passed, 0 failed** on net8.0.

```
Passed!  - Failed: 0, Passed: 104, Skipped: 0, Total: 104

Note: the new functionality (array args, Type args in attrs) is exercised by the fixes themselves but no dedicated integration test is added — adding one would require a new generative provider test fixture with attributes using these specific parameter patterns.

Generated by Repo Assist

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@afb00b92a9514fee9a14c583f059a03d05738f70

…sValue to all attribute args

- Implement encodeCustomAttrElemTypeForObject for obj[] — previously threw
  'TODO: can't yet emit arrays in attrs'. Now infers element type from the
  runtime array type and encodes SZARRAY + element type byte per ECMA-335.
- Apply transValue to constructorArgs and named property/field values in
  defineCustomAttrs. Previously transValue was defined but never called,
  meaning System.Type values in attribute constructor arguments were not
  translated to their target ILType — potentially encoding design-time type
  names instead of target names.

Tests: 104 passed, 0 failed

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants