Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FSharp.UMX no longer compiles on .NET 7 #12881

Closed
kerams opened this issue Mar 25, 2022 · 10 comments · Fixed by #12892
Closed

FSharp.UMX no longer compiles on .NET 7 #12881

kerams opened this issue Mar 25, 2022 · 10 comments · Fixed by #12892
Assignees
Labels
Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Ready Regression

Comments

@kerams
Copy link
Contributor

kerams commented Mar 25, 2022

Repro steps

Install .NET SDK 7.0.100-preview.4.22175.1 or newer and run this in fsi

[<MeasureAnnotatedAbbreviation>] type Guid<[<Measure>] 'm> = System.Guid

Expected behavior

Compiles.

Actual behavior

error FS3360: 'Guid<'m>' cannot implement the interface 'IComparable<_>' with the two instantiations 'IComparable<Guid>' and 'IComparable<Guid<'m>>' because they may unify.

Known workarounds

Downgrade to 7.0.100-preview.2.22103.2.

@kerams kerams added the Bug label Mar 25, 2022
@dsyme dsyme added Regression Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Mar 28, 2022
@dsyme dsyme self-assigned this Mar 28, 2022
@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2022

So the problem here is that there is specific logic for computing the apparent interfaces of MeasureAnnotatedAbbreviation types, for example consider the library definition:

[<MeasureAnnotatedAbbreviation>] type Guid<[<Measure>] 'm> = System.Guid

What interfaces does Guid<'m> support? Well, the non-unitized System.Guid supports many interfaces such as IComparable<System.Guid>. The unitizing declaration has the effect that interfaces all get rewritten, e.g. to IComparable<Guid<'m>>, substituting System.Guid --> Guid<'m>. This is done for IComparable and IEquatable and nothing else in GetImmediateInterfacesOfType

The problem is, this is only done for a fixed, known set of interface - because it is baking in a set of assumptions about how these interfaces behave with regard to the unit annotations. And in .NET 7, there are a lot of important new interfaces appearing on these primitive types that get used with MeasureAnnotatedAbbreviation, e.g. Guid: now has new interfaces:

.NET 6:

public readonly struct Guid : IComparable, IComparable<Guid>, IEquatable<Guid>, ISpanFormattable

.NET 7:

public readonly struct Guid : IComparable<Guid>, IComparisonOperators<Guid,Guid>, IEqualityOperators<Guid,Guid>, IEquatable<Guid>, IParseable<Guid>, ISpanFormattable, ISpanParseable<Guid>

Here IComparisonOperators<Guid,Guid> and IEqualityOperators<Guid,Guid> themselves include IComparable<Guid>, but that included/inherited interface type is not rewritten.

@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2022

Note the list of interfaces appearing is large, e.g. for System.Double:

public readonly struct Double : 
IAdditionOperators<double,double,double>, IAdditiveIdentity<double,double>, 
IBinaryFloatingPoint<double>, IBinaryNumber<double>, IBitwiseOperators<double,double,double>, 
IComparable<double>, IComparisonOperators<double,double>, IConvertible, 
IDecrementOperators<double>, IDivisionOperators<double,double,double>, 
IEqualityOperators<double,double>, IEquatable<double>, IFloatingPoint<double>, 
IIncrementOperators<double>, IMinMaxValue<double>, 
IModulusOperators<double,double,double>, 
IMultiplicativeIdentity<double,double>, IMultiplyOperators<double,double,double>, 
INumber<double>, IParseable<double>, ISignedNumber<double>, 
ISpanParseable<double>, ISubtractionOperators<double,double,double>, 
IUnaryNegationOperators<double,double>, IUnaryPlusOperators<double,double>

I will admit I find that interface list overkill and I fear for the simplicity of the whole of .NET, but hey. I also fear a little for the performance of the F# compiler as we don't generally expect to see quite so many interfaces of such complexity on primitive types. I wonder if we will start to hit more bottlenecks in interface processing code.

Anyway, the specific technical problem of what to do about these interfaces for "unitized" types is significant and problematic and something we need to solve. For example the unitized double<'m> needs to be careful about these interface types - for example supporting

IAdditionOperators<double<'m>, double<'m>, double<'m>

is fine but

IMultiplyOperators<double<'m>, double<'m>, double<'m>

would be wrong, it should be minimally

