Skip to content

Add regression test: #12796, DefaultValue null on array type record field#19472

Open
T-Gro wants to merge 4 commits intomainfrom
regression-test/issue12796
Open

Add regression test: #12796, DefaultValue null on array type record field#19472
T-Gro wants to merge 4 commits intomainfrom
regression-test/issue12796

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented Mar 24, 2026

Fixes #12796

Adds a regression test verifying that [<DefaultValue(null)>] on a record field of type A[] compiles successfully without internal error FS0192.

@T-Gro T-Gro requested a review from a team as a code owner March 24, 2026 10:33
@T-Gro T-Gro added NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes AI-Issue-Regression-PR PR adding regression test for a closed issue labels Mar 24, 2026
@T-Gro T-Gro requested a review from abonie March 24, 2026 10:33
@T-Gro T-Gro enabled auto-merge (squash) March 24, 2026 10:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026 •
edited
Loading

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

T-Gro
T-Gro commented Mar 26, 2026
View reviewed changes
tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs Outdated
open System.ComponentModel

type A = { AField: string }
type B = { [<DefaultValue(null)>] BField: A[] }
Copy link
Copy Markdown
Member Author

@T-Gro T-Gro Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big NO, the issue really says:

[<DefaultValue([||] : A[])>]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Commit pushed: 6d1be63

Generated by Regression PR Shepherd

@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions bot mentioned this pull request Mar 30, 2026
Open
12 tasks
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Commit pushed: 7128e25

Generated by Regression PR Shepherd

@github-actions github-actions bot mentioned this pull request Mar 31, 2026
Open
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

🤖 This is an automated response from Regression PR Shepherd.

This regression test proves the bug in #12796 still exists on Desktop .NET Framework. Closing this PR is recommended.

The bug: [<DefaultValue([||] : A[])>] on a record field of type A[] triggers an internal compiler error FS0192: encodeCustomAttrElemType: unrecognized custom element type. The issue was opened in 2022, and was labeled AI-thinks-issue-fixed after the bot detected SynPat.Record and SynPat.QuoteExpr fixes — but that was a different issue; this bug is specifically about attribute encoding for array-typed DefaultValue arguments.

The test: tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs typechecks type B = { [<DefaultValue([||] : A[])>] BField: A[] } and expects success.

The failure: Build WindowsNoRealsig_testDesktop fails — the test fails only on Desktop .NET Framework, not on CoreCLR/Linux. The FS0192 error is triggered during attribute argument encoding, which happens during type checking on Desktop.

The AI-thinks-issue-fixed label has been removed from #12796.

cc @T-Gro @abonie

Generated by Regression PR Shepherd · ◷

This was referenced Apr 1, 2026
Open
Open
@T-Gro T-Gro force-pushed the regression-test/issue12796 branch 3 times, most recently from c9f9eb2 to 929e77c Compare April 10, 2026 09:46
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Apr 10, 2026

I don't get why we would have problems encoding empty arrays of any type? It does not matter if we cannot store instances of that type, since it is empty. If we can store a string array, an empty string array, why also not an arbitrary empty array?

@T-Gro T-Gro force-pushed the regression-test/issue12796 branch 2 times, most recently from 819c8be to 73cfbce Compare April 13, 2026 10:19
T-Gro and others added 3 commits April 14, 2026 00:52
@T-Gro
Fix #12796: proper diagnostic for invalid custom attribute array elem…
ce4c244 
…ent types

Previously, using an array of a user-defined type as a custom attribute
argument (e.g. [<DefaultValue([||] : A[])>]) caused an internal error
FS0192 in encodeCustomAttrElemType. The ECMA 335 metadata format requires
the array element type to be encoded even for empty arrays, and only
primitive types, enums, string, System.Type, and System.Object are valid.

This change adds a validation check in GenAttribArg (IlxGen.fs) that
detects unsupported element types before reaching the IL writer, and
reports a proper diagnostic (FS3885) instead of crashing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix empty array handling in custom attributes: use Object element typ…
26e4d83 
…e instead of erroring

Empty arrays of user-defined types in custom attributes (e.g. [<DefaultValue([||] : A[])>])
no longer produce FS3885. Since no elements need encoding, System.Object is used as the
element type. Non-empty arrays of unencodable types still error correctly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Add type annotation to fix FS0072 in bootstrap build
a0a1a5d 
The bootstrap compiler needs an explicit type annotation on the 'g' parameter
in GenAttribArg to resolve g.ilg.typ_Object, since g's type is not constrained
until later match cases (TypeOfExpr, EnumExpr, etc.).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the regression-test/issue12796 branch from a5a1e73 to a0a1a5d Compare April 13, 2026 22:57
@T-Gro
Add enum and primitive empty array attribute tests for #12796
9799996 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Apr 14, 2026

@copilot :

See my comment above. Research implementation changes needed to support empty arrays as default values, and develop using TDD (via ComponentTests. Since this is IL specific, make sure to use "compileAndRun" and also use the helper for "ilverify" of the compiled code in that test.

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

Reviewers

@abonie abonie Awaiting requested review from abonie

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

AI-Issue-Regression-PR PR adding regression test for a closed issue NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

F# Compiler and Tooling
Status: New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

internal error : encodeCustomAttrElemType: unrecognized custom element type

1 participant

@T-Gro