dotnet / dotnet/fsharp

Improve diagnostics for inlining in class methods - FS1114, FS1113, FS1116, FS1118

Open
#1,565 28 comments 3 reactions 1 assignee Claimed by @abelbraaksma View on GitHub
Area-Diagnostics Feature Improvement
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 11h
Merged PRs (30d)
131

Description

The general rule in F# let-bindings is that they must appear in order if they are dependent on one another, but inside classes, and among classes chained with `and` , the order is irrelevant.

Most of the time this is true, but if you use inlined members and these are forward-declared in your class or class hierarchy, they may lead to spurious compile-time errors. These errors (see below) are generally only raised when using inlining on recursive functions, leading to very hard-to-diagnose uncompilable code. Besides, these errors are not only misplaced, they seem to be plain wrong, there should probably not be an error at all.
## Repro steps

Over the cause of a few weeks analyzing this, leading to a [first](http://stackoverflow.com/questions/39440568/curious-errors-when-combining-inline-with-explicit-member-constraints) and a [second](http://stackoverflow.com/questions/39707477/using-inline-within-type-members-fails-with-fs1114-fs1113-fs1116-fs1118?noredirect=1#) yet to be answered StackOverflow question, it suddenly dawned to me that rearranging the members solved the problem. But first things first, here's how to repro.
### First example (copy/paste and compile to see the error)

```
/// Does a bounds check and raises an error if bounds check is not met
let inline checkBounds f (g: 'b -> ^c) (tp: ^a) =
let convertFrom = (^a: (static member name: string) ())
let convertTo = (^c: (static member name : string) ())
let value = (^a: (member Value: 'b) tp)
if f value then
g value
else
failwithf "Cannot convert from %s to %s." convertFrom convertTo

type ConverterA =
struct
val Value: sbyte
new v = { Value = v }
end

static member inline name with get() = "converter-a"
static member inline convert (x: ConverterA) : ConverterB =
checkBounds ((>=) 0y) (byte >> ConverterB) x

and ConverterB =
struct
val Value: byte
new v = { Value = v }
end
static member inline name with get() = "converter-b"
```
### Second example:

```
[]
type MyType =
| Integer = 0b0001
| Float = 0b0010

module Test =
[]
type SomeType =
| Int of int64
| Float of float

override x.Equals other =
match other with
| :? SomeType as y ->
// following line throws on compiling this
match SomeType.getType x &&& SomeType.getType y with
| MyType.Integer -> int64 x = int64 y // highest type is integer (both are int)
| MyType.Float -> float x = float y // highest type is float (either is or both are float)
| _ -> false // impossible
| _ -> false

override x.GetHashCode() =
match x with Int i -> hash i | Float f -> hash f

static member inline op_Explicit(n: SomeType): float =
match n with
| Int i -> float i
| Float f -> f

static member inline op_Explicit(n: SomeType): int64 =
match n with
| Int i -> i
| Float f -> int64 f

static member inline getType x =
match x with
| Int _ -> MyType.Integer
| Float _ -> MyType.Float
```
### Third (simplest I could muster) example:

```
module Test =
type SomeType =
| Int of int64
| Float of float

static member MyEquals (x, other: SomeType) =
// following line throws on compiling this
float x = float other

static member inline op_Explicit(n: SomeType): float =
match n with
| Int i -> float i
| Float f -> f

static member inline op_Explicit(n: SomeType): int64 =
match n with
| Int i -> i
| Float f -> int64 f
```

(these examples are also used in the SO questions)
## Expected behavior

While typing these examples, the syntax checker and inference algorithms won't show any errors. When you compile them, the expected behavior is that they compile just fine (apart from perhaps debatable practices of the code examples, but I had to simplify, they originate from a large codebase).
## Actual behavior

Instead of compiling just fine, the following errors and warnings are raised by the compiler (and the variants thereof for each example above, but the error numbers and order wherein they appear are the same).

> error FS1114: The value 'Foo.Bar.name' was marked
> inline but was not bound in the optimization environment
>
> error FS1113:The value 'name' was marked inline but its
> implementation makes use of an internal or private function which is not
> sufficiently accessible
>
> warning FS1116: A value marked as 'inline' has an unexpected value
>
> error FS1118: Failed to inline the value 'name'
> marked 'inline', perhaps because a recursive value was marked 'inline'

Note that the errors seem to point out that it fails to inline the following method, which itself has no dependencies and seems trivial to inline:

```
static member inline name with get() = "converter-b"
```

Note also that FS1113 is not applicable at all, there are no private or internal members or functions.

Note that FS1116 is incomprehensible (how can an inline function have an unexpected value? What is it? And what is the value?)

Note that FS1118 suggests it is a recursive function marked inline. This is the only error that hints that the compiler has not enough knowledge to inline the method, but hints in the wrong direction (there is nothing recursive).

This gets worse when you have a compilable codebase and you decide, for instance, to implement `override x.Equals()`, or `IComparable`. It is common practice to place these on top. But if they call into inline functions you suddenly get those errors. And I've found that they can appear in related classes, not even in the just-added methods, which furthers the troubles of diagnosing this.
## Known workarounds

Changing the order in which the class members, or classes themselves appear solves the problem. Though this is, I believe, against the policy that inside a class the order ought to be insignificant. An example of a compilable version of repro 2 above:

```
/// Does a bounds check and raises an error if bounds check is not met
let inline checkBounds f (g: 'b -> ^c) (tp: ^a) =
let convertFrom = (^a: (static member name: string) ())
let convertTo = (^c: (static member name : string) ())
let value = (^a: (member Value: 'b) tp)
if f value then
g value
else
failwithf "Cannot convert from %s to %s." convertFrom convertTo

// place ConverterB before ConverterA solves the error
type ConverterB =
struct
val Value: byte
new v = { Value = v }
end
static member inline name with get() = "converter-b"

and ConverterA =
struct
val Value: sbyte
new v = { Value = v }
end

static member inline name with get() = "converter-a"
static member inline convert (x: ConverterA) : ConverterB =
checkBounds ((>=) 0y) (byte >> ConverterB) x
```
## Related information

The examples above fail to compile with VS2015, Update 1, 2 and 3, using F# 4.0 (FSharp.Core 4.4.0.0).

I consider this issue relatively severe because the errors raised have so little to do with the actual situation and can appear "out of nowhere" after you add a simple new method. Even if an error is legit in this case, something like "Method X is unknown, but exists in your declarations, consider changing the order in which they appear in the source so that the optimization environment can bind to them."

BTW, there are many situations where the order of inlined functions _does not matter_, leading to situations where you have compilable code that just, after adding an otherwise independent method, stops being compilable.

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.