Skip to content

Commit

Permalink
Implemented fix and refactoring (#1647)
Browse files Browse the repository at this point in the history
  • Loading branch information
daveMueller committed Apr 11, 2024
1 parent 67e37bb commit 723d341
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 23 deletions.
3 changes: 3 additions & 0 deletions Documentation/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Fixed
- Fix incorrect coverage await using in generic method [#1490](https://github.com/coverlet-coverage/coverlet/issues/1490)

## Release date 2024-03-13
### Packages
coverlet.msbuild 6.0.2
Expand Down
52 changes: 30 additions & 22 deletions src/coverlet.core/Symbols/CecilSymbolHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,22 @@ private static bool IsCompilerGenerated(FieldDefinition fieldDefinition)
return fieldDefinition.DeclaringType.CustomAttributes.Any(ca => ca.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName);
}

private static bool IsCompilerGeneratedField(Instruction instruction, out FieldDefinition field)
{
switch (instruction.Operand)
{
case FieldDefinition fieldDefinition when IsCompilerGenerated(fieldDefinition):
field = fieldDefinition;
return true;
case FieldReference fieldReference when IsCompilerGenerated(fieldReference.Resolve()):
field = fieldReference.Resolve();
return true;
default:
field = null;
return false;
}
}

private static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition)
{
if (methodDefinition.FullName.EndsWith("::MoveNext()") && IsCompilerGenerated(methodDefinition))
Expand Down Expand Up @@ -537,11 +553,9 @@ static bool CheckForAsyncEnumerator(List<Instruction> instructions, Instruction
(instructions[currentIndex - 2].OpCode == OpCodes.Ldarg ||
instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0) &&
instructions[currentIndex - 1].OpCode == OpCodes.Ldfld &&
(
(instructions[currentIndex - 1].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) ||
(instructions[currentIndex - 1].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator"))
)
)
IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field) &&
field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")
)
{
return true;
}
Expand Down Expand Up @@ -582,10 +596,8 @@ static bool CheckIfExceptionThrown(List<Instruction> instructions, Instruction i
for (int i = currentIndex - 1; i >= minFieldIndex; --i)
{
if (instructions[i].OpCode == OpCodes.Ldfld &&
(
(instructions[i].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FieldType.FullName == "System.Object") ||
(instructions[i].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FieldType.FullName == "System.Object")
))
IsCompilerGeneratedField(instructions[i], out FieldDefinition field) &&
field.FieldType.FullName == "System.Object")
{
// We expect the call to GetResult() to be no more than four
// instructions before the loading of the field's value.
Expand Down Expand Up @@ -708,16 +720,16 @@ static bool CheckForSkipDisposal(List<Instruction> instructions, Instruction ins

bool isFollowedByDisposeAsync = false;

if (instructions[currentIndex - 1].OpCode == OpCodes.Ldfld &&
instructions[currentIndex - 1].Operand is FieldDefinition field &&
IsCompilerGenerated(field))
if (instructions[currentIndex - 1].OpCode == OpCodes.Ldfld)
{
if(! IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field)) return false;

int maxReloadFieldIndex = Math.Min(currentIndex + 2, instructions.Count - 2);

for (int i = currentIndex + 1; i <= maxReloadFieldIndex; ++i)
{
if (instructions[i].OpCode == OpCodes.Ldfld &&
instructions[i].Operand is FieldDefinition reloadedField &&
IsCompilerGeneratedField(instructions[i], out FieldDefinition reloadedField) &&
field.Equals(reloadedField) &&
instructions[i + 1].OpCode == OpCodes.Callvirt &&
instructions[i + 1].Operand is MethodReference method &&
Expand Down Expand Up @@ -758,8 +770,8 @@ static bool CheckForSkipDisposal(List<Instruction> instructions, Instruction ins
if ((instructions[i].OpCode == OpCodes.Leave ||
instructions[i].OpCode == OpCodes.Leave_S) &&
instructions[i - 1].OpCode == OpCodes.Stfld &&
instructions[i - 1].Operand is FieldDefinition storeField &&
IsCompilerGenerated(storeField))
IsCompilerGeneratedField(instructions[i - 1], out FieldDefinition _)
)
{
return true;
}
Expand Down Expand Up @@ -811,9 +823,7 @@ static bool CheckForCleanup(List<Instruction> instructions, Instruction instruct

for (int i = currentIndex - 2; i >= minLoadFieldIndex; --i)
{
if (instructions[i].OpCode == OpCodes.Ldfld &&
instructions[i].Operand is FieldDefinition loadedField &&
IsCompilerGenerated(loadedField))
if (instructions[i].OpCode == OpCodes.Ldfld && IsCompilerGeneratedField(instructions[i], out FieldDefinition _))
{
int minRethrowIndex = Math.Max(0, i - 4);

Expand Down Expand Up @@ -918,10 +928,8 @@ static bool DisposeCheck(List<Instruction> instructions, Instruction instruction

if (currentIndex >= 2 &&
instructions[currentIndex - 1].OpCode == OpCodes.Ldfld &&
(
(instructions[currentIndex - 1].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FullName.EndsWith("__disposeMode")) ||
(instructions[currentIndex - 1].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FullName.EndsWith("__disposeMode"))
) &&
IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field) &&
field.FullName.EndsWith("__disposeMode") &&
(instructions[currentIndex - 2].OpCode == OpCodes.Ldarg ||
instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public void AwaitUsing()
{
await (ValueTask)instance.HasAwaitUsing();
await (Task)instance.Issue914_Repro();
await (Task)instance.Issue1490_Repro<string>();
}, persistPrepareResultToFile: pathSerialize[0]);
return 0;
Expand All @@ -39,8 +40,10 @@ public void AwaitUsing()
(28, 1), (29, 1), (30, 1),
// Issue914_Repro_Example2()
(34, 1), (35, 1), (36, 1), (37, 1),
// Issue1490_Repro<T>()
(40, 1), (41, 1), (42, 1), (43, 1),
// MyTransaction.DisposeAsync()
(43, 2), (44, 2), (45, 2)
(48, 3), (49, 3), (50, 3)
)
.ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ async private Task Issue914_Repro_Example2()
await transaction.DisposeAsync();
}

async public Task<T> Issue1490_Repro<T>()
{
await using var transaction = new MyTransaction();
return default(T);
}

private class MyTransaction : IAsyncDisposable
{
Expand Down

0 comments on commit 723d341

Please sign in to comment.