Restore legacy cross-assembly inline metadata - #20260
Open
T-Gro wants to merge 14 commits into
Open
Conversation
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>
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
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
Member
Author
|
cc @auduchinok |
T-Gro
marked this pull request as ready for review
August 14, 2026 10:53
github-actions
Bot
added
the
⚠️ Affects-Compiler-Output
This comment has been minimized.
This comment has been minimized.
auduchinok
approved these changes
Aug 14, 2026
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-actions
Bot
added
the
⚠️ Affects-Agent-Config
Contributor
|
🔍 Tooling Safety Check — Affects-Agent-Config, Affects-Compiler-Output
|
abonie
approved these changes
Aug 14, 2026
github-project-automation
Bot
moved this from New
to In Progress
in F# Compiler and Tooling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.