IMultiplyOperators<double<'m>, double<'m>, double<'m*^2>

Even that - while sound - is not ideal - as the units should in theory be allowed to be different for the two operators, so it should really be

IMultiplyOperators<double<'m>, double<'n>, double<'m 'n>

But where is 'n quantified? It's not possible.... So indeed it's not clear the operators-as-interfaces stuff in .NET 7 can really apply fully to unitized types in F#. We need to think about this and its ramifications.

@tannergooding You might like to take a look at this. Also it would be good to get an understanding for how many further changes are in the works for these fundamental interface types, as it looks like the F# compiler will need to be taught some understanding of them to get the unit of measure feature to play out as soundly and nicely as possible.

@tannergooding
Copy link
Member

I will admit I find that interface list overkill and I fear for the simplicity of the whole of .NET, but hey.

.NET is largely statically/strongly typed and needs to support a broad range of scenarios and languages. Many of these interfaces are built for extensibility and reusability so that everything can have the right amount of flexibility and you can find the relevant information in the type system.

The hierarchy exists in order to support a broad range of requirements as you need to be able to support and represent things like Complex, Rational, Integer, Binary Floating-Point, Decimal Floating-Point, Fixed Width Floating-Point, etc. It is not significantly different from what other languages provide/expose and it is not dissimilar to the "traits" that other languages/runtimes expose. The main difference is that it directly exposes it as an interface, which is largely how it has to work at the ABI (Application Binary Interface) level.

I also fear a little for the performance of the F# compiler as we don't generally expect to see quite so many interfaces of such complexity on primitive types. I wonder if we will start to hit more bottlenecks in interface processing code.

The runtime was able to workaround the perf implications here and essentially mitigate the cost altogether. One of the ways it did this was to specialize the primitive types. I believe F# should be able to achieve the same and likewise handle the potential perf implications of types implementing such complex hierarchies.

Also it would be good to get an understanding for how many further changes are in the works for these fundamental interface types, as it looks like the F# compiler will need to be taught some understanding of them to get the unit of measure feature to play out as soundly and nicely as possible.

The next API review is tomorrow and it will result in some additional changes around the name/shape of certain APIs, as well as the shuffling about of various APIs to better fit the feedback we've gotten from the community.

Here IComparisonOperators<Guid,Guid> and IEqualityOperators<Guid,Guid> themselves include IComparable, but that included/inherited interface type is not rewritten.

This isn't quite right. The hierarchy is:

IComparisonOperators<TSelf, TOther>
|- IComparable
|- IComparable<TOther>
|- IEqualityOperators<TSelf, TOther>
   |- IEquatable<TOther>

That is IComparisonOperators implements IComparable, IComparable<TOther>, and IEqualityOperators.
IEqualityOperators then implements IEquatable.

For basically all of the interfaces, there is largely a single "root" interface implemented by these types and that pulls in "everything else". In the case of most of the integer types, this is IBinaryInteger<TSelf>. In the case of most of the IEEE 754 floating-point types, this is IBinaryFloatingPoint<TSelf>. There are only a couple interfaces which don't fit into this hierarchy and which are "distinct".

@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2022

The complexity is well-motivated, of course, as complexity always is. The question here is the impact of complexification.

Anyway, this is not the place to discuss this - we should focus on the specific bug here, plus the general question of "what's going to go wrong when F# sees this new metadata". But as a general point: I fear for a platform that doesn't have simplicity as the primary core virtue - one valued more highly than extensibility, reusability, flexibility etc. And what's being added here is most definitely not simple.

...handle the potential perf implications of types implementing such complex hierarchies.

I'd imagine that's right, though beware there's a risk we're going to discover this through a series of perf regressions only experienced in particular scenarios, or through general reduction in type checking performance or IDE tooling.

There's some risk there will be a quadratic or worse performance reduction in type inference, again only detected once starting to use .NET 7 for real.

Note no one is specifically testing or monitoring for such performance regressions, they may just start happening once F# starts being used against .NET 7 metadata, and perhaps only in corner cases. It's not certain this will happen, it's just a risk. If we increase the complexity of any metadata from say 10 to 100 interfaces (taking into account the full hierarchy) it's pretty certain to cause some kinds of problems.

@dsyme
Copy link
Contributor

dsyme commented Mar 28, 2022

