theraot / Theraot

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

Change `DynamicMethod` constructor to one that actually exists pre-.NET 3.5. #153

Closed clintonmead closed 3 years ago

clintonmead commented 3 years ago

The DynamicMethod constructor call doesn't exist in at least some versions of .NET 2.0 on Windows XP, and the documentation suggest this method doesn't exist until .NET 3.5, so I've replaced with a constructor call which I think will achieve the same.

NN--- commented 3 years ago

I see the constructor here: https://docs.microsoft.com/en-us/dotnet/api/system.reflection.emit.dynamicmethod.-ctor?view=netframework-2.0#System_Reflection_Emit_DynamicMethod__ctor_System_String_System_Type_System_Type___System_Boolean_

clintonmead commented 3 years ago

Yes, but read the note in the documentation:

image

theraot commented 3 years ago

You might have found a bug in the documentation. While it is correct that it says that the constructor was introduced in .NET 3.5, it also says it applies to .NET 2.0. So it contradicts itself.

image

Now, you say this is a problem in Windows XP. Which makes me suspect of the Service Packs. Are you able to install .NET 2.0 Service Pack 2.0? Does it solve the problem?

Finding what was introduced in service packs is not easy.

clintonmead commented 3 years ago

Moving to the latest service pack might work, but I can't generally control the service pack I install on. I've had a similar problem in the past btw with the one argument version of WaitOne so you might want to avoid that also (I've just noticed it's used in a few places in Theraot).

Could you avoid this constructor for .NET 2 builds? I understand it's difficult to determine the exact service pack being used.

Perhaps at least catch the method not found exception and use my approach as a fallback if you don't want to affect .NET 2 installs which have the correct constructor available?

theraot commented 3 years ago

I had a look at the source. It would use a dynamic assembly when no module is specified, but we can't use AssemblyBuilder.DefineDynamicAssembly.

Based on that, this plan: .NET 2.0 and .NET 3.0 targets will use reflection to check if the overload exist. Cache the result so we don't check every call. If the overload exists use it. Otherwise, use AssemblyBuilder.DefineDynamicModule, and they should cache that module so they don't do that every call.

clintonmead commented 3 years ago

That sounds like a good plan. Do you want me to adjust my PR to do that?

theraot commented 3 years ago

@clintonmead It is done. See the linked commit https://github.com/theraot/Theraot/commit/8e71dca829978f0daf1c408b0eacf27a469e9248

I also reviewed the use of WaitOne: https://github.com/theraot/Theraot/commit/fbce4fc7c57dcd27117221fcc81399676b24bb97

theraot commented 3 years ago

@clintonmead Pushed Nuget 3.2.5.