[Out] parameters no longer added to return tuple - #1308
Conversation
Sorry, something went wrong.
…ed in addition to the regular method return value (unless they are passed with `ref` or `out` keyword).
|
I think this was included for compatibility with IronPython. How would you handle out parameters that still have automatic conversions like int and str? |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #1308 +/- ##
===========================================
- Coverage 87.88% 74.04% -13.85%
===========================================
Files 1 1
Lines 289 289
===========================================
- Hits 254 214 -40
- Misses 35 75 +40
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Sorry, something went wrong.
|
@filmor this is the confusing part. This change does not affect GetInt(out int val) functions. It affects GetInt([Out] int val) functions, where [Out] parameter does nothing. The only real use case I see is that in Read([Out] byte[] buf) [Out] indicates, that the data will be put into the buf. The data can not be put into string or int without ref int/out int, and these are handled separately. This annotation is normally meant for P/Invoke marshaling scenarios, where you have to add [Out] to byte[] arr into PInvoke signature for data to be properly marshaled back into arr upon exiting C function. I am not even sure why MemoryStream.Read has this annotation except for informational purposes. I believe passing Python lists in place of buf is why it was added, but considering lack of [Out] annotations in most places, I don't think it is a good thing to rely on. |
Sorry, something went wrong.
|
For an instance of API difference between implementations in regards to [Out] attribute: In .NET Core MemoryStream.Read has none: https://github.com/dotnet/runtime/blob/5432a66018c2499b88a54276f468ea903ed2b8de/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs#L333 In Mono it is present for COM-compatibility: https://github.com/mono/mono/blob/c5b88ec4f323f2bdb7c7d0a595ece28dae66579c/mcs/class/referencesource/mscorlib/system/io/memorystream.cs#L339 Without this change the same Python code would not work with both Mono and .NET Core. |
Sorry, something went wrong.
|
Thanks for the explanation. I ran into this problem while testing .NET Core and couldn't really make sense of it, wasn't aware of [Out]. |
Sorry, something went wrong.
Parameters marked with ParameterAttributes.Out (aka [Out]) are no longer returned in addition to the regular method return value (unless they are passed with ref or out keyword).
What does this implement/fix? Explain your changes.
As it is now possible to pass raw .NET objects to .NET methods, there's no need to marshal [Out] parameters back to Python manually. Python users can simply access modified object's data directly.
Any other comments?
It was confusing to see read, _ = stream.Read(buff, 0, buff.Length) in the test code, as Read method does not really have any out or ref parameters. It also failed on some .NET implementations, as the corresponding parameter does not always have an [Out] attribute.
Related issues
This should unblock #1307 (tests there are failing because of this difference).
Checklist
Check all those that are applicable and complete.