F# match expressions on an algebraic type have an unreachable branch

[SteveGilham]

My Framework

• .NET 2
• .NET 3.5
• .NET 4
• .NET 4.5
• .NET 4.6
• .NET 4.6.1
• .NET 4.6.2
• .NET Core 1.0.0
• .net 4.7

My Environment

• Windows 7 or below (not truly supported due to EOL)
• Windows 8
• Windows 8.1
• Windows 10
• Windows 10 IoT Core
• Windows Server 2012
• Windows Server 2012 R2
• Windows Server 2016

I have already...

• repeated the problem using the latest stable release of OpenCover.
• reviewed the usage guide and usage document.
• have looked at the opencover output xml file in an attempt to resolve the issue.
• reviewed the current issues to check that the issue isn't already known.

My issue is related to (check only those which apply):

• no coverage being recorded
• 32 or 64 bit support
• feature request

Expected Behavior

Eventually branch coverage will recognise and thus not report any of the spurious uncovered/uncoverable branches inserted surreptitiously by the various compilers.

Actual Behavior

F# match expressions on an algebraic type of N kinds show N+1 branches, one of which cannot possibly be taken and thus stubbornly remains uncovered.

Steps to reproduce the problem:

Code like

type internal FilterClass =
  | File of string
  | Assembly of string
  | Type of string
  | Method of string
  | Attribute of string

module Filter =
  let internal Match (nameProvider:Object) (filter:FilterClass) =
    match filter with
    | File name -> ...
    | Assembly name -> ...
    | Type name -> ...
    | Method name -> ...
    | Attribute name -> ...
    ...

which has an exhaustive match generates a branch coverage report like

                <BranchPoint vc="0" uspid="934" ordinal="0" offset="9" sl="21" path="0" offsetchain="34" offsetend="53" fileid="1" />
                <BranchPoint vc="20" uspid="935" ordinal="1" offset="9" sl="21" path="1" offsetchain="34" offsetend="53" fileid="1" />
                <BranchPoint vc="2" uspid="936" ordinal="2" offset="9" sl="21" path="2" offsetchain="36" offsetend="105" fileid="1" />
                <BranchPoint vc="74" uspid="937" ordinal="3" offset="9" sl="21" path="3" offsetchain="38" offsetend="168" fileid="1" />
                <BranchPoint vc="68" uspid="938" ordinal="4" offset="9" sl="21" path="4" offsetchain="43" offsetend="226" fileid="1" />
                <BranchPoint vc="2" uspid="939" ordinal="5" offset="9" sl="21" path="5" offsetchain="48" offsetend="284" fileid="1" />

The IL for the match selection looks like

	IL_0004: call instance int32 FilterClass::get_Tag()
	IL_0009: switch (IL_0022, IL_0024, IL_0026, IL_002b, IL_0030)
	IL_0022: br.s IL_0035
	IL_0024: br.s IL_0069
	IL_0026: br IL_00a8
	IL_002b: br IL_00e2
	IL_0030: br IL_011c

and the uncoverable branch looks to be the default/"none of the above" drop through that just happens to have the same flow as for the case with the numerically first of the tag values. The marker here would be that the switch is on the result of a get_Tag() on a SourceConstructFlags.SumType type

This is not the only form of match for which I've seen this sort of issue, but this is the one that has been simplest to characterise. Those others may eventually form separate reports.

[sawilde]

@SteveGilham the branch must be there for a reason but if you can't exercise it as a dev then does it matter?

[SteveGilham]

The IL produced is what is both easiest to produce in the compile phase, and most efficient to execute, treating as it does, all proper values of the type in the same fashion, rather than picking favourites in an extra "if tag-zero subtype drop through else switch" step. The drop-through case is just unimportant (it doesn't even have any code associated with it, let alone a trap).

Algebraic types are class types, so in principle a null could be smuggled in via a language that doesn't enforce non-null semantics as a nearly-proper value -- but in that case the get_Tag call would throw. Otherwise it would take reflective monkeying with the internal readonly tag field to make an object drop through to the first type clause -- and there, in the general case where payloads differ, runtime exceptions would result from accessing fields that aren't of the right type.

[sawilde]

Yeah, but how are we supposed to exclude it? This is the problem with most of the "extra" branch issues that the compiler puts in but can't be exercised normally.

[SteveGilham]

In this case, the switch instruction is immediately preceded by a get_Tag() call on a type that is a SourceConstructFlags.SumType type, where the range on the switch is from 0 to the maximum value of the type's nested internal Tags type. This gives -- debug or release -- IL looking like

	IL_0004: call instance int32 FilterClass::get_Tag()
	IL_0009: switch (IL_0022, IL_0024, IL_0026, IL_002b, IL_0030)

where in this case FilterClass.Tags is equivalent to

internal static class Tags
{
	public const int File = 0;
	public const int Assembly = 1;
	public const int Type = 2;
	public const int Method = 3;
	public const int Attribute = 4;
}

This is not the only case I've seen where the compiler leaves an uncoverable branch, but it is one that is fairly easily characterised.

[sawilde]

Hmmm.... in order to identify a get_Tag() we'd have to do a lot more processing of the IL than we currently do and that will have a big impact on the performance

[SteveGilham]

If the cost/benefit isn't good, then by all means close "won't fix"; at least the issue will be on the record for the next time anyone stumbles across it.

[SteveGilham]

Having had another look at this issue, one of the signs of such an exhaustive switch is that one of the target instructions (it happens to be the first one in this case, but it could be any one of them, depending how the case clauses are ordered compared with how the cases are ordered in the type definition) is also the very next instruction

	IL_0009: switch (IL_0022, IL_0024, IL_0026, IL_002b, IL_0030)
	IL_0022: br.s IL_0035

This then prompts us to ask whether there is any meaningful distinction between the two different ways of going from offset 0x9 to offset 0x22, as they both cause exactly the same code to execute.