Fix #3938: nested-type members must not inherit ExtensionInfo - #3942
Merged
Merged
Conversation
christophwille
left a comment
Member
There was a problem hiding this comment.
Verdict: LGTM. The fix addresses the root cause rather than the crash site, and comes with targeted regression tests for both failure layers. Detailed notes below; inline suggestions are all minor and non-blocking.
Correctness (traced against all consumers)
- The crash path is closed at its root.
DecompileBodyonly receives a non-nullextensionInfofrom the isolated-member entry points -- whole-type decompilation passesnulldown (CSharpDecompiler.cs:1888/1896) and uses the container info only to skip implementation members. GatingResolveExtensionInfoonInfoOfExtensionMembermembership therefore guarantees the!.ValueatCSharpDecompiler.cs:2150is only reached with a valid mapping. - The
Debug.Assert(extensionInfo != null)at lines 1398/1410 stays valid: those paths are reached only from extension-member UI nodes, whose members are always in the map. - The null-when-empty change fixes a latent second bug for free. The two-level fallback
td.DeclaringTypeDefinition?.ExtensionInfo ?? ...DeclaringTypeDefinition?.ExtensionInfo(inResolveExtensionInfo,RequiredNamespaceCollector.cs:88,CSharpAmbience.cs:248,CSharpDecompiler.cs:1646/1666) previously could stop at a non-null-but-empty info from an intermediate nested type and never fall through to the real container. Now it falls through correctly. - The claim about empty extension blocks holds in both encodings: a bare
extension(int) {}still yields a marker (<>E__group with<Extension>$, or a<M>$marker type), soExtensionGroups.Count > 0. - All other consumers handle the new null (
TypeTreeNode.cs:152pattern-matches,CSharpDecompiler.cs:1700uses?.+?? [];MinimalCorlib/SyntheticWpfModulealready returned null, so null was always part of the contract). - Removing the loop-carried
parentExtensionInfoinDecompile(definitions)is a good cleanliness fix -- the old variable persisted stale values across iterations. - Caching still works: the empty
ExtensionInfois stored in the field, so repeated getter calls only repeat the cheapCount > 0check, not the nested-type scan.
Non-blocking follow-up (outside this diff)
MethodTreeNode.cs:54andPropertyTreeNode.cs:58still doResolveExtensionInfo()?.InfoOfExtensionMember(...)themselves, so the membership test now runs twice. Harmless; they could be simplified to a null check onResolveExtensionInfo()in a follow-up.- External ILSpyX/plugin consumers of
ITypeDefinition.ExtensionInfowill now see null for classic-only[Extension]classes -- a correct tightening, but worth a mention if release notes track API behavior.
Test coverage, conventions (self-contained comments, headerless fixture matching TestCases/, ASCII-only), performance, and security all check out.
🤖 Generated with Claude Code
The .NET 10 BCL ships static [Extension] classes that contain ordinary nested types (e.g. XDocumentExtensions.XDocumentNavigable). Decompiling such a nested type's member in isolation resolved the enclosing container's ExtensionInfo, and DecompileBody then dereferenced the missing extension-member mapping. A container without any extension blocks now reports no ExtensionInfo at all, and ResolveExtensionInfo applies a container's info only to members that actually belong to one of its extension blocks. Assisted-by: Claude:claude-fable-5:Claude Code
siegfriedpammer
force-pushed
the
issue-3938-xdocument-navigable
branch
from
July 31, 2026 10:42
62338d9 to
fffd2e7
Compare
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
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 #3938.
Decompiling a single member of an ordinary nested type inside a static
[Extension]class (e.g.System.Xml.XPath.XDocumentExtensions.XDocumentNavigable.CreateNavigatorin the .NET 10 BCL) threwInvalidOperationException: Nullable object must have a valuewith language version C# 14+.Root cause, in two layers:
MetadataTypeDefinition.ExtensionInfowas non-null for every static class carrying[Extension]— including classicthis-parameter extension classes with no C# 14 extension blocks at all. The group scan found nothing, so the object was empty, but consumers took non-null to mean "extension container". It now returns null when there are no extension groups. (Empty extension blocks likeextension(int) {}still produce marker groups and are unaffected.)ResolveExtensionInfohanded the container's info to members of any nested type, soDecompileBodydereferenced a missingInfoOfExtensionMembermapping. It now returns the info only for members that actually belong to one of the container's extension blocks — which also covers containers that mix real extension blocks with ordinary nested types.ExtensionInfoonly ever models C# 14 extension blocks (its maps are populated solely from the<>E__/<G>$encodings); classic extension methods are resolved viaHasExtensions/IsExtensionMethod, which are unchanged.Both new tests in
IsolatedMethodDecompilationTests(classic-only and mixed-container fixtures) reproduced the reported exception before the fix; the stock .NET 10System.Xml.XPath.XDocument.dllmember from the issue now decompiles cleanly.🤖 Generated with Claude Code