-
Notifications
You must be signed in to change notification settings - Fork 6k
fixed a compile-time error in type-marshalling.md #48780
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
Conversation
this change adds the `out` keyword to fix such compile-time error as well as inlines the variable declaration for simplicity and renames the variable from `st` to `systemTime` for both clarity and consistency.
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.
Thank you for this improvement! Do you think you can improve it some more? As it is, you can't really even copy-paste this code into a code file and have it work, there are a few errors such as it has to be declared in a partial class and a scope modifier needs to be added too.
If not, I'll take your improvement and iterate on it myself, then merge.
Thanks again!
I'll do just that sensei > ~ < |
It doesn't have to be "Main" it could just be a utility class with a method named |
this change makes the code block ready-to-run for ease of use with copying.
@dotnet-policy-service agree |
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.
Beautiful! Thanks!
@adegeo I also considered other changes, regarding the first block, here's what I had in mind: using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
internal static partial class Interop
{
[LibraryImport("SomeNativeLibrary.dll")]
internal static partial int MethodA([MarshalAs(UnmanagedType.LPStr)] string parameter);
// or
[LibraryImport("SomeNativeLibrary.dll")]
internal static partial int MethodB([MarshalAs(UnmanagedType.LPUTF8Str)] string parameter);
// or
[LibraryImport("SomeNativeLibrary.dll")]
internal static partial int MethodC([MarshalUsing(typeof(Utf8StringMarshaller))] string parameter);
// or
[LibraryImport("SomeNativeLibrary.dll", StringMarshalling = StringMarshalling.Utf8)]
internal static partial int MethodD(string parameter);
} this is to complete the set of options originally proposed. I'd also like to note that one of the originally suggested approaches behaves differently in semantics, and that is that is worrisome to me since the documentation states that:
followed by the code block, which again, the first method in it uses the |
@ophura would you mind opening an issue with your suggestion? This way we can tag the engineer and get a discussion going. Go to https://learn.microsoft.com/en-us/dotnet/standard/native-interop/type-marshalling (you won't see your updates just yet) and use the link at the bottom of the page to give feedback on GitHub. This will create an issue and associate with that article. |
Summary
this change adds the
out
keyword to fix such compile-time error as well as inlines the variable declaration for simplicity and renames the variable fromst
tosystemTime
for both clarity and consistency.additionally, it removes the
string[] args
parameter from the method signature as it isn't required and also unused.Internal previews