Xamarin-macios: CTRunDelegate incorrectly uses float instead of nfloat

[chamons]

See: https://github.com/xamarin/xamarin-macios/pull/5131

The binding is not 64-bit safe, we missed it in audit.

[VincentDondain]

Fair breaking change IMHO (:

[rolfbjarne]

It's possible to fix this without doing a breaking - in any case this whole binding file can be significantly improved.

[Therzok]

A few things to note if these bindings are supposed to be high performance and really good codegen:

• Reflection here should be removed: https://github.com/xamarin/xamarin-macios/blob/bc492585d137d8c3d3a2ffc827db3cdaae3cc869/src/CoreText/CTRunDelegate.cs#L116

• Virtual methods might not be able to be devirtualized, maybe a recommendation that subclasses should be sealed or have sealed GetAscent, GetWidth, GetHeight methods.

• The callbacks are not inlinable. The self null check itself is preventing this due to mono inliner behaviour. Maybe apply aggressive inlining (not sure about interaction with pinvoke wrappers)

[rolfbjarne]

• Reflection here should be removed

Agreed.

• Virtual methods might not be able to be devirtualized

We have a linker step that is able to devirtualize virtual methods.

• The callbacks are not inlinable.

The native-to-managed transition will completely overshadow any inlining benefits (and how does even inlining come into play when the method is called from native code?)

[Therzok]

The named-to-managed transition behaviour implemented via a pinvoke wrapper generated that calls the JITed managed method, right? That would be an extra call per pinvoke.

[spouliot]

I don't see why a breaking change would be needed - we control the managed API and there's code in between (us and native) where the conversion can be hidden.

It's extra work so we can have it fixed _properly_ under XAMCORE_4_0