Open SteveGilham opened 6 years ago
@SteveGilham the branch must be there for a reason but if you can't exercise it as a dev then does it matter?
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.
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.
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.
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
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.
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.
My Framework
My Environment
I have already...
My issue is related to (check only those which apply):
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
which has an exhaustive match generates a branch coverage report like
The IL for the match selection looks like
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
typeThis 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.