search

theraot / Theraot

Backporting .NET and more: LINQ expressions in .net 2.0 - nuget Theraot.Core available.
MIT License
158 stars 30 forks source link

Add Span and Memory classes. #138

Closed NN--- closed 3 years ago

theraot commented 3 years ago

This looks good. And once more thank you taking the time.

It needs tests for slice. Could be based on the examples here: https://docs.microsoft.com/en-us/dotnet/api/system.span-1?view=net-5.0#spant-and-arrays

Do we need to define TARGET_64BIT?

I have used IntPtr.Size == 8 elsewhere.

NN--- commented 3 years ago

Don’t know about target 64, I just copied the code. It can be == 8.

NN--- commented 3 years ago

Didn’t have time for tests:) There are a lot of things to tests there.

NN--- commented 3 years ago

What do you think about unsafe code? Let’s allow it in the library. There is no additional safety of doing Marshal.ReadInt32 over int* p. More than that, pointers are type safer :)

NN--- commented 3 years ago

@theraot So what is the decision about unsafe ?

theraot commented 3 years ago

@NN--- I found it: https://github.com/theraot/Theraot/pull/2 @qwertie

I needed Theraot.Core to have a "strong name" because any DLL or EXE that has a strong name must only reference assemblies that have a strong name.

NN--- commented 3 years ago

I guess it is better to create a separate NuGet with unsafe then.

NN--- commented 3 years ago

I am not sure that strong name requires safe assembly. mscorlib is not a safe and yet has strong name. The FullTrust argument is totally correct, luckily it is not relevant in .NET Core does in older .NET Frameworks :( It seems that the only good solution is to have a separate package for Span/Memory. The drawback of this solution is that you cannot have Span/Memory overloads in the Theraot library interfaces and solve it only via extensions methods.

NN--- commented 3 years ago

@theraot What is your suggestion here ? I think the only solution is a separate NuGet or 2 NuGets for Theraot which is even less convenient.

theraot commented 3 years ago

Do you know if Nuget allows to push version numbers older than the latest?

This is what I'm thinking: The demand for Span and Memory goes up, and the demand for not unsafe build goes down. So, we increment version number, and turn on unsafe (and we don't put conditional compilation or anything like that)…

Now, for people on an environment where they need safe builds only, they would have to stay on the old version. So that works. If somebody still need them, I would like to still be able to bugfix those. Then the bugfixes could be version numbers between the current one and whatever version number we give the first unsafe build.

Now, everything else being equal, if we need to release a safe build, we branch, insert conditional compilation to remove whatever has unsafe code, and publish that.

theraot commented 3 years ago

I think I'll finish one more release before that. I haven't release a version fixing the ConcurrentDictionary issue. And doing this with that bug open will, well, bug me.

NN--- commented 3 years ago

What is the plan? Move to unsafe and require FullTrust ? Another option to create 2 different libraries , however I don’t think FullTrust is a problem.