[mono] Fix detecting [DisableRuntimeMarshalling] in MarshalingPInvokeScanner. #112981
Conversation
This file contains 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
…Scanner. Fix detecting the [DisableRuntimeMarshalling] attribute in MarshalingPInvokeScanner for assemblies that reference the attribute from another assembly (i.e. any assembly except System.Runtime.dll). Fixes dotnet#112980.
There was a problem hiding this comment.
PR Overview
This PR addresses the issue of detecting the [DisableRuntimeMarshalling] attribute when it is referenced from assemblies other than System.Runtime.dll. The changes refactor inline string comparisons into a new helper method and apply it in both type definition and member reference contexts.
- Introduced the IsDisableRuntimeMarshallingAttribute helper method.
- Replaced inline attribute checks with the new helper method for clearer logic.
- Updated attribute checks for both direct and member reference cases.
Reviewed Changes
| File | Description |
|---|---|
| src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs | Refactored attribute detection to improve consistency while fixing the attribute resolving issue. |
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs:107
- Ensure unit tests cover scenarios for both direct type definition and member reference attribute detections using IsDisableRuntimeMarshallingAttribute to confirm the fix works under all expected conditions.
private bool IsDisableRuntimeMarshallingAttribute (MetadataReader mdtReader, StringHandle ns, StringHandle name)
src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs
Outdated
Show resolved
Hide resolved
…PInvokeScanner.cs Co-authored-by: Theodore Tsirpanis <teo@tsirpanis.gr>
|
lgtm but there are a few code style errors |
akoeplinger
approved these changes
Mar 3, 2025
|
@akoeplinger looks like the failure is unrelated? is there anything else I need to do to get this in? |
no, I just forgot to merge, sorry :D |
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.
Fix detecting the [DisableRuntimeMarshalling] attribute in
MarshalingPInvokeScanner for assemblies that reference the attribute from
another assembly (i.e. any assembly except System.Runtime.dll).
Fixes #112980.