Here is a .NET 6 repro for the specific problem with F# user-defined unit-of-measure-annotated types where the underlying type supports an interface derived from IComparable or IEquality. I will fix this to unblock.

open System
type IDerivedComparable<'T> =
    inherit IComparable<'T>

type Prim() =
    interface IDerivedComparable<Prim> with 
        member x.CompareTo(y) = 0

[<MeasureAnnotatedAbbreviation>]
type Prim<[<Measure>] 'm> = Prim

// Check that Prim<'m> supports the unit-annotated IComparable interface
let f1 (x: Prim<'m>) = (x :> IComparable<Prim<'m>>)
let f2 (x: Prim<'m>) = (x :> IDerivedComparable<Prim<'m>>)

@dsyme
Copy link
Contributor

dsyme commented Mar 29, 2022

The ix for the immediate issue is in #12892

Looking beyond, my assessment is that the .NET 7 hierarchy just doesn't jive with units of measure (e.g. would need a complete redesign were C# ever to support units of measure in its type system). In practice I believe this means .NET and C# will never support units of measure in the type system (except perhaps via some kind of separate code analysis).

As an example, consider some of the interfaces appearing in the .NET 7 mega-hierarchy:

  • An interface implementation like IAdditionOperators<SomeType,SomeType,SomeType> would be adjusted to IAdditionOperators<SomeType<'m>,SomeType<'m>,SomeType<'m>>. That's ok
  • For an interface implementation like IMultiplyOperators<SomeType,SomeType,SomeType> the unitized version of this is not expressable as a unitized instantiation of this same interface declared on the type itself, since the left and right of the multiply can take different units, multiplied in the result IMultiplyOperators<SomeType<'m1>, SomeType<'m2>, SomeType<'m1 'm2>>. It is fundamentally not possible to capture the "trait" of unitized multiplication using an object-oriented interface without much more complexifying additions such as higher-kinded types, plus a re-design of the hierarchy to use them.

Why doesn't F# hit this problem today with its generic math capabilities? It's because F# uses structural SRTP constraints and these are able to capture the patterns involved in both non-unitized and unitized types. For example, in F# today is is possible to generalize arithmetic and then later satisfy the SRTP constraints that arise with either unitized or non-unitized types:

let inline someGenericMath x y = x * y + x * y 

let g0 (x: float) (y: float) = someGenericMath x y
let g1 (x: float<'u>) (y: float<'v>) = someGenericMath x y
let g2 (x: int<'v>) (y: int<'v>) = someGenericMath x y

with inferred types:

val inline someGenericMath:
  x:  ^a -> y:  ^b ->  ^d
    when ( ^a or  ^b) : (static member ( * ) :  ^a *  ^b ->  ^c) and
          ^c: (static member (+) :  ^c *  ^c ->  ^d)
val g0: x: float -> y: float -> float
val g1: x: float<'u> -> y: float<'v> -> float<'u 'v>
val g2: x: int<'v> -> y: int<'v> -> int<'v ^ 2>

Here SRTP constraints arise from the use-site of each operator +, * in the function someGenericMath. These constraints are then resolved to concrete operators (whether unitized or non-unitized) in g0, g1, g2.

@tannergooding
Copy link
Member

Note no one is specifically testing or monitoring for such performance regressions, they may just start happening once F# starts being used against .NET 7 metadata, and perhaps only in corner cases.

Notably we do have C# tests (and in some cases IL tests) covering the impact of startup, devirtualization, casting, type checks, etc. These generally live in https://github.com/dotnet/performance and we do weekly triage of these for Windows, Linux, and MacOS and covering x86/x64 and Arm32/Arm64.

If there are F# scenarios outside what we are already testing, then we should definitely look at adding those as well so we can test the impact over time and likewise prevent regressions for important F# specific scenarios (whether specific to static abstracts in interfaces or other key scenarios outside this).

It's not certain this will happen, it's just a risk. If we increase the complexity of any metadata from say 10 to 100 interfaces (taking into account the full hierarchy) it's pretty certain to cause some kinds of problems.

For certain and we specifically order interface declarations to reduce this cost for the most common usage scenarios. This is one of the reasons why IComparable<T> and IEquatable<T> are generally the "first" interfaces in the BCL source code. Things like IConvertible, the non-generic IComparable, and other interfaces are typically ordered after since they are less frequently used.

