Add RequiresUnreferencedCode to DefinePInvokeMethod#56537
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
| public System.Reflection.Emit.MethodBuilder DefineGlobalMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.CallingConventions callingConvention, System.Type? returnType, System.Type[]? requiredReturnTypeCustomModifiers, System.Type[]? optionalReturnTypeCustomModifiers, System.Type[]? parameterTypes, System.Type[][]? requiredParameterTypeCustomModifiers, System.Type[][]? optionalParameterTypeCustomModifiers) { throw null; } | ||
| public System.Reflection.Emit.MethodBuilder DefineGlobalMethod(string name, System.Reflection.MethodAttributes attributes, System.Type? returnType, System.Type[]? parameterTypes) { throw null; } | ||
| public System.Reflection.Emit.FieldBuilder DefineInitializedData(string name, byte[] data, System.Reflection.FieldAttributes attributes) { throw null; } | ||
| [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Constructors of SafeHandle or blittable classes passed as arguments or returned can be trimmed if not referenced directly.")] |
There was a problem hiding this comment.
@MichalStrehovsky @vitek-karas @sbomer @AaronRobinsonMSFT - Is it only constructors that could be an issue? Or are there other things we should let the user know about?
If it is only constructors that are the concern, it is a bit confusing to say "passed as arguments", because the marshalling code shouldn't call the ctors of SafeHandle or blittable classes on the way "in" to a P/Invoke. Only on the way "out".
There was a problem hiding this comment.
Only on the way "out".
Yep. The general guidance here any type used in by-refs (i.e., ref, out) or as return arguments should have a non-parameterized ctor. I've no opinion on the message in this case. I will say the DllImport source generator is coming up with guidance in this area as well. The interop team has also updated the official documentation too - https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.safehandle#notes-to-implementers.
/cc @jkoritzinsky
There was a problem hiding this comment.
Could we make the message more like "P/Invokes may use reflection to access a parameterless constructor of any return values or out parameters"?
There was a problem hiding this comment.
Or "P/invoke marshalling may dynamically access members that could be trimmed". I don't know if it helps being specific and listing parameterless constructors - I'm not sure if we're covering all the possible cases with this message. E.g. there's also COM marshalling that this could emit.
ghost
locked as resolved and limited conversation to collaborators
5 participants
Fixes: #45703