Skip to content

Fix #3008: local function forwarding a display class loses its scope - #4100

Merged
siegfriedpammer merged 1 commit into
masterfrom
fix/3008-callbuilder-assert
Sep 3, 2026
Merged

siegfriedpammer merged 1 commit into
masterfrom
fix/3008-callbuilder-assert

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

Written by Claude (agent), posted on behalf of @siegfriedpammer.

Fixes #3008.

Decompiling Microsoft.CodeAnalysis.CSharp.Binder.CreateConversion asserted at Debug.Assert(localFunction != null) in CallBuilder, and the Release build emitted uncompilable code - calls written as raw metadata names, <CreateConversion>g__reportUseSiteDiagnostics|253_1(...).

DetermineCaptureAndDeclarationScope derives a local function's declaration scope from the display-class variable at its use sites. Roslyn passes display classes into local functions as ref parameters, and GetClosureInitializer returns null for a parameter, so a local function used only from inside another local function never resolved a scope. It fell through to the blanket root-body fallback and was hoisted out of the function whose closure its callees read, leaving those callees out of scope. A closure parameter now resolves to the variable that actually holds the display class, so all three end up siblings in the same local function, matching the source layout.

The constructor branch had a second, smaller instance of the same problem: use sites in separate function bodies share no BlockContainer, so FindCommonAncestorInstruction returned null and a valid closure-derived scope was discarded. The use-site scope is now computed separately and the closure-derived one kept when there is none.

Two points for review:

  • The constructor-path change makes the use-site scope win outright instead of folding it into the closure scope. The suite is green either way; it is the one judgement call in the diff.
  • An alternative fix - declaring at the innermost function containing all use sites - also resolved this case but tripped TryValidateSkipCount's assert on Generic<T1>.MixedLocalFunction, which assumes generic local functions are declared at the root. Worth knowing if anyone revisits this area.

Pretty fixture Issue3008_DisplayClassForwardedThroughLocalFunction covers the shape. A whole-assembly Debug decompile of the reporter's assembly no longer asserts.

Roslyn passes a display class into a local function by ref, and a local
function that only forwards that parameter to a sibling has no closure
variable of its own. The closure analysis therefore found nothing to
anchor it and fell back to the root method body, which put it out of
reach of the callees it forwards to; CallBuilder then hit the assert
guarding a local function reference it cannot resolve and emitted the
raw metadata name of the target instead.

The constructor path also mixed use-site containers into a scope the
closure analysis had already determined; when the use-sites live in
separate function bodies there is no common container, and resetting to
the constructor body threw that scope away.

Assisted-by: Claude:claude-opus-5:Claude Code
@siegfriedpammer
siegfriedpammer merged commit 7873fd5 into master Sep 3, 2026
17 checks passed
@siegfriedpammer
siegfriedpammer deleted the fix/3008-callbuilder-assert branch September 3, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant