-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Tweaks to warning formatting #118797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Tweaks to warning formatting #118797
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves warning and error message formatting in the AOT compiler by adding assembly information and fixing MSBuild formatting issues. The changes address GitHub issue #118788 to provide clearer diagnostic messages when methods will always throw exceptions.
Key changes:
- Adds assembly name information to error/warning messages about methods that will always throw
- Includes extra space after "ILC" prefix to fix MSBuild formatting compatibility
- Refactors code to extract assembly name consistently for both resilient and non-resilient modes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| RyuJitCompilation.cs | Extracts assembly name and includes it in error/warning messages for methods that always throw |
| MessageContainer.cs | Adds trailing space to "ILC" origin string to fix MSBuild formatting issue |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| if (exception != null) | ||
| { | ||
| var mdOwningType = method.OwningType as MetadataType; | ||
| string owningAssembly = mdOwningType?.Module?.Assembly?.GetName()?.Name ?? "<unknown>"; |
Copilot
AI
Aug 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The assembly name extraction logic uses multiple null-conditional operators in sequence, which could be simplified for better readability. Consider extracting this into a helper method or using a more explicit approach to handle the null cases.
Copilot uses AI. Check for mistakes.
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageContainer.cs
Show resolved
Hide resolved
|
What does it print? It is not obvious why we want to include the owning assembly in one message, but not the other. |
|
Should we fix formatting that happens inside LogError to include assembly name instead, so that all uses of LogError get this improvement? |
Fixes #118788
Sample output after this change:
Note that the error message is only reachable if the
--resilientargument is missing. Otherwise this prints a non-error/non-warning message that already includes the assembly name--resilientis passed by default and there is an undocumented opt out.Cc @dotnet/ilc-contrib