Skip to content

Commit 56d124d

Browse files
committed
Adjustments from code review
Two adjustments for things that arose during code review: (1) Now using static local functions instead of non-static ones. (2) Removed InstructionHelpers class and reverted to straightforward ways of checking for instruction types.
1 parent 0f5be78 commit 56d124d

File tree

2 files changed

+29
-260
lines changed

2 files changed

+29
-260
lines changed

src/coverlet.core/Extensions/InstructionExtensions.cs

Lines changed: 0 additions & 247 deletions
This file was deleted.

src/coverlet.core/Symbols/CecilSymbolHelper.cs

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ private static bool SkipGeneratedBranchesForAwaitForeach(List<Instruction> instr
451451
// if GetResult() returned true.
452452

453453

454-
bool CheckForAsyncEnumerator(List<Instruction> instructions, Instruction instruction, int currentIndex)
454+
static bool CheckForAsyncEnumerator(List<Instruction> instructions, Instruction instruction, int currentIndex)
455455
{
456456
// We're looking for the following pattern, which checks whether a
457457
// compiler-generated field of type IAsyncEnumerator<> is null.
@@ -467,7 +467,8 @@ bool CheckForAsyncEnumerator(List<Instruction> instructions, Instruction instruc
467467
}
468468

469469
if (currentIndex >= 2 &&
470-
instructions[currentIndex - 2].IsLdarg(0) &&
470+
(instructions[currentIndex - 2].OpCode == OpCodes.Ldarg ||
471+
instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0) &&
471472
instructions[currentIndex - 1].OpCode == OpCodes.Ldfld &&
472473
instructions[currentIndex - 1].Operand is FieldDefinition field &&
473474
IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator"))
@@ -479,7 +480,7 @@ bool CheckForAsyncEnumerator(List<Instruction> instructions, Instruction instruc
479480
}
480481

481482

482-
bool CheckIfExceptionThrown(List<Instruction> instructions, Instruction instruction, int currentIndex)
483+
static bool CheckIfExceptionThrown(List<Instruction> instructions, Instruction instruction, int currentIndex)
483484
{
484485
// Here, we want to find a pattern where we're checking whether a
485486
// compiler-generated field of type Object is null. To narrow our
@@ -537,7 +538,7 @@ instructions[j].Operand is MethodReference callRef &&
537538
}
538539

539540

540-
bool CheckThrownExceptionType(List<Instruction> instructions, Instruction instruction, int currentIndex)
541+
static bool CheckThrownExceptionType(List<Instruction> instructions, Instruction instruction, int currentIndex)
541542
{
542543
// In this case, we're looking for a branch generated by the compiler to
543544
// check whether a previously-thrown exception has (at least) the type
@@ -567,7 +568,12 @@ bool CheckThrownExceptionType(List<Instruction> instructions, Instruction instru
567568
if (instructions[i].OpCode == OpCodes.Isinst &&
568569
instructions[i].Operand is TypeReference typeRef &&
569570
typeRef.FullName == "System.Exception" &&
570-
instructions[i - 1].IsLdloc())
571+
(instructions[i - 1].OpCode == OpCodes.Ldloc ||
572+
instructions[i - 1].OpCode == OpCodes.Ldloc_S ||
573+
instructions[i - 1].OpCode == OpCodes.Ldloc_0 ||
574+
instructions[i - 1].OpCode == OpCodes.Ldloc_1 ||
575+
instructions[i - 1].OpCode == OpCodes.Ldloc_2 ||
576+
instructions[i - 1].OpCode == OpCodes.Ldloc_3))
571577
{
572578
return true;
573579
}
@@ -585,7 +591,7 @@ private static bool SkipGeneratedBranchesForAwaitUsing(List<Instruction> instruc
585591
CheckForCleanup(instructions, instruction, currentIndex);
586592

587593

588-
bool CheckForSkipDisposal(List<Instruction> instructions, Instruction instruction, int currentIndex)
594+
static bool CheckForSkipDisposal(List<Instruction> instructions, Instruction instruction, int currentIndex)
589595
{
590596
// The async state machine generated for an "await using" contains a branch
591597
// that checks whether the call to DisposeAsync() needs to be skipped.
@@ -654,9 +660,18 @@ instructions[i].Operand is FieldDefinition reloadedField &&
654660
}
655661
}
656662
}
657-
else if (instructions[currentIndex - 1].IsLdloc(out int localVariableIndex) &&
658-
instructions[currentIndex + 1].IsLdloc(out int reloadedLocalVariableIndex) &&
659-
localVariableIndex == reloadedLocalVariableIndex &&
663+
else if ((instructions[currentIndex - 1].OpCode == OpCodes.Ldloc ||
664+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_S ||
665+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_0 ||
666+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_1 ||
667+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_2 ||
668+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_3) &&
669+
(instructions[currentIndex + 1].OpCode == OpCodes.Ldloc ||
670+
instructions[currentIndex + 1].OpCode == OpCodes.Ldloc_S ||
671+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_0 ||
672+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_1 ||
673+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_2 ||
674+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_3) &&
660675
instructions[currentIndex + 2].OpCode == OpCodes.Callvirt &&
661676
instructions[currentIndex + 2].Operand is MethodReference method &&
662677
method.DeclaringType.FullName == "System.IAsyncDisposable" &&
@@ -686,7 +701,7 @@ instructions[i].Operand is FieldDefinition reloadedField &&
686701
}
687702

688703

689-
bool CheckForCleanup(List<Instruction> instructions, Instruction instruction, int currentIndex)
704+
static bool CheckForCleanup(List<Instruction> instructions, Instruction instruction, int currentIndex)
690705
{
691706
// The pattern we're looking for here is this:
692707
//
@@ -721,7 +736,8 @@ bool CheckForCleanup(List<Instruction> instructions, Instruction instruction, in
721736
}
722737

723738
if (currentIndex >= 1 &&
724-
instructions[currentIndex - 1].IsLdc_I4(1))
739+
(instructions[currentIndex - 1].OpCode == OpCodes.Ldc_I4 ||
740+
instructions[currentIndex - 1].OpCode == OpCodes.Ldc_I4_1))
725741
{
726742
int minLoadFieldIndex = Math.Max(1, currentIndex - 5);
727743

@@ -770,7 +786,7 @@ private static bool SkipGeneratedBranchesForAsyncIterator(List<Instruction> inst
770786
DisposeCheck(instructions, instruction, currentIndex);
771787

772788

773-
bool CheckForStateSwitch(List<Instruction> instructions, Instruction instruction, int currentIndex)
789+
static bool CheckForStateSwitch(List<Instruction> instructions, Instruction instruction, int currentIndex)
774790
{
775791
// The pattern we're looking for here is this one:
776792
//
@@ -813,7 +829,7 @@ instructions[i].Operand is FieldDefinition field &&
813829
}
814830

815831

816-
bool DisposeCheck(List<Instruction> instructions, Instruction instruction, int currentIndex)
832+
static bool DisposeCheck(List<Instruction> instructions, Instruction instruction, int currentIndex)
817833
{
818834
// Within the compiler-generated async iterator, there are at least a
819835
// couple of places where we find this pattern, in which the async

0 commit comments

Comments
 (0)