symmetryinvestments / autowrap

Wrap existing D code for use in Python, Excel, C#
BSD 3-Clause "New" or "Revised" License
80 stars 16 forks source link

Csharp - DateTime and Date should be marshalled to C# types #36

Open Laeeth opened 5 years ago

Laeeth commented 5 years ago

If TimeZone proves tricky just do the simplest thing that will mostly work and add to README any warnings.

LightBender commented 5 years ago

The simplest thing to do is to marshal DateTime/SysTime as an ISO 8601 string. In C# a struct can be either blittable and non-blittable, only blittable types can be used with P/Invoke. DateTime/DateTimeOffset are non-blittable types so marshalling them directly is impossible. Marshalling by string will allow us to work with any date/time type whether or not they have timezones. We are lucky that both .NET and D have two different date/time types. C# DateTime is most similar to D's DateTime and DateTimeOffset is most similar to SysTime. We will also probably need to make TimeSpan/Duration work as well, but that can be done with a struct instead of string.

For more on blittable/non-blittable types: https://docs.microsoft.com/en-us/dotnet/framework/interop/blittable-and-non-blittable-types https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/constraints-on-type-parameters#unmanaged-constraint

Laeeth commented 5 years ago

Why not just marshal as a long? DateTime.Ticks() https://docs.microsoft.com/en-us/dotnet/api/system.datetime.ticks?view=netframework-4.7.2

LightBender commented 5 years ago

Because it's an identical amount of code either way and solves the TimeZone problem.

Laeeth commented 5 years ago

There's no TimeZone problem for anything except SysTime. And I am nervous about strings in C# given GC. We can have a lot of dates so they should be fast

LightBender commented 5 years ago

Because strings are immutable in both languages it was easier to treat them not as pointers from a marshalling standpoint, instead they are marshalled as byte-arrays and copied into memory allocated by the receiving side, similar to struct blitting. It was the safest way to marshal strings due to the GC.

Laeeth commented 5 years ago

So let's not use strings for date conversion unless no alternative?

LightBender commented 5 years ago

Here are the reasons to use strings:

Here are reasons to not use Ticks:

I agree that arrays of strings will be slower than arrays of longs, however, I was told that interface speed was not a priority. Is there some specific reason that I should go to the significant extra effort to implement something that will be less flexible while significantly complicating the code generation process and increase the potential for bugs that are guaranteed to be difficult to track down?

I already know @atilaneves's position on multiple code paths for the same thing, so if you really want me to use Ticks, I certainly can, but I want both you and Atila to acknowledge and understand why I am implementing it that way so there is no confusion in the code review.

atilaneves commented 5 years ago

The GC shouldn't be a problem if the strings are only used for serialisation. Neither D or C# would still have a reference to it after the call finished.

LightBender commented 5 years ago

The immutable nature of strings make them a special case as far ownership goes because copies are by definition owned by whichever GC created them. I've attached a PR that explicitly copies the strings in .NET and releases them in D. It wouldn't be a problem in this issue for the reason that @atilaneves stated, but it could potentially be in the case of long-lived strings returned from D. The Phobos toUTF series of methods creates copies implicitly.