Implement label validation and typedef resolution in C# ilasm #123002
Conversation
- Consolidate instruction tokens into main grammar to establish proper lexer precedence over DOTTEDNAME and ID; reorder methodDecl alternatives to match instructions first; add custom attribute handling in method bodies - Implement visitor stubs for debug/symbol directives: VisitLanguageDecl, VisitEsHead, VisitExportHead, VisitExtSourceSpec, VisitFieldInit - Add typedef resolution in ResolveTypeDef for type aliases - Implement label validation: track declared vs referenced labels, report errors for undefined labels at method end - Allow recoverable errors in method bodies to still emit assembly - Add tests for label validation, typedef resolution, and error handling
github-actions
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
am11
added
area-ILTools-coreclr
and removed
needs-area-label
|
Tagging subscribers to this area: @JulieLeeMSFT |
src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTests.cs
Outdated
Show resolved
Hide resolved
src/tools/ilasm/tests/ILAssembler.Tests/DocumentCompilerTests.cs
Outdated
Show resolved
Hide resolved
| private static bool IsRecoverableError(string diagnosticId) | ||
| { | ||
| // Method body diagnostics are recoverable - we emit the assembly but report the error | ||
| return diagnosticId is DiagnosticIds.ByteArrayTooShort | ||
| or DiagnosticIds.ArgumentNotFound | ||
| or DiagnosticIds.LocalNotFound | ||
| or DiagnosticIds.LabelNotFound; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think more of these errors might be recoverable, but I'm not 100% positive. Can we compare against ilasm to see what's recoverable there?
For example, I think GenericParameterIndexOutOfRange should be recoverable (the method signature would be invalid, but should still be emittable).
There was a problem hiding this comment.
I have gone through them by comparing native ilasm and fixed them. We may still be missing a few adjustments, but I haven't found much differences. I think once we address all the TODOs in subsequent PRs, we can run the entire native ilasm test suite and to make it a drop-in replacement (+ improvements). We are getting closer. :)
|
|
||
| public GrammarResult VisitErrorNode(IErrorNode node) => throw new UnreachableException(NodeShouldNeverBeDirectlyVisited); | ||
| public GrammarResult VisitEsHead(CILParser.EsHeadContext context) => throw new NotImplementedException("TODO: Symbols"); | ||
| public GrammarResult VisitEsHead(CILParser.EsHeadContext context) => GrammarResult.SentinelValue.Result; |
There was a problem hiding this comment.
Another TODO that hasn't been implemented yet.
There was a problem hiding this comment.
Now we have a comment explaining why it's just a pass-thru: // esHead is '.line' or '#line' - this is just the keyword, actual parsing is in VisitExtSourceSpec.
|
Can we gather test coverage for ilasm/this change? Test coverage for ilasm should be pretty easy to measure and ideally we would never regress coverage percent. |
3 participants
Consolidate instruction tokens into main grammar to establish proper lexer precedence over DOTTEDNAME and ID; reorder methodDecl alternatives to match instructions first; add custom attribute handling in method bodies
Implement visitor stubs for debug/symbol directives: VisitLanguageDecl, VisitEsHead, VisitExportHead, VisitExtSourceSpec, VisitFieldInit
Add typedef resolution in ResolveTypeDef for type aliases
Implement label validation: track declared vs referenced labels, report errors for undefined labels at method end
Allow recoverable errors in method bodies to still emit assembly
Add tests for label validation, typedef resolution, and error handling