Skip to content

[Repo Assist] Fix decodeILCustomAttribData: read correct byte width for non-int32 enum fixed args#472

Closed
github-actions[bot] wants to merge 4 commits intomasterfrom
repo-assist/fix-decode-enum-attr-size-20260313-ab27eb57e3810ec7
Closed

[Repo Assist] Fix decodeILCustomAttribData: read correct byte width for non-int32 enum fixed args#472
github-actions[bot] wants to merge 4 commits intomasterfrom
repo-assist/fix-decode-enum-attr-size-20260313-ab27eb57e3810ec7

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

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

Problem

decodeILCustomAttribData had a catch-all arm for enum-typed fixed arguments that always read 4 bytes regardless of the enum's actual underlying type:

| ILType.Value _ ->  (* assume it is an enumeration *)
    let n, sigptr = sigptr_get_i32 bytes sigptr
    (argty, box n), sigptr
```

Per ECMA-335 §II.23.3, custom attribute blobs encode enum fixed arguments using the **underlying type's width** — so a `byte`-backed enum encodes in 1 byte, an `int64`-backed one in 8 bytes. Always reading 4 bytes corrupts the stream offset for any subsequent attribute arguments when a non-`Int32` enum appears.

## Fix

Add a `resolveEnumUnderlyingILType: ILType -> ILType` parameter to `decodeILCustomAttribData`. At the call site in `txCustomAttributesDatum` (inside `TargetTypeDefinition`), the resolver:

1. Calls `txILType` to resolve the `ILType.Value` to a `System.Type`
2. Calls `Enum.GetUnderlyingType` to get the backing type
3. Maps back to the appropriate `ILType` (`typ_Byte`, `typ_Int16`, `typ_Int64`, etc.)
4. Falls back to `ilGlobals.typ_Int32` on any exception (preserving prior behaviour for unresolvable types)

The `ILType.Value _` arm now delegates to `parseVal underlyingTy sigptr`, which dispatches to the already-correct handler for the underlying primitive type, and returns `argty` (the original enum type) as the result type.

**For the common case (int32-backed enums):** the resolver returns `ilGlobals.typ_Int32` and behaviour is bit-for-bit identical to before. No regression risk for existing type providers.

**For non-int32 enums:** type providers consuming reference assemblies with `byte`, `int16`, `uint16`, `uint32`, `int64`, or `uint64` enum parameters on custom attributes will now decode correctly rather than silently mis-reading the blob.

## Test Status

```
Passed!  - Failed: 0, Passed: 110, Skipped: 0, Total: 110, Duration: 6 s

All 110 existing tests pass. This fix addresses the "Future Work" item noted in #465 (decodeILCustomAttribData enum-size bug).

Generated by Repo Assist ·

To install this agentic workflow, run

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

… enum fixed args

Previously the catch-all ILType.Value arm in parseVal always read 4 bytes
(int32) for any enum-typed fixed argument in a custom attribute blob.
ECMA-335 requires that enum values are stored using the width of the enum's
underlying type, so a byte-backed enum attribute argument encodes in 1 byte
and an int64-backed one in 8 bytes.

Fix: add a resolveEnumUnderlyingILType resolver that is threaded through
decodeILCustomAttribData. At the txCustomAttributesDatum call site the
resolver uses txILType to get the runtime System.Type for the enum, calls
Enum.GetUnderlyingType, and maps back to the correct ILType. Any resolution
failure falls back to ilg.typ_Int32 (preserving prior behaviour).

For the common case (int32 enums) the resolver returns ilg.typ_Int32 and
behaviour is identical to before. The fix benefits type providers that
reference assemblies containing attributes with byte, int16, uint16,
uint32, int64, or uint64 enum parameters.

All 110 existing tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@dsyme
Copy link
Copy Markdown
Contributor

dsyme commented Mar 16, 2026

/repo-assist we need tests added for this case - unit tests would be fine if they can be arranged, even better integration tests

…m fixed args

Three new tests in BasicGenerativeProvisionTests.fs verify that decodeILCustomAttribData
correctly reads byte-, int16-, and int64-backed enum custom attribute arguments via
ReadRelatedAssembly (the path that exercises the IL blob decoder).

Also fixes TargetTypeDefinition.GetEnumUnderlyingType() which previously always returned
Int32 (noted as a TODO). It now reads the 'value__' field from the ILTypeDef to
determine the actual underlying type. This is required for resolveEnumUnderlyingILType
(introduced in the main fix) to work correctly for target types.

All 113 tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor Author

Commit pushed: 79de50a

Generated by Repo Assist

@github-actions
Copy link
Copy Markdown
Contributor Author

🤖 This is an automated response from Repo Assist.

I've pushed a commit to this PR with the tests you asked for.

What was added (tests/BasicGenerativeProvisionTests.fs):

Three integration tests:

  • decodeILCustomAttribData decodes byte-backed enum custom attribute correctly
  • decodeILCustomAttribData decodes int16-backed enum custom attribute correctly
  • decodeILCustomAttribData decodes int64-backed enum custom attribute correctly

Each test creates a generative type provider that attaches a custom attribute backed by a non-int32 enum, generates the assembly, then reads it back via tp.TargetContext.ReadRelatedAssembly(...). This exercises decodeILCustomAttribData directly (the runtime Assembly.Load path bypasses the IL decoder). The tests verify both that no exception is thrown and that the decoded argument value matches the encoded one.

Also fixed (src/ProvidedTypes.fs):

TargetTypeDefinition.GetEnumUnderlyingType() previously had a // TODO and always returned Int32. The new resolveEnumUnderlyingILType closure introduced by this PR calls Enum.GetUnderlyingType(resolvedType), which delegates to that method—so without fixing it, the resolver would always fall back to Int32 and the decoding bug would remain for target types. The fix reads the value__ field from the ILTypeDef to return the actual underlying type.

All 113 tests pass (110 pre-existing + 3 new).

Generated by Repo Assist for issue #472 ·

To install this agentic workflow, run

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

@dsyme dsyme marked this pull request as ready for review March 16, 2026 17:32
@dsyme dsyme closed this Mar 16, 2026
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.

1 participant