-
Notifications
You must be signed in to change notification settings - Fork 202
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
Cleanup marshalling handling #1102
Cleanup marshalling handling #1102
Conversation
src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs
Outdated
Show resolved
Hide resolved
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/NativeCallManagedComVisible/**/*"> | ||
<Issue>https://github.com/dotnet/runtimelab/issues/155: COM</Issue> | ||
</ExcludeList> | ||
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/SizeParamIndex/**/*"> | ||
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/SizeParamIndex/ReversePInvoke/**/*"> |
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.
This folder has separate issues IMO. Maybe warrant to change Issue number here.
@MichalStrehovsky I hope that I properly understand direction which should I go. I have some doubt points (like adding new ExceptionStringId) and miss new exception messages. |
302c163
to
751e588
Compare
src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/TypeLoaderExceptionHelper.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs
Outdated
Show resolved
Hide resolved
751e588
to
ea76dd0
Compare
src/coreclr/nativeaot/System.Private.CoreLib/src/Resources/Strings.resx
Outdated
Show resolved
Hide resolved
Related to dotnet#167 and Closes dotnet#168
1db2bad
to
53f9283
Compare
src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalHelpers.Aot.cs
Outdated
Show resolved
Hide resolved
/azp run runtimelab-nativeaot Pri-0 Tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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!
@jkotas thanks for your patience. I appreciate a lot that you help me push this through. |
@@ -434,9 +434,6 @@ public static partial class MarshalHelpers | |||
{ | |||
case NativeTypeKind.Array: | |||
{ | |||
if (isField || isReturn) |
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.
This will need more refinement. The IsBlittable check is failing with stack overflow on structs that contain arrays of itself. dotnet/runtime#53650 (comment)
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.
Where should I make changes? In NativeAOT or I can go directly to crossgen? Probably for me would be more familiar to work with NativeAOT codebase, but if crossgen similarly easy to start with I can go there.
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.
Manish rolled back this line in dotnet/runtime#53760 . It is fine for you to work on this in NativeAOT codebase.
Related to #167 and
Closes #168