Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Branches in F# inlines get added to the total #807

Open
7 of 26 tasks
SteveGilham opened this issue Mar 29, 2018 · 5 comments
Open
7 of 26 tasks

Branches in F# inlines get added to the total #807

SteveGilham opened this issue Mar 29, 2018 · 5 comments

Comments

@SteveGilham
Copy link
Contributor

SteveGilham commented Mar 29, 2018

Please provide the following information when submitting an issue.

Where appropriate replace the [ ] with a [X]

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
  • Something else

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.
  • the issue is another symptom of a deeper underlying issue with multiple previous manifestations

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

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

Expected Behavior

This code

[<EntryPoint>]
let main _ = 
    let sample = System.DateTime.UtcNow |> string
    printfn "%s" sample
    0 // return an integer exit code

should report no <BranchPoint ...> entries

Actual Behavior

A coverage report looks like

            <Method visited="true" cyclomaticComplexity="3" nPathComplexity="4" sequenceCoverage="100" branchCoverage="60" isConstructor="false" isStatic="true" isGetter="false" isSetter="false">
              <Summary numSequencePoints="3" visitedSequencePoints="3" numBranchPoints="5" visitedBranchPoints="3" sequenceCoverage="100" branchCoverage="60" maxCyclomaticComplexity="3" minCyclomaticComplexity="3" visitedClasses="0" numClasses="0" visitedMethods="1" numMethods="1" />
              <MetadataToken>100663297</MetadataToken>
              <Name>System.Int32 Program::main(System.String[])</Name>
              <FileRef uid="1" />
              <SequencePoints>
                <SequencePoint vc="1" uspid="1" ordinal="0" offset="2" sl="6" sc="5" el="6" ec="50" bec="4" bev="2" fileid="1" />
                <SequencePoint vc="1" uspid="2" ordinal="1" offset="80" sl="7" sc="5" el="7" ec="17" bec="0" bev="0" fileid="1" />
                <SequencePoint vc="1" uspid="3" ordinal="2" offset="110" sl="8" sc="5" el="8" ec="6" bec="0" bev="0" fileid="1" />
              </SequencePoints>
              <BranchPoints>
                <BranchPoint vc="1" uspid="4" ordinal="0" offset="20" sl="6" path="0" offsetend="22" fileid="1" />
                <BranchPoint vc="0" uspid="5" ordinal="1" offset="20" sl="6" path="1" offsetend="39" fileid="1" />
                <BranchPoint vc="1" uspid="6" ordinal="2" offset="33" sl="6" path="0" offsetchain="35" offsetend="47" fileid="1" />
                <BranchPoint vc="0" uspid="7" ordinal="3" offset="33" sl="6" path="1" offsetchain="37" offsetend="67" fileid="1" />
              </BranchPoints>
              <MethodPoint vc="1" uspid="8" ordinal="0" offset="0" />
            </Method>

because the string function is inlined, and the decompiled IL looks like

[EntryPoint]
public static int main(string[] _arg1)
{
	DateTime utcNow = DateTime.UtcNow; // Offset = 2
	DateTime dateTime = utcNow;
	object obj = dateTime;
	object obj2;
	if (obj != null) // Offset = 20
	{
		IFormattable formattable = obj as IFormattable;
		if (formattable != null) // Offset = 33
		{
			IFormattable formattable2 = formattable;
			obj2 = formattable2.ToString(null, CultureInfo.InvariantCulture);
		}
		else
		{
			object obj3 = obj;
			obj2 = obj3.ToString();
		}
	}
	else
	{
		obj2 = "";
	}
	string sample = (string)obj2;
	FSharpFunc<string, Unit> fSharpFunc = ExtraTopLevelOperators.PrintFormatLine(new PrintfFormat<FSharpFunc<string, Unit>, TextWriter, Unit, Unit, string>("%s")); // Offset = 80
	string func = sample;
	fSharpFunc.Invoke(func);
	return 0;
}

