Uh oh!
There was an error while loading. Please reload this page.
Convert managed ilasm to use ANTLR actions instead of visiting the parse tree - #132346
Convert managed ilasm to use ANTLR actions instead of visiting the parse tree#132346jkoritzinsky wants to merge 19 commits into
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib |
| ``` | ||
| ./dotnet.sh build src/tools/ilasm/src/ILAssembler/gen | ||
| ./dotnet.sh build src/tools/ilasm/src/ILAssembler |
There was a problem hiding this comment.
Is it intentional that gen was dropped here?
0fc4042 to
f0b2d00CompareThere was a problem hiding this comment.
Pull request overview
This pull request refactors the managed ILAssembler pipeline to avoid building a full ANTLR parse tree by switching to action-driven parsing (with an unbuffered token stream), and introduces an explicit reference-assembly “contract” to constrain the intended public API surface while keeping ANTLR-generated types out of the supported contract.
Changes:
- Switch
DocumentCompilerto parse withUnbufferedTokenStream, disable parse-tree construction, and drive compilation throughGrammarActions. - Add a custom reference-assembly project under
src/tools/ilasm/src/ILAssembler/ref/and adjust tests to compile against implementation where needed. - Add extensive regression tests for error-tolerant parsing and state-leak prevention across documents/scopes.
Reviewed changes
Copilot reviewed 83 out of 88 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/ilasm/tests/ILAssembler.Tests/TypedefTests.cs | Updates custom-attribute assertions to match new emission/binding behavior. |
| src/tools/ilasm/tests/ILAssembler.Tests/SyntaxTests.cs | Adds corpora for truncated/malformed input to ensure diagnostics are produced instead of exceptions. |
| src/tools/ilasm/tests/ILAssembler.Tests/SourceDirectiveTests.cs | Adds multi-document and malformed language directive tests to ensure debug state doesn’t leak. |
| src/tools/ilasm/tests/ILAssembler.Tests/SecurityTests.cs | Adds permissionset ordering/leak tests; expands coverage for structured security declarations. |
| src/tools/ilasm/tests/ILAssembler.Tests/MethodTests.cs | Adds tests for override ownership and state isolation after malformed nested method headers. |
| src/tools/ilasm/tests/ILAssembler.Tests/InteropTests.cs | Adds coverage for raw marshal blobs and “no state leak” scenarios across fields/documents. |
| src/tools/ilasm/tests/ILAssembler.Tests/ILAssembler.Tests.csproj | Opts into implementation assembly for tests and links additional CLI helper code. |
| src/tools/ilasm/tests/ILAssembler.Tests/FieldTests.cs | Adds tests ensuring trailing .custom directives bind correctly and don’t leak across scopes. |
| src/tools/ilasm/tests/ILAssembler.Tests/ExceptionHandlingTests.cs | Adds ordering/bounds tests for EH region synthesis and catch-type resolution timing. |
| src/tools/ilasm/tests/ILAssembler.Tests/EventTests.cs | Adds test for .event without an explicit type emitting a nil event type. |
| src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTests.cs | Adds tests ensuring truncated/syntax-broken documents don’t leak namespace/type scopes into subsequent documents. |
| src/tools/ilasm/tests/ILAssembler.Tests/DataTests.cs | Adds large bytearray/data declaration tests and leak-prevention tests across documents. |
| src/tools/ilasm/tests/ILAssembler.Tests/CommandLineTests.cs | Adds a Windows sharing-violation retry test for output writing behavior. |
| src/tools/ilasm/tests/ILAssembler.Tests/AssemblyTests.cs | Adds coverage for accepting “legacy library” assembly attribute variants. |
| src/tools/ilasm/src/ILAssembler/TypeName.cs | Removes the old internal TypeName record (migrated into the new semantic model). |
| src/tools/ilasm/src/ILAssembler/StringCharStream.cs | Adds a lightweight ICharStream over string to avoid AntlrInputStream overhead. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/SourceText.cs | Adds contract type for document text + path. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/SourceSpan.cs | Adds contract type for source spans. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/Options.cs | Adds contract type defining compiler options. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/Location.cs | Adds contract type for diagnostic locations. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/DocumentCompiler.cs | Adds contract surface for the compiler entry points. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/Diagnostic.cs | Adds contract surface for diagnostics and ids. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler/CompilationResult.cs | Adds contract surface for serializing compiler output. |
| src/tools/ilasm/src/ILAssembler/ref/ILAssembler.csproj | Introduces the ref-contract project. |
| src/tools/ilasm/src/ILAssembler/PreprocessedTokenSource.cs | Keeps root source on stack at EOF to support parser error recovery on truncated input. |
| src/tools/ilasm/src/ILAssembler/ILAssembler.csproj | Hooks up the new contract project and excludes ref sources from implementation compile. |
| src/tools/ilasm/src/ILAssembler/gen/ilasm-generator.csproj | Disables visitor generation and removes generated visitor/parser compile items from the generator build. |
| src/tools/ilasm/src/ILAssembler/EntityRegistry.cs | Allows events to have an absent type and emits a nil handle accordingly. |
| src/tools/ilasm/src/ILAssembler/DocumentCompiler.cs | Switches from visitor-based traversal to action-driven parsing with parse-tree disabled. |
| src/tools/ilasm/src/ILAssembler/CompatibilitySuppressions.xml | Adds CP0001 suppressions for intentionally unsupported implementation-only public types. |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.Signatures.cs | Adds strongly-typed semantic value model for signatures/types/member refs. |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.MethodBodies.cs | Adds semantic value model for method bodies (EH/debug/data/security/instruction values). |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.Marshalling.cs | Adds semantic value model for marshalling/native type synthesis. |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.Manifest.cs | Adds semantic value model for manifest-level constructs (assembly/resources/typedef/etc.). |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.Declarations.cs | Adds semantic value model for headers/builders used during action parsing. |
| src/tools/ilasm/src/ILAssembler/CILParser.SemanticValues.CustomAttributes.cs | Adds semantic value model for custom attributes and serialization blobs. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Types.References.cs | Implements action helpers for type-name construction and type resolution. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Types.Headers.Actions.cs | Implements action helpers for namespace/class header parsing and attribute synthesis. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Types.cs | Implements scope ownership/unwind to prevent state leaks across declarations. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Signatures.References.cs | Implements action helpers for signature/member reference synthesis and materialization. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Signatures.cs | Provides thin helpers connecting parser contexts to the materializers. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Security.cs | Implements action-driven declarative security parsing and permission-set emission. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.MethodHeaders.Generics.cs | Implements generic parameter name/constraint materialization logic. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.MethodHeaders.cs | Implements action-driven method definition creation/signature emission. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.MethodBodies.cs | Adds label validation and method-name parsing actions. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Members.Fields.cs | Implements action-driven field declaration parsing and emission; tracks pending .custom binding. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.VTable.cs | Implements vtfixup parsing; rejects raw .vtable blob support. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.Typedefs.cs | Implements typedef parsing and alias resolution materialization. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.Resources.cs | Implements action-driven manifest resource declaration materialization. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.References.cs | Implements action-driven assembly reference header/declarations materialization. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.Files.cs | Implements action-driven file declarations and entrypoint tracking. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Manifest.Assembly.cs | Implements action-driven assembly definition parsing, keys, and directive application. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Literals.cs | Implements action-driven parsing for identifiers, dotted names, numeric literals, etc. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Instructions.References.cs | Implements instruction emission paths that require resolving types/members/tokens. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Declarations.Actions.cs | Implements top-level declaration dispatch actions. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Debug.cs | Implements action-driven .line/.language handling and sequence point emission. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Data.cs | Implements streamed .data emission and reference fixups. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.CustomAttributes.Sequences.cs | Implements custom-attribute “sequence” value emission. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.CustomAttributes.Actions.cs | Implements custom-attribute descriptor parsing and blob materialization. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.cs | Adds per-document lifecycle reset for action-driven compilation. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Conversions.cs | Centralizes shared state/diagnostics and attribute application helpers. |
| src/tools/ilasm/src/ILAssembler/Actions/GrammarActions.Bytes.cs | Adds bytearray parsing that tolerates lexer ambiguities and avoids exceptions. |
| src/tools/ilasm/src/ilasm/Program.cs | Switches output writing to use OutputFile.Write. |
| src/tools/ilasm/src/ilasm/OutputFile.cs | Adds retrying output writer to mitigate transient Windows sharing violations. |
| src/tools/ilasm/README.md | Updates documentation to describe the action-driven architecture and ref contract pattern. |
| src/tools/ilasm/ilasm.slnx | Adds the ref contract project to the solution. |
| <ItemGroup> | ||
| <PackageReference Include="Antlr4.Runtime.Standard" Version="$(Antlr4RuntimeStandardVersion)" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Compile Remove="ref\**\*.cs" /> | ||
| </ItemGroup> |
| public sealed class Options | ||
| { |
There was a problem hiding this comment.
This package is internal-only, so API review is not necessary.
f0b2d00 to
0be05e9Compare| [Fact] | ||
| public async Task OutputFile_Write_RetriesTransientSharingViolation() | ||
| { | ||
| if (!OperatingSystem.IsWindows()) | ||
| { | ||
| return; | ||
| } |
0be05e9 to
cd72d2eCompareThere was a problem hiding this comment.
Pull request overview
Copilot reviewed 83 out of 88 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/tools/ilasm/src/ILAssembler/DocumentCompiler.cs:112
ParserErrorListenerusesTokenSource.SourceNameto look upSourceTextinloadedDocuments. For synthetic/missing tokens created during error recovery,TokenSourcemay bePreprocessedTokenSource, whoseSourceNamecan include include-chain decorations (and may not match the raw document keys). Using the underlyingInputStream.SourceNamealigns withLocation.From(...)and should make document lookup robust for recovery tokens.
| public (System.Collections.Immutable.ImmutableArray<Diagnostic>, CompilationResult?) Compile( | ||
| System.Collections.Immutable.ImmutableArray<SourceText> documents, | ||
| System.Func<string, SourceText> includedDocumentLoader, | ||
| System.Func<string, byte[]> resourceLocator, | ||
| Options options) { throw null; } |
Use an unbuffered token stream and disable whole-document parse-tree construction. Retain only bounded declaration and member subtrees while semantic actions compile them, and stream byte arrays directly. Split the compiler actions by IL feature to keep the implementation maintainable and add error-recovery and large-input coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Emit common instruction and method-body forms directly from thin parser actions without constructing temporary parse subtrees. Keep complex operands in isolated bounded subtrees for subsequent stack layers. Preserve instruction diagnostics and deterministic output while reducing large-method assembly time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Synthesize floating literals and composed strings directly from parser rules, and stream switch labels into an action-owned accumulator. Remove the corresponding bounded instruction subtrees while preserving output and error recovery. Also fix odd-length ANSI string padding to match native ilasm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Remove the generic instruction parse-tree island and emit reference, token, type, and calli instructions directly after their operand rules. Each complex operand now retains only its own bounded subtree, preparing the type and signature rules for independent synthesis. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Convert type, signature, class-name, and member-reference rules to compact semantic values so reference instructions no longer require parse-tree subtrees. Preserve internal entity types behind object-valued generated context slots and materialize them through strongly typed action helpers. Keep marshalling as the final bounded signature island. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Convert native marshalling, SAFEARRAY variants, IID parameters, and raw marshal blobs to compact semantic values. Remove the final signature-layer parse subtree while preserving descriptor bytes and recursive native-type ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Replace the generic method-declaration and SEH subtrees with direct method directive actions and synthesized exception-region descriptors. Keep nested scopes and handlers tree-free while preserving catch type and label allocation order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Replace class-member and method-header subtrees with synthesized method, field, property, event, generic, and P/Invoke values. Resolve class-level method overrides at type close so forward method definitions bind like native ilasm. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Replace the top-level declaration, namespace-header, and class-header subtrees with direct dispatch and synthesized type headers. Preserve class attribute, generic constraint, inheritance, interface, and nested scope ordering while leaving shared directives in minimal islands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Convert custom attribute descriptors, named arguments, serialized values, arrays, object sequences, and field/parameter initializers to semantic values. Remove their parse-tree islands while preserving pseudo-attribute lowering and owner binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Stream data declaration items and synthesize declarative security, source mapping, and language directives. Remove their shared parse-tree islands while preserving label fixups, parent ownership, and PDB state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Synthesize assembly definitions and references, files, exported types, resources, vtable fixups, and typedefs. Remove the final parse-tree islands so BuildParseTree remains disabled for the complete parse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Synthesize assembly definitions and references, files, exported types, resources, vtable fixups, and typedefs. Remove the final parse-tree islands so BuildParseTree remains disabled for the complete parse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Disable visitor generation and remove the generated visitor interfaces, parser Accept overrides, explicit forwarding methods, and visitor-only dead code. The parser action pipeline now owns all semantic traversal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Replace the remaining GrammarResult wrappers with direct semantic return values and remove the parse-tree mode stack, which became redundant once tree construction stayed disabled for the entire parse. Inject the action object directly through the generated parser. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Add an explicit ILAssembler reference contract so generated parser and implementation-only types do not define the supported API. Replace object-typed grammar returns and parser accumulation stacks with concrete CILParser semantic values and rule-local builders. Harden every typed return against ANTLR error recovery and add malformed input mutation coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e2b8e8da-8e3f-4a26-9c8b-73857fabfa6c
Retry transient Windows sharing and lock violations while opening the output file, and cover the IL_Conformance race with a focused regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c986c103-8383-4db4-9b11-9be49dc68a05
cd72d2e to
eeb421eCompare| byte[] resourceData = _resourceLocator(header.Alias); | ||
| if (resourceData is null) |
| bool neg = text.StartsWith('-'); | ||
| if (!double.TryParse(text, NumberStyles.Float, CultureInfo.InvariantCulture, out double result)) | ||
| { | ||
| result = neg ? double.MaxValue : double.MinValue; | ||
| } |
| builder.AddEvent( | ||
| evt.Attributes, | ||
| builder.GetOrAddString(evt.Name), | ||
| evt.Type.Handle); | ||
| evt.Type?.Handle ?? default(TypeDefinitionHandle)); |
| } | ||
| } | ||
| private static bool IsSharingViolation(IOException exception) |
There was a problem hiding this comment.
Do other compilers (e.g. Roslyn, linkers, ...) have a retry logic like this? It does not feel right to compensate for OS race conditions between the file creating/deletion and execution in the compiler.
Currently, managed ilasm builds a parse tree for the whole input .il file. This causes massive memory allocations (and OOMs on x86) for some of our larger test assemblies (HardwareIntrinsics AVX512 and HugeArray) that are on the scale of CoreLib. It also causes a massive slowdown due to the allocations.
This PR converts our parse tree visitor to instead be called as separate "actions" during grammar parsing, similar to how native ilasm does in the YACC file. Unlike native ilasm, all of the real logic is in C# files, and the logic in the CIL.g4 file is minimal (function calls into C#).
Because ANTLR4 only emits public types, we limit the exposed public API surface for (expected) internal consumers of the ILAssembler library (such as Roslyn for their test tree) to only the expected public API surface.
Stack created with GitHub Stacks CLI • Give Feedback 💬