-
Notifications
You must be signed in to change notification settings - Fork 388
Change CecilSymbolHelper to instance class #846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 22 commits
a6e0687
84c0aff
5b4e316
45075e1
ef7d0b0
a50f143
28e038c
a8198b6
fce0e2d
8b8505b
566262b
eb5969d
a3f4a2f
c6fb0fc
903ccdf
3a4a37b
438132c
54b7e29
01946ee
2fefa04
4ff29e3
970280a
befd87e
777351a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
using System.Collections.Generic; | ||
using Coverlet.Core.Symbols; | ||
using Mono.Cecil; | ||
using Mono.Cecil.Cil; | ||
|
||
namespace Coverlet.Core.Abstractions | ||
{ | ||
internal interface ICecilSymbolHelper | ||
{ | ||
IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition); | ||
bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Runtime.CompilerServices; | ||
|
||
using Coverlet.Core.Abstractions; | ||
using Coverlet.Core.Extensions; | ||
|
||
using Mono.Cecil; | ||
|
@@ -16,15 +16,15 @@ | |
|
||
namespace Coverlet.Core.Symbols | ||
{ | ||
internal static class CecilSymbolHelper | ||
internal class CecilSymbolHelper : ICecilSymbolHelper | ||
{ | ||
private const int StepOverLineCode = 0xFEEFEE; | ||
private static ConcurrentDictionary<string, int[]> CompilerGeneratedBranchesToExclude = null; | ||
private readonly ConcurrentDictionary<string, int[]> _compilerGeneratedBranchesToExclude; | ||
|
||
static CecilSymbolHelper() | ||
public CecilSymbolHelper() | ||
{ | ||
// Create single instance, we cannot collide because we use full method name as key | ||
CompilerGeneratedBranchesToExclude = new ConcurrentDictionary<string, int[]>(); | ||
_compilerGeneratedBranchesToExclude = new ConcurrentDictionary<string, int[]>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can initialize inline and remove ctor, before we wanted to be sure single thread initialization, now we use an singleton instance so no double init issue. Keep the comment above pls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} | ||
|
||
// In case of nested compiler generated classes, only the root one presents the CompilerGenerated attribute. | ||
|
@@ -265,9 +265,9 @@ private static bool SkipGeneratedBranchForExceptionRethrown(List<Instruction> in | |
instructions[branchIndex + 3].OpCode == OpCodes.Throw; | ||
} | ||
|
||
private static bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions) | ||
private bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions) | ||
{ | ||
if (!CompilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName)) | ||
if (!_compilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName)) | ||
{ | ||
/* | ||
This method is used to parse compiler generated code inside async state machine and find branches generated for exception catch blocks | ||
|
@@ -393,13 +393,13 @@ private static bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition m | |
} | ||
} | ||
|
||
CompilerGeneratedBranchesToExclude.TryAdd(methodDefinition.FullName, detectedBranches.ToArray()); | ||
_compilerGeneratedBranchesToExclude.TryAdd(methodDefinition.FullName, detectedBranches.ToArray()); | ||
} | ||
|
||
return CompilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset); | ||
return _compilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset); | ||
} | ||
|
||
public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition) | ||
public IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition) | ||
{ | ||
var list = new List<BranchPoint>(); | ||
if (methodDefinition is null) | ||
|
@@ -664,7 +664,7 @@ await ... | |
IL_00eb: br.s IL_00ed | ||
... | ||
*/ | ||
internal static bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction) | ||
public bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction) | ||
{ | ||
if (!IsMoveNextInsideAsyncStateMachine(methodDefinition)) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use
serviceProvider.GetRequiredService
everywhere where we don't use variable for more than one place?I see some assignation above,sourceTranslator
is used in more than one place?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done