Various common coding patterns also help reduce the costs. Constrained generics can mitigate checks, generics over value types will mitigate checks (the JIT specializes in this case), and basically any other place where the type can be statically determined will likewise have the cost reduced or completely mitigated (JIT time constant). The remaining cases are largely where you have generics over a reference type or some base type (including object) that can't be statically determined to implement the interface.

We'll continue carefully monitoring the impact of these things and common patterns users are utilizing around here to try and ensure that these costs stay down and aren't negatively impactful.

@dsyme
Copy link
Contributor

dsyme commented Mar 29, 2022

Notably we do have C# tests (and in some cases IL tests) covering the impact of startup, devirtualization, casting, type checks, etc.

My concern is compilation performance (and in particular type inference and name resolution performance both in compiler and in IDE, i.e. any logic in toolchains that involves walking the hierarchy of interfaces looking at metadata along the way). For example a compiler might search the hierarchy for, say IDisposable, or IEnumerable, or might search for name resolutions. All of this will be at least slightly impacted in some scenarios - more information, more cost - but it's unclear to me if there's any critical case where this occurs on a main path for compilation.

The performance of generated code is not a concern I have, I'm sure you'll look after that.

@tannergooding
Copy link
Member

Looking beyond, my assessment is that the .NET 7 hierarchy just doesn't jive with units of measure (e.g. would need a complete redesign were C# ever to support units of measure in its type system). In practice I believe this means .NET and C# will never support units of measure in the type system (except perhaps via some kind of separate code analysis).

If there are specific suggestions on tweaks we could make to the surface area to better support F# or F# specific features, then I'm all ears.

We need to support a range of concepts including those that are outside "standard arithmetic". This includes concepts like Complex = Complex * double, Vector<T> = Vector<T> * T, DateTime = DateTime + TimeSpan, etc. Likewise, it needs to be representable in the current IL metadata and cannot rely on external metadata resources embedded independently.

This representation ensures that a wide variety of languages are able to utilize the relevant metadata and features and ensures optimizations can happen at any stage of the development process (whether manually done by the user, handled via compiler optimizations, or handled via JIT/AOT optimizations in processing the IL). It also ensures that the full breadth of the .NET stack can be represented as can concepts that programmers need to deal with that mathematicians may not (this is why we don't represent or support concepts such as commutativity, associativity, distributivity, rings, groups, fields, etc).

@dsyme
Copy link
Contributor

dsyme commented Mar 29, 2022

If there are specific suggestions on tweaks we could make ...

I don't have any concrete suggestions - Generic math is basically a bottomless quagmire of complexity as more and more dimensions of the problem are considered - and what you have is already at the upper limit of any reasonable complexity boundary - adding unitization would be many bridges too far.

As you've mentioned, .NET 7 uses explicit-declaration-of-traits-via-interfaces (rather than, say, implicit trait instances arising from the declarations themselves, as in SRTP), and expresses those declarations using the existing C# and IL interfaces. It is just not possible to properly support unitized types within those limitations (without adding higher-kinded types at very least - and all the complexity that comes with using that).

For example of why higher-kinded typed would be needed - IMultiplyOps would have to become (in C# syntax)

interface IMultiplyOps<UnitizableType1, UnitizableType2, UnitizableType3>
{
    UnitizableType3<Unit1*Unit2>  op_Multiply<Unit1, Unit2>(UnitizableType1<Unit1> x, UnitizableType2<Unit2> y)
}
struct Double<Unit>: IMultiplyOps<Double<_>, Double<_>, Double<_>>
{
    Double<Unit1*Unit2>  op_Multiply<Unit1, Unit2>(Double<Unit1> x, Double<Unit2> y)
...
}

Note

  • The unit-friendly versions of op_Multiply must be generic in the units
  • UnitizableType1 etc are all higher-kinded
  • Unit1*Unit2 is some imagined C# syntax for the multiplication of two units (kg * m for example).
  • Double<_> is some imagined C# syntax for instantiating a thing taking a higher-kinded type
  • A similar story plays out for some other operations.

Anyway none of this is expressible within .NET or C# or F#.

I think on the whole we just won't worry about this. F# will still play nicely enough with the non-unitized hierarchy you have and the type inference will work well. We'll just make it clear that there are limitations when combining the feature with units of measure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Ready Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants