Skip to content

Restore legacy cross-assembly inline metadata - #20260

Merged
T-Gro merged 14 commits into
dotnet:mainfrom
T-Gro:t-gro-fix-cross-assembly-srtp-release
Aug 16, 2026
Merged

Restore legacy cross-assembly inline metadata#20260
T-Gro merged 14 commits into
dotnet:mainfrom
T-Gro:t-gro-fix-cross-assembly-srtp-release

Conversation

@T-Gro

Copy link
Copy Markdown
Member

Fixes#20253

F# 4.7 and earlier encoded required inline values with flag bits now used by InlinedDefinition, causing newer compilers to emit calls to dynamic-invocation stubs. Normalize legacy flags while importing metadata.

T-Groand others added 10 commits August 13, 2026 20:25
Regression fixture: LegacyInline.dll built with .NET SDK 10.0.105
(F# compiler without ValInline.InlinedDefinition). That compiler used
the pre-InlinedDefinition encoding where ValInline.Always = 0x00 bits.
The current compiler reads 0x00 as InlinedDefinition (ShouldInline=false).
With --optimize-, crossAssemblyOpt() returns false and ShouldInline=false,
so the body is never fetched; the optimizer emits a direct IL call instead
of the inlined form expected for an `inline` function.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the arithmetic-only `increment` fixture with a cross-assembly
inline SRTP function whose compiled (non-inlined) fallback body is the
compiler-generated "Dynamic invocation of Invoke is not supported"
placeholder, matching the shape of the real-world regression (issue
20253, Aether's op_HatEquals). LegacyInline.dll is regenerated from the
updated LegacyInline.fs with the official .NET SDK 10.0.105 F# compiler.
The test now exercises Release/optimized codegen (withOptimize) and
compiles+runs the consumer, asserting no direct call to the placeholder
remains in the imported IL.
Note: exhaustive testing against SDK 10.0.100/10.0.105/10.0.203/10.0.301
shows none of these official compilers actually emit the ambiguous
zero-bit ValInline encoding for this shape (SRTP trait resolution and
witness-passing both resolve the call at the consumer's type-check time,
independent of the ShouldInline metadata bit), so this fixture does not
currently reproduce a failing run against HEAD. It does correctly
validate the cross-assembly SRTP import path and regresses if a future
change reintroduces a direct call to the placeholder body.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Build LegacyInline.dll with the official .NET SDK 3.1.100 F# compiler (10.7.0.0 for F# 4.7), which predates witness metadata and preserves the legacy inline encoding.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Refactor Issue 20253 test to focus on IL verification, removing runtime
execution that doesn't contribute to the regression contract. The test
now compiles as a library and verifies that the legacy inline metadata
from LegacyInline.dll is properly inlined, preventing the direct call
to LegacyInline.Library::invoke from appearing in the emitted IL.
This change:
- Simplifies Consumer module from EntryPoint program to library function
- Removes runtime execution verification (run | shouldSucceed | verifyOutputContains)
- Retains compile success verification (shouldSucceed)
- Maintains IL regression assertion (verifyILNotPresent [ "LegacyInline.Library::invoke" ])
The IL assertion is the actual regression contract for this issue.
Co-authored-by: Copilot <copilot@example.com>
u_ValData deserialized ValFlags directly from the pickled int64, so
DLLs written by compilers <= 4.7 (pre-witness), which encoded
PseudoVal/Always inline info as all-zero inline bits, were imported as
ValInline.InlinedDefinition after PR dotnet#19548 repurposed the same 0x00
bits for that case. Add ValFlags.OfPickledBits, mirroring the
InlinedDefinition -> Always normalization PickledBits already applies
on write, and use it in u_ValData so legacy zero-bit values import as
Always (ShouldInline=true) regardless of which compiler wrote them.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Compile the Issue 20253 consumer out-of-process via runFscProcess
instead of CompilerAssert.CompileRaw in-process. The in-process path
can mutate shared compiler/import state across tests sharing the same
process, so keep this legacy pre-witness FSharp.Core regression check
at the IL boundary without touching the test host's own state.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actionsBot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
`src/Compiler`docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

T-Groand others added 3 commits August 13, 2026 22:14
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9bb5fd2-ae26-4369-934d-1bae177a1266
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9bb5fd2-ae26-4369-934d-1bae177a1266
@T-Gro

Copy link
Copy Markdown
MemberAuthor

cc @auduchinok

@T-Gro
T-Gro requested review from abonie and auduchinokAugust 14, 2026 10:52
@T-Gro
T-Gro marked this pull request as ready for review August 14, 2026 10:53
@T-Gro
T-Gro requested a review from a team as a code ownerAugust 14, 2026 10:53
@github-actionsgithub-actionsBot added the ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen label Aug 14, 2026
@github-actions

This comment has been minimized.

@auduchinok

Copy link
Copy Markdown
Member

@T-Gro Thanks for fixing it!

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d9bb5fd2-ae26-4369-934d-1bae177a1266
@github-actionsgithub-actionsBot added the ⚠️ Affects-Agent-Config Tooling check: PR modifies AI agent instructions or workflows label Aug 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Agent-Config, Affects-Compiler-Output
Affects-Agent-Config: modifies .github/instructions/TypedTreePickle.instructions.md
Affects-Compiler-Output: changes metadata import flag normalization in TypedTree/TypedTreePickle

Generated by PR Tooling Safety Check · opus46 3.1M ·

@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingAug 14, 2026
@T-Gro
T-Gro merged commit 7efd608 into dotnet:mainAug 16, 2026
49 checks passed
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in F# Compiler and ToolingAug 16, 2026
@T-Gro

Copy link
Copy Markdown
MemberAuthor

/backport to release/10.0.4xx

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0.4xx (link to workflow run)

T-Gro added a commit that referenced this pull request Aug 18, 2026
Chiron's chiron-6 branch serializes through Aether 8.2.0's `inline` SRTP
optics, which reproduces the SDK 10.0.400 legacy cross-assembly
inline-metadata regression (#20253, fixed by #20260). The bug is
runtime-only: a compiler that misreads Aether's legacy pickled inline flag
drops the inline body and emits a call to the dynamic-invocation stub, so
the build stays clean but System.NotSupportedException is thrown at run
time. The matrix entry therefore runs Chiron's own xunit suite; a build-only
check would pass despite the bug. Two sed edits only retarget dead
frameworks (the library's net452 leg cannot restore on a current SDK; the
test project targets the removed netcoreapp2.0) and change no test logic.
Verified: 11 tests fail with NotSupportedException on SDK 10.0.400, and all
pass with the compiler built from main.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
T-Gro added a commit that referenced this pull request Aug 18, 2026
Chiron's chiron-6 branch serializes through Aether 8.2.0's `inline` SRTP
optics, which reproduces the SDK 10.0.400 legacy cross-assembly
inline-metadata regression (#20253, fixed by #20260). The bug is
runtime-only: a compiler that misreads Aether's legacy pickled inline flag
drops the inline body and emits a call to the dynamic-invocation stub, so
the build stays clean but System.NotSupportedException is thrown at run
time. The matrix entry therefore runs Chiron's own xunit suite, filtered to
the Aether Optic get/set/map tests that exercise the miscompiled path; a
build-only check would pass despite the bug. Two sed edits only retarget
dead frameworks (the library's net452 leg cannot restore on a current SDK;
the test project targets the removed netcoreapp2.0) and change no test logic.
Verified: the Optic tests fail with NotSupportedException on SDK 10.0.400,
and all pass with the compiler built from main.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
T-Gro added a commit that referenced this pull request Aug 18, 2026
Chiron's chiron-6 branch serializes through Aether 8.2.0's `inline` SRTP
optics, which reproduces the SDK 10.0.400 legacy cross-assembly
inline-metadata regression (#20253, fixed by #20260). The bug is
runtime-only: a compiler that misreads Aether's legacy pickled inline flag
drops the inline body and emits a call to the dynamic-invocation stub, so
the build stays clean but System.NotSupportedException is thrown at run
time. The matrix entry therefore runs Chiron's own xunit suite, filtered to
the Aether Optic get/set/map tests that exercise the miscompiled path; a
build-only check would pass despite the bug. Two sed edits only retarget
dead frameworks (the library's net452 leg cannot restore on a current SDK;
the test project targets the removed netcoreapp2.0) and change no test logic.
Verified: the Optic tests fail with NotSupportedException on SDK 10.0.400,
and all pass with the compiler built from main.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
abonie pushed a commit that referenced this pull request Aug 19, 2026
Chiron's chiron-6 branch serializes through Aether 8.2.0's `inline` SRTP
optics, which reproduces the SDK 10.0.400 legacy cross-assembly
inline-metadata regression (#20253, fixed by #20260). The bug is
runtime-only: a compiler that misreads Aether's legacy pickled inline flag
drops the inline body and emits a call to the dynamic-invocation stub, so
the build stays clean but System.NotSupportedException is thrown at run
time. The matrix entry therefore runs Chiron's own xunit suite, filtered to
the Aether Optic get/set/map tests that exercise the miscompiled path; a
build-only check would pass despite the bug. Two sed edits only retarget
dead frameworks (the library's net452 leg cannot restore on a current SDK;
the test project targets the removed netcoreapp2.0) and change no test logic.
Verified: the Optic tests fail with NotSupportedException on SDK 10.0.400,
and all pass with the compiler built from main.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@T-Gro

Copy link
Copy Markdown
MemberAuthor

/backport to release/dev18.9

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/dev18.9 (link to workflow run)

@T-Gro

Copy link
Copy Markdown
MemberAuthor

/backport to release/dev18.10

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/dev18.10 (link to workflow run)

@github-actions

Copy link
Copy Markdown
Contributor

@T-Gro an error occurred while backporting to release/dev18.10. See the workflow output for details.

@github-actions

Copy link
Copy Markdown
Contributor

@T-Gro an error occurred while backporting to release/dev18.9. See the workflow output for details.

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

Labels

⚠️ Affects-Agent-ConfigTooling check: PR modifies AI agent instructions or workflows⚠️ Affects-Compiler-OutputTooling check: PR touches IL emission or codegen

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

SDK 10.0.400 regression: cross-assembly inline SRTP call throws in Release

3 participants

@T-Gro@auduchinok@abonie