Generate and resolve xml doc ID strings at the metadata level - #3926
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request rewrites XML documentation ID generation and resolution to operate directly on System.Reflection.Metadata, improving navigation and XML doc lookup (notably for C++/CLI assemblies with custom modifiers) without requiring full type-system materialization.
Changes:
- Reworked
IdStringProviderto generate ID strings and resolve IDs back to metadata entities using metadata-only logic, and added multi-dialect candidate generation (C#/Roslyn + MSVC C++/CLI where different). - Updated XML documentation lookup and UI integration to use entity-based documentation retrieval and the new ID candidate flow.
- Added extensive test coverage and fixtures (including an MSVC-generated probe IL/XML) to validate both dialects and round-trip resolution.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ILSpy/TextView/DecompilerTextView.axaml.cs | Switches XML doc retrieval to GetDocumentation(IEntity) to leverage dialect candidates. |
| ILSpy/AssemblyTree/AssemblyTreeModel.cs | Updates --navigateto entity resolution to use metadata-level lookup and forwarder chasing. |
| ICSharpCode.Decompiler/Documentation/XmlDocumentationProvider.cs | Tries ID-string candidates (C#/Roslyn + MSVC C++/CLI) when looking up docs for an entity. |
| ICSharpCode.Decompiler/Documentation/IdStringProvider.cs | Major rewrite: metadata-based ID generation, candidate IDs, and metadata-level FindEntity. |
| ICSharpCode.Decompiler/Documentation/IdStringMemberReference.cs | Removes now-obsolete parser/reference-based member resolution helper. |
| ICSharpCode.Decompiler/Documentation/GetPotentiallyNestedClassTypeReference.cs | Removes now-obsolete type-reference parsing helper. |
| ICSharpCode.Decompiler.Tests/Helpers/Tester.cs | Exposes default core reference list for Roslyn-based test compilation. |
| ICSharpCode.Decompiler.Tests/Documentation/IdStringProviderTests.cs | Adds broad Roslyn cross-check suite + metadata builder and MSVC dialect coverage. |
| ICSharpCode.Decompiler.Tests/Documentation/IdStringProbe.xml | Adds MSVC-produced XML doc fixture for dialect validation. |
| ICSharpCode.Decompiler.Tests/Documentation/IdStringProbe.il | Adds trimmed MSVC-produced IL fixture assembled at test time. |
| ICSharpCode.Decompiler.Tests/Documentation/IdStringProbe.cpp | Adds source reference for the MSVC probe assembly. |
| ICSharpCode.Decompiler.Tests/Documentation/.gitignore | Ignores built probe outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
siegfriedpammer
added a commit
that referenced
this pull request
Jul 28, 2026
A bound generic argument list without its closing brace fell off the end of the string and silently produced an arity, making malformed ID strings appear to parse and resolve to nothing instead of failing with the documented ReflectionNameParseException. Also replaces the non-ASCII punctuation in comments added by this branch with ASCII equivalents, per the repository convention. Both raised by review on #3926. Assisted-by: Claude:claude-fable-5:Claude Code
Generate ID strings directly from metadata instead of the type system,
so callers holding only a MetadataFile and an EntityHandle do not need
to materialize a compilation first, and raw metadata names survive
verbatim. GetIdString(IEntity) stays as a thin wrapper over the new
implementation, keeping the published entity-based entry point intact.
The implementation is validated by a differential suite comparing every
symbol of a test corpus against Roslyn's GetDocumentationCommentId.
Corners pinned by those tests: generic arguments distribute to their
nesting level (Outer{A}.Inner{B}), op_CheckedExplicit carries the
~ReturnType suffix like the other conversion operators, and custom
modifiers are ignored like Roslyn ignores them (a virtual method's 'in'
parameter is modreq(InAttribute) but documented as T@). Array shapes
use the spec's lowerbound:size notation, covered by a hand-built
module, since C# cannot express non-default array bounds in signatures.
Assisted-by: Claude:claude-fable-5:Claude Code
Resolve an ID string to a (module, handle) pair by scanning metadata directly: namespace/type-name splits are tried at every dot (the format does not mark the boundary), nested types and type forwarders are walked, and members are matched by regenerating each candidate's ID string instead of parsing the signature portion, which keeps resolution in sync with generation by construction. This gives navigation a resolution path that needs no type system and works on assemblies exactly as their metadata spells them. Inherent format limitations (metadata names containing ID string special characters; function pointer types rendering empty, so such overloads share an ID) are documented on the method. Assisted-by: Claude:claude-fable-5:Claude Code
FindEntityInRelevantAssemblies now uses the metadata-level FindEntity instead of parsing the ID string into type-system references and resolving them per assembly. Two behaviors of the old path need explicit handling because FindEntity only searches the modules it is handed: reference assemblies are skipped so the search prefers an assembly with a usable definition, and a member whose declaring type is present only as a type forwarder is looked up in the assembly the forwarder points to, which the assembly resolver may load on demand (the old path got this through DecompilerTypeSystem resolution). Assisted-by: Claude:claude-fable-5:Claude Code
MSVC documents C++/CLI members with ECMA-372-style ID strings that
differ from Roslyn's in signatures: custom modifiers are rendered after
the modified type (a 'const int' parameter becomes
System.Int32!System.Runtime.CompilerServices.IsConst), arity markers
stay on generic instantiations (List`1{System.Int32}.Enumerator), and
the default indexed property is called 'default'. Roslyn ignores
modifiers entirely, so one generated string cannot match both
compilers' xml files: GetIdString keeps producing the C#/Roslyn form,
and the new GetIdStringCandidates additionally yields the C++/CLI form,
most specific first, for lookup code to try in order.
The dialect is pinned by IdStringProbe.il/.xml, the trimmed disassembly
of an MSVC-compiled probe assembly together with the unmodified xml MSVC
generated for it. Notable observed deviations from MSVC's documented
format: modreq is generated, but only modreq(IsVolatile) uses the
documented '|'; modreq(IsByValue) on conversion operator operands is
rendered with '!'.
Assisted-by: Claude:claude-fable-5:Claude Code
The member keys in an xml doc file depend on the compiler that wrote it (C# vs the MSVC C++/CLI dialect), so XmlDocumentationProvider's entity-based lookup now tries each candidate form in order. The tooltip path goes through the entity overload instead of building the key itself, so the fallback lives in one place. The C++/CLI form is tried first: wherever it differs it contains character sequences Roslyn never writes, so it can only match MSVC-generated keys, while the stripped Roslyn form of one member could match the key of a different member in an MSVC-generated file. Assisted-by: Claude:claude-fable-5:Claude Code
Member lookup compares regenerated ID strings, and with two dialects a single mixed pass can resolve to the wrong member: the stripped C#/Roslyn form of one member can equal the C++/CLI-dialect key of a different member, e.g. C++ overloads differing only in a custom modifier such as char* vs signed char*. FindMemberInType therefore runs two passes over the whole member list, the more specific C++/CLI form first, so an MSVC-written cref resolves to the member it names instead of the first member whose stripped form happens to collide. Assisted-by: Claude:claude-fable-5:Claude Code
The recursive-descent parser for the full ID string grammar existed only to feed the type-system-based FindEntity, while signatures were already matched by regenerating candidate IDs; with two dialects a parser would have to implement both grammars and stay in sync with the generator. FindEntity now only decodes the structural skeleton - the declaring type name and the member name - resolves the type, and narrows the members by metadata name before the generate-and-compare step, falling back to an unfiltered scan for keys whose name does not equal the metadata name (a C++/CLI 'default' indexer). ParseTypeName, ParseMemberIdString and their reference types (IdStringMemberReference, GetPotentiallyNestedClassTypeReference) are removed. Assisted-by: Claude:claude-fable-5:Claude Code
The C#/Roslyn-form ID of a member whose C++/CLI form differs can equal the only key of a same-named sibling overload (char* vs signed char*). Assemblies containing such overloads cannot come from the C# compiler, so their xml files use the C++/CLI dialect, where that key documents the sibling: falling back to the Roslyn form would show the sibling's documentation for an undocumented member. GetIdStringCandidates now omits the Roslyn form when a same-named sibling's C++/CLI form owns it, so a lookup miss stays a miss. Assisted-by: Claude:claude-fable-5:Claude Code
A bound generic argument list without its closing brace fell off the end of the string and silently produced an arity, making malformed ID strings appear to parse and resolve to nothing instead of failing with the documented ReflectionNameParseException. Also replaces the non-ASCII punctuation in comments added by this branch with ASCII equivalents, per the repository convention. Both raised by review on #3926. Assisted-by: Claude:claude-fable-5:Claude Code
IdStringMemberReference was its only implementation and left with the ID string grammar parser; unlike the NRefactory-era type system, IMember does not extend IMemberReference here, so nothing in the library produces or consumes it anymore. Assisted-by: Claude:claude-fable-5:Claude Code
Each fixture test assembled Documentation/IdStringProbe.il separately, and ilasm writes its output next to the input: on Windows the second test failed with a sharing violation (0x80070020) because the first test's PEFile still had the previous output open. The probe is now assembled once per test run from a temp copy of the .il, so the tests share one PEFile and nothing is written into the source tree, which also makes the per-directory .gitignore unnecessary. Assisted-by: Claude:claude-fable-5:Claude Code
siegfriedpammer
force-pushed
the
id-strings-rewrite
branch
from
July 30, 2026 05:22
8d28fdb to
f302a37
Compare
siegfriedpammer
added a commit
that referenced
this pull request
Jul 30, 2026
A bound generic argument list without its closing brace fell off the end of the string and silently produced an arity, making malformed ID strings appear to parse and resolve to nothing instead of failing with the documented ReflectionNameParseException. Also replaces the non-ASCII punctuation in comments added by this branch with ASCII equivalents, per the repository convention. Both raised by review on #3926. Assisted-by: Claude:claude-fable-5:Claude Code
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.
Rewrites
IdStringProviderto work directly on System.Reflection.Metadata: ID strings (C# spec §A.3.1 / ECMA-372 documentation comments) are generated from raw metadata and resolved back without materializing a type system, so navigation and xml doc lookup work on assemblies exactly as their metadata spells them, including assemblies whose dependencies are not resolvable.Fixes #2728.
What changed
GetIdString(MetadataFile, EntityHandle)generates the C#/Roslyn dialect straight from metadata: raw names, generic arguments distributed to their nesting level,op_CheckedExplicitreturn-type suffix, spec-conformant arraylowerbound:sizeshapes, custom modifiers omitted like Roslyn omits them.GetIdString(IEntity)remains as a thin wrapper.GetIdStringCandidatesadditionally yields the MSVC C++/CLI dialect where it differs: custom modifiers rendered after the modified type (System.Int32!System.Runtime.CompilerServices.IsConstfor aconst intparameter), arity markers kept on generic instantiations (List`1{System.Int32}.Enumerator), default indexed properties nameddefault. Documentation lookup (including tooltips) tries the candidates most-specific-first, which is what fixes xml doc display for C++/CLI assemblies.FindEntityresolves an ID by decoding only the structural skeleton (declaring type, member name), then narrows members by metadata name and matches by regenerating candidate IDs — one dialect per pass, so overloads differing only in a custom modifier (char*vssigned char*) resolve to the member the key names. The--navigateto/cref path additionally skips reference assemblies and follows type forwarders into their target assembly.ParseTypeName,ParseMemberIdString,IdStringMemberReference,GetPotentiallyNestedClassTypeReferenceand (now orphaned)IMemberReferenceAPIs are removed.Validation
GetDocumentationCommentId(the C# dialect oracle).IdStringProbe.il/.xmlpin the C++/CLI dialect: the trimmed disassembly of an MSVC-compiled probe assembly together with the unmodified xml MSVC generated for it (source inIdStringProbe.cpp), assembled at test time. Every member key MSVC wrote must be reachable through the candidates. Notably, MSVC's own documentation of the format is wrong in places: modreq is generated, but|is used only formodreq(IsVolatile)whilemodreq(IsByValue)renders with!.MetadataBuildermodules cover shapes C# cannot express (non-default array bounds, pinned, modifier placement and collision scenarios).M:System.String.Concat(System.String,System.String)through themscorlibfacade verified the forwarder chase.The commits are structured to be reviewable one at a time, each building and tested in isolation.
🤖 Generated with Claude Code