Looking at the XML we can see that we have a reported sequence point at offset 2, and then another at 80, and the branches span between offsets 20 and 67. The comment in CecilSymbolManager.cs method BuildPointsForBranch that

            // only add branch if branch does not match a known sequence 
            // e.g. auto generated field assignment
            // or encapsulates at least one sequence point

suggests that the intent is that such branching which spans no offset matching any reported sequence point should not be included.

My first guess is that the test on line 566 should also include the test from line 400, to wit the one that goes x.Item2.StartLine != StepOverLineCode, so as to exclude such not-in-source sequence points. I suspect that this would clear up most of the lingering compiler-surprise branch issues in one go.

Steps to reproduce the problem:

Build above code, and run

OpenCover.Console.exe -register:user -target:ConsoleApplication2.exe
@sawilde sawilde added the medium label Jan 1, 2019
@sawilde sawilde added this to To do in OpenCover - Future Jan 6, 2019
@sawilde sawilde moved this from To do to In progress in OpenCover - Future Jan 25, 2019
@sawilde
Copy link
Member

sawilde commented Jan 29, 2021

when I try to repeat this scenario the compiler does not create the extra branches and the C# produced looks like this

[EntryPoint]
public static int main(string[] _arg1)
{
	DateTime utcNow = DateTime.UtcNow;
	DateTime dateTime = utcNow;
	DateTime dateTime2 = dateTime;
	string sample = dateTime2.ToString(null, CultureInfo.InvariantCulture);
	FSharpFunc<string, Unit> fSharpFunc = ExtraTopLevelOperators.PrintFormatLine(new PrintfFormat<FSharpFunc<string, Unit>, TextWriter, Unit, Unit, string>("%s"));
	string func = sample;
	fSharpFunc.Invoke(func);
	return 0;
}

@SteveGilham
Copy link
Contributor Author

Presumably due to compiler changes over the last two and a bit years.

I'll make a few experiments to see what the code generation is like for other types than non-nullables like structs.

@sawilde
Copy link
Member

sawilde commented Jan 29, 2021

thanks - I tried net472 and netcoreapp3.1

@SteveGilham
Copy link
Contributor Author

Using Microsoft Visual Studio Community 2019 Version 16.8.4 (with Visual F# Tools 16.8.0-beta.20507.4+da6be68280c89131cdba2045525b80890401defd), create a new F# library project.

Change the code to be

namespace ForOpenCover

open System

module Issue807 =
    let example () =
      let sample = Console.Out |> string
      printfn "Hello %s" sample

and with a nullable type, we get back to the inlined branches

public unsafe static void example()
{
	TextWriter @out = Console.Out;
	TextWriter textWriter = @out;
	object obj = textWriter;
	IFormattable formattable = obj as IFormattable;
	object obj2;
	if (formattable == null)
	{
		if (obj == null)
		{
			obj2 = "";
		}
		else
		{
			TextWriter textWriter2 = textWriter;
			obj2 = ((TextWriter)(&textWriter2)).ToString();
		}
	}
	else
	{
		IFormattable formattable2 = formattable;
		obj2 = formattable2.ToString(null, CultureInfo.InvariantCulture);
	}
	string sample = (string)obj2;
	FSharpFunc<string, Unit> val = ExtraTopLevelOperators.PrintFormatLine<FSharpFunc<string, Unit>>((PrintfFormat<FSharpFunc<string, Unit>, TextWriter, Unit, Unit>)(object)new PrintfFormat<FSharpFunc<string, Unit>, TextWriter, Unit, Unit, string>("Hello %s"));
	string text = sample;
	val.Invoke(text);
}

sawilde added a commit to sawilde/opencover that referenced this issue Jan 29, 2021
@sawilde
Copy link
Member

sawilde commented Feb 2, 2021

@SteveGilham I did have a look at this but I can't find an obvious way of detecting/hiding these branches

var a = b ? "y" : "n";

creates a branch with no sequence points - these branches are very short though so it may need some criteria where if the branch has more than n IL instructions and no "visible" sequence points then we can hide it otherwise we keep it as it could be a branch as seen in the example.

@sawilde sawilde moved this from In progress to To do in OpenCover - Future May 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Development

No branches or pull requests

2 participants