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

Type extension issue with allows ref struct #18001

Open
bent-rasmussen opened this issue Nov 14, 2024 · 13 comments
Open

Type extension issue with allows ref struct #18001

bent-rasmussen opened this issue Nov 14, 2024 · 13 comments
Assignees
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Area-Compiler-ImportAndInterop import of .NET DLLs and interop Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Milestone

Comments

@bent-rasmussen
Copy link

A type extension on Seq<'T> does not work. The compiler appears to expect 'T to be constrained but I do not know how.

The code below used to work in prior versions of .NET up until .NET 9.0.0-rc2. Starting with .NET 9.0.100 the code is broken. The new compiler breaks helper type extensions at the bottom of our software stack.

Repro steps

Provide the steps required to reproduce the problem:

  1. Install .NET SDK 9.0.100
  2. Create a F# library project targeting .NET 9
  3. Use the following code in Library.fs:
module SeqExtensions

type System.Collections.Generic.IEnumerable<'T> with

    member _.Foo = ()

Expected behavior

Compilation errors.

Actual behavior

Compilation errors:

  • Library.fs(3, 6): [FS0957] One or more of the declared type parameters for this type extension have a missing or wrong type constraint not matching the original type constraints on 'IEnumerable<_>'
  • Library.fs(3, 45): [FS0341] The signature and implementation are not compatible because the type parameter 'T' has a constraint of the form 'T: allows ref struct but the implementation does not. Either remove this constraint from the signature or add it to the implementation.

Known workarounds

  • Not using type extensions
  • Not using .NET 9 and F# 9
@github-actions github-actions bot added this to the Backlog milestone Nov 14, 2024
@T-Gro T-Gro self-assigned this Nov 14, 2024
@T-Gro T-Gro added Regression Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Nov 14, 2024
@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2024

This is indeed a regression.

The reason why F# does not have syntax for authoring this constraint is because F# has no mechanisms to enforce it.
That is also valid for type extensions - F# does not have checks in place to prevent you from e.g. allocating an array of 'T, which is however not allowed for byref types.

For the case of type extensions we can pretend the constraint is not there, but byref instantiations will then fail at runtime if the usage of T does not permit byref types - I am not sure if there is a way to resolve this issue other than:

  • Allowing it, but then potentially crashing when T is instantiated with a byref type
  • Fully doing "allows byref"-authoring support for F# code as well

@T-Gro T-Gro added Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Area-Compiler-ImportAndInterop import of .NET DLLs and interop labels Nov 14, 2024
@bent-rasmussen
Copy link
Author

Okay, it does not sound like there is an (acceptable) easy fix. If that is the case, then I guess we should probably focus our efforts on refactoring our codebase to not use this kind of type extension. Switching to extension methods perhaps. It makes me uneasy but what to do.

@vzarytovskii
Copy link
Member

Ugh, it's a nasty issue, unfortunately the closest servicing window is in January

@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2024

Okay, it does not sound like there is an (acceptable) easy fix. If that is the case, then I guess we should probably focus our efforts on refactoring our codebase to not use this kind of type extension. Switching to extension methods perhaps. It makes me uneasy but what to do.

Just for my understanding, is IEnumerable the major pain point, or are there others at the intersection of : ".NET 9 added allows ref struct" && "Its actually useful for F# to do type extensions for them" ?

@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2024

Okay, it does not sound like there is an (acceptable) easy fix. If that is the case, then I guess we should probably focus our efforts on refactoring our codebase to not use this kind of type extension. Switching to extension methods perhaps. It makes me uneasy but what to do.

I am thinking if a middle ground is possible:

  • Allowing you to extend types which are originally "allows ref struct", not doing any byref types analysis in that generic code (because F# right now does not have it for generic code), and then NOT allowing to use this extension for type instantiations which are byref types.

Would there be some value in this compromise?

i.e. the type extension could change the constraints (in this case dropping "allows ref struct"), but the type extension would then reduce availability for consumption compared to plain IEnumerable<T>.

@bent-rasmussen
Copy link
Author

I'm counting 92 type extensions involving generics. Not sure how many members, I'm also not sure how many of these are problematic. But I will start fixing.

@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2024

I was especially at the ones being "problematic", i.e. the generic parameter at the interface level in .NET BCL has been marked as "allows ref struct" between .NET 8 and .NET 9.

No matter what the outcome is, IEnumerable<T> alone is a reason to address this with priority.

@bent-rasmussen
Copy link
Author

bent-rasmussen commented Nov 14, 2024

I think there is definitely value in your middle ground suggestion, as a first step. We are using type extensions with IEnumerable<T>, ImmutableArray<T>, IObservable<T>, and other collection types. I'm not sure how many are affected by the new anti-constraint, if that is the correct term.

@bent-rasmussen
Copy link
Author

bent-rasmussen commented Nov 14, 2024

"Good news everybody..." Our primary codebase is now fixed (apart from test cases), so the damage was quite limited. Also, only IEnumerable<T> was involved.

@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2024

"Good news everybody..." Our primary codebase is now fixed (apart from test cases), so the damage was quite limited. Also, only IEnumerable<T> was involved.

I am really glad this worked out.
For future readers, was your solution to move from type extensions to "C#-style extensions methods", or did you use another technique?

@bent-rasmussen
Copy link
Author

bent-rasmussen commented Nov 14, 2024

In our case I changed two properties from type extensions to C#-style extension methods.

@huoyaoyuan
Copy link
Member

The reason why F# does not have syntax for authoring this constraint is because F# has no mechanisms to enforce it.
That is also valid for type extensions - F# does not have checks in place to prevent you from e.g. allocating an array of 'T, which is however not allowed for byref types.

Can the type extension feature be somehow extended, to support extending a subset of instantiation of a type?
Consider the following extension method in C#:

public static void Foo<T>(this IEnumerable<T> source) where T : class { }

It's allowed to specify a narrower constraint for the type parameter. It's also often used effectively as an generic specialization mechanism.

@T-Gro
Copy link
Member

T-Gro commented Nov 15, 2024

The reason why F# does not have syntax for authoring this constraint is because F# has no mechanisms to enforce it.
That is also valid for type extensions - F# does not have checks in place to prevent you from e.g. allocating an array of 'T, which is however not allowed for byref types.

Can the type extension feature be somehow extended, to support extending a subset of instantiation of a type? Consider the following extension method in C#:

public static void Foo(this IEnumerable source) where T : class { }
It's allowed to specify a narrower constraint for the type parameter. It's also often used effectively as an generic specialization mechanism.

It is being considered.

The difference is that in C# you have to repeat the constraints, otherwise the method is considered to be an unconstrained generic.
We need to find a good balance between sensible defaults (= keep constraints as the original type), yet allow to deviate in specialized cases.

It's definitely worth a language suggestions, since the need to "add" / "remove" / "replace" constraints does need syntax design as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Area-Compiler-ImportAndInterop import of .NET DLLs and interop Bug Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. Regression
Projects
Status: New
Development

No branches or pull requests

5 participants