dotnet / dotnet/fsharp

Make typedefof more efficient

Open
#5,019 25 comments 5 reactions 0 assignees View on GitHub
Area-Compiler-Optimization Feature Improvement
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 11h
Merged PRs (30d)
131

Description

I don't think it's actually a _suggestion_, so I'm filing it as a compiler issue.

As @rspeele mentions in [comment to TaskBuilder.fs repo](https://github.com/rspeele/TaskBuilder.fs/issues/21#issuecomment-392299384), currently `typedefof` looks rather inefficient. Compare the [following C#](https://sharplab.io/#v2:EYLgZgpghgLgrgJwgZwLRWQTwHYGNXIxJQC2yANDCAJYA25AJiANQA+AAgAwAE7AjAG4AsACguvPgBZhYgMy8ATNwDC3AN6juW3vPaTuAWQAUASnWbtlgG5QE3AB7cAvNxiYADhAD2YIwBlqQgAeAD4TGUsAX1FIoA==) and the [following F#](https://sharplab.io/#v2:DYLgZgzgNAJiDUAfA9gBwKYDsAEBlAnhAC7oC2AsAFBpZ6EmkB0AwssMOgMZECWymERgHEs6AE49OVIvgzZmACgCU2ALxVsm7KTIAjcdiIALHoICyytRq02ORbAA81h2ehjowyMAB4AMqaJvAH0APhDrG01lIA==) snippets:

```csharp
using System.Collections.Generic;
public class C {
public void M() {
var x = typeof(List<>);
}
}
```

```fsharp
open System.Collections.Generic
type C() =
member this.M() =
let x = typedefof>
()
```

C# compiles to the following CIL:

```
ldtoken [mscorlib]System.Collections.Generic.List`1
call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
stloc.0
```

While F# compiles to the following:

```
ldtoken class [mscorlib]System.Collections.Generic.List`1
call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
stloc.1
ldloc.1
callvirt instance bool [mscorlib]System.Type::get_IsGenericType()
brfalse.s IL_001b

ldloc.1
callvirt instance class [mscorlib]System.Type [mscorlib]System.Type::GetGenericTypeDefinition()
br.s IL_001c

IL_001b: ldloc.1
IL_001c: stloc.0
```

which is an equivalent of

```csharp
Type typeFromHandle = typeof(List);
Type type = (!typeFromHandle.IsGenericType) ? typeFromHandle : typeFromHandle.GetGenericTypeDefinition()
```

Could we improve that? Are there any issues I miss with plain `ldtoken` that **require** the current approach?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.