[CFE] Fix coverage destroyed by (wrong) offsets on extension method tearoffs
Say you have a class method like this:
```
class Bar {
/*offset a*/ void /*offset b*/ qux() {
/*offset c*/ print("hello");
}
}
```
Lets also say that this method is run.
This will mark offset a and offset c as hits.
Offset b does not exist in coverage terms.
Now say you have an extension mehod like this:
```
extension Foo on Bar {
/*offset a*/ void /*offset b*/ baz() {
/*offset c*/ print("hello");
}
}
```
Lets also say that this method is also executed.
This will mark offset a and offset c as hits.
Offset b does not exist in coverage terms for this method.
Because extension methods are special though we create a special tearoff for
it. Lets say we didn't execute that one.
The tearoff method - before this CL - had offset b on positions that caused
the position to exist in coverage terms, and as the method wasn't executed
this would make it a miss.
This CL fixes the issue by setting the offsets on the tearoff that before
introduced offset b to offset a instead.
Change-Id: I3a5339135f3d76327624b35f04cc14afccaf487a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404563
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/front_end/lib/src/fragment/method.dart b/pkg/front_end/lib/src/fragment/method.dart
index a6c822d..dbe8e64 100644
--- a/pkg/front_end/lib/src/fragment/method.dart
+++ b/pkg/front_end/lib/src/fragment/method.dart
@@ -750,6 +750,7 @@
Procedure procedure, NameScheme nameScheme, Reference? tearOffReference) {
_extensionTearOffParameterMap = {};
+ int fileStartOffset = _fragment.startOffset;
int fileOffset = _fragment.nameOffset;
int fileEndOffset = _fragment.endOffset;
@@ -837,7 +838,10 @@
procedure,
new Arguments(closurePositionalArguments,
types: typeArguments, named: closureNamedArguments))
- ..fileOffset = fileOffset)
+ // We need to use the fileStartOffset on the StaticInvocation to
+ // avoid a possible "fake coverage miss" on the name of the
+ // extension method.
+ ..fileOffset = fileStartOffset)
..fileOffset = fileOffset;
FunctionExpression closure = new FunctionExpression(
@@ -850,7 +854,10 @@
returnType: closureReturnType)
..fileOffset = fileOffset
..fileEndOffset = fileEndOffset)
- ..fileOffset = fileOffset;
+ // We need to use the fileStartOffset on the FunctionExpression to
+ // avoid a possible "fake coverage miss" on the name of the
+ // extension method.
+ ..fileOffset = fileStartOffset;
FunctionNode function = new FunctionNode(
new ReturnStatement(closure)..fileOffset = fileOffset,
diff --git a/pkg/front_end/test/coverage_suite_expected.dart b/pkg/front_end/test/coverage_suite_expected.dart
index 7ea66756..0dca096 100644
--- a/pkg/front_end/test/coverage_suite_expected.dart
+++ b/pkg/front_end/test/coverage_suite_expected.dart
@@ -148,10 +148,10 @@
hitCount: 97,
missCount: 0,
),
- // 99.88331388564761%.
+ // 100.0%.
"package:front_end/src/base/incremental_compiler.dart": (
hitCount: 856,
- missCount: 1,
+ missCount: 0,
),
// 100.0%.
"package:front_end/src/base/incremental_serializer.dart": (
@@ -183,10 +183,10 @@
hitCount: 31,
missCount: 0,
),
- // 99.26470588235294%.
+ // 100.0%.
"package:front_end/src/base/modifiers.dart": (
hitCount: 135,
- missCount: 1,
+ missCount: 0,
),
// 100.0%.
"package:front_end/src/base/name_space.dart": (
@@ -208,10 +208,10 @@
hitCount: 260,
missCount: 0,
),
- // 99.38837920489296%.
+ // 100.0%.
"package:front_end/src/base/scope.dart": (
hitCount: 650,
- missCount: 4,
+ missCount: 0,
),
// 100.0%.
"package:front_end/src/base/ticker.dart": (
@@ -535,7 +535,7 @@
),
// 100.0%.
"package:front_end/src/fragment/method.dart": (
- hitCount: 771,
+ hitCount: 773,
missCount: 0,
),
// 100.0%.
@@ -920,7 +920,7 @@
),
// 100.0%.
"package:front_end/src/source/source_constructor_builder.dart": (
- hitCount: 1085,
+ hitCount: 1084,
missCount: 0,
),
// 100.0%.
@@ -941,13 +941,13 @@
),
// 100.0%.
"package:front_end/src/source/source_factory_builder.dart": (
- hitCount: 717,
+ hitCount: 715,
missCount: 0,
),
- // 97.91666666666666%.
+ // 100.0%.
"package:front_end/src/source/source_function_builder.dart": (
hitCount: 47,
- missCount: 1,
+ missCount: 0,
),
// 100.0%.
"package:front_end/src/source/source_library_builder.dart": (
@@ -991,7 +991,7 @@
),
// 100.0%.
"package:front_end/src/source/type_parameter_scope_builder.dart": (
- hitCount: 1445,
+ hitCount: 1446,
missCount: 0,
),
// 100.0%.
@@ -1109,10 +1109,10 @@
hitCount: 20,
missCount: 0,
),
- // 92.0%.
+ // 100.0%.
"package:front_end/src/util/local_stack.dart": (
hitCount: 23,
- missCount: 2,
+ missCount: 0,
),
// 100.0%.
"package:front_end/src/util/parser_ast.dart": (
diff --git a/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.1.expect b/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.1.expect
index 6cdd1fb..9453eca 100644
--- a/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.1.expect
+++ b/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.1.expect
@@ -121,18 +121,18 @@
import "org-dartlang-test:///foo.dart";
- static field foo::Foo newFoo = foo::Foo::•($creationLocationd_0dea112b090073317d4: #C37);
- static field () → foo::Foo newFooFunction = #C38;
+ static field foo::Foo newFoo = foo::Foo::•($creationLocationd_0dea112b090073317d4: #C38);
+ static field () → foo::Foo newFooFunction = #C39;
static field foo::Foo newFooFunctionCall = main::newFooFunction(){() → foo::Foo};
- static field foo::Foo extensionFoo = foo::FooExtension|foo(null, $creationLocationd_0dea112b090073317d4: #C41);
+ static field foo::Foo extensionFoo = foo::FooExtension|foo(null, $creationLocationd_0dea112b090073317d4: #C42);
static field () → foo::Foo extensionFooFunction = foo::FooExtension|get#foo(null);
static field foo::Foo extensionFooFunctionCall = main::extensionFooFunction(){() → foo::Foo};
- static field foo::Foo extensionBar = foo::FooExtension|bar(null, $creationLocationd_0dea112b090073317d4: #C42);
+ static field foo::Foo extensionBar = foo::FooExtension|bar(null, $creationLocationd_0dea112b090073317d4: #C43);
static field foo::Foo extensionBaz = foo::FooExtension|baz(null);
- static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C43);
- static field foo::Foo extensionConstFoo = foo::FooExtension|constFoo(null, $creationLocationd_0dea112b090073317d4: #C45);
- static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C49);
- static field () → Null extensionSetterFoo = () → Null => let final has-declared-initializer Null #t1 = null in let final void #t2 = foo::FooExtension|set#setterFoo(null, #t1, $creationLocationd_0dea112b090073317d4: #C53) in #t1;
+ static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C44);
+ static field foo::Foo extensionConstFoo = foo::FooExtension|constFoo(null, $creationLocationd_0dea112b090073317d4: #C46);
+ static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C50);
+ static field () → Null extensionSetterFoo = () → Null => let final has-declared-initializer Null #t1 = null in let final void #t2 = foo::FooExtension|set#setterFoo(null, #t1, $creationLocationd_0dea112b090073317d4: #C54) in #t1;
static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C57);
static field foo::Foo extensionStaticFoo = foo::FooExtension|staticFoo();
}
@@ -150,50 +150,50 @@
#C11 = 10.0
#C12 = 9.0
#C13 = wid::_Location {file:#C3, line:#C11, column:#C12, name:#C6, parameterLocations:#C1}
- #C14 = 15.0
- #C15 = 7.0
+ #C14 = 14.0
+ #C15 = 3.0
#C16 = "FooExtension|foo"
#C17 = wid::_Location {file:#C3, line:#C14, column:#C15, name:#C16, parameterLocations:#C1}
- #C18 = 18.0
+ #C18 = 17.0
#C19 = "FooExtension|bar"
#C20 = wid::_Location {file:#C3, line:#C18, column:#C15, name:#C19, parameterLocations:#C1}
#C21 = 21.0
#C22 = 20.0
#C23 = wid::_Location {file:#C3, line:#C21, column:#C22, name:#C6, parameterLocations:#C1}
- #C24 = 24.0
+ #C24 = 23.0
#C25 = "FooExtension|boz"
#C26 = wid::_Location {file:#C3, line:#C24, column:#C15, name:#C25, parameterLocations:#C1}
#C27 = 27.0
#C28 = wid::_Location {file:#C3, line:#C27, column:#C27, name:#C6, parameterLocations:#C1}
#C29 = foo::Foo {_location:#C28}
#C30 = "FooExtension|constFoo"
- #C31 = wid::_Location {file:#C3, line:#C27, column:#C15, name:#C30, parameterLocations:#C1}
+ #C31 = wid::_Location {file:#C3, line:#C5, column:#C15, name:#C30, parameterLocations:#C1}
#C32 = 39.0
#C33 = 33.0
#C34 = wid::_Location {file:#C3, line:#C32, column:#C33, name:#C6, parameterLocations:#C1}
#C35 = "org-dartlang-test:///main.dart"
#C36 = 2.0
- #C37 = wid::_Location {file:#C35, line:#C36, column:#C18, name:#C6, parameterLocations:#C1}
- #C38 = static-tearoff foo::Foo::_#new#tearOff
- #C39 = 5.0
- #C40 = 25.0
- #C41 = wid::_Location {file:#C35, line:#C39, column:#C40, name:#C16, parameterLocations:#C1}
- #C42 = wid::_Location {file:#C35, line:#C4, column:#C40, name:#C19, parameterLocations:#C1}
- #C43 = wid::_Location {file:#C35, line:#C11, column:#C40, name:#C25, parameterLocations:#C1}
- #C44 = 30.0
- #C45 = wid::_Location {file:#C35, line:#C9, column:#C44, name:#C30, parameterLocations:#C1}
- #C46 = 12.0
- #C47 = 31.0
- #C48 = "FooExtension|get#getterFoo"
- #C49 = wid::_Location {file:#C35, line:#C46, column:#C47, name:#C48, parameterLocations:#C1}
- #C50 = 13.0
- #C51 = 37.0
- #C52 = "FooExtension|set#setterFoo"
- #C53 = wid::_Location {file:#C35, line:#C50, column:#C51, name:#C52, parameterLocations:#C1}
- #C54 = 14.0
+ #C37 = 18.0
+ #C38 = wid::_Location {file:#C35, line:#C36, column:#C37, name:#C6, parameterLocations:#C1}
+ #C39 = static-tearoff foo::Foo::_#new#tearOff
+ #C40 = 5.0
+ #C41 = 25.0
+ #C42 = wid::_Location {file:#C35, line:#C40, column:#C41, name:#C16, parameterLocations:#C1}
+ #C43 = wid::_Location {file:#C35, line:#C4, column:#C41, name:#C19, parameterLocations:#C1}
+ #C44 = wid::_Location {file:#C35, line:#C11, column:#C41, name:#C25, parameterLocations:#C1}
+ #C45 = 30.0
+ #C46 = wid::_Location {file:#C35, line:#C9, column:#C45, name:#C30, parameterLocations:#C1}
+ #C47 = 12.0
+ #C48 = 31.0
+ #C49 = "FooExtension|get#getterFoo"
+ #C50 = wid::_Location {file:#C35, line:#C47, column:#C48, name:#C49, parameterLocations:#C1}
+ #C51 = 13.0
+ #C52 = 37.0
+ #C53 = "FooExtension|set#setterFoo"
+ #C54 = wid::_Location {file:#C35, line:#C51, column:#C52, name:#C53, parameterLocations:#C1}
#C55 = 28.0
#C56 = "FooExtension|unary-"
- #C57 = wid::_Location {file:#C35, line:#C54, column:#C55, name:#C56, parameterLocations:#C1}
+ #C57 = wid::_Location {file:#C35, line:#C14, column:#C55, name:#C56, parameterLocations:#C1}
}
diff --git a/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.2.expect b/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.2.expect
index 054f50d..57ed023 100644
--- a/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.2.expect
+++ b/pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.2.expect
@@ -133,7 +133,7 @@
static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C46);
static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C50);
static field () → Null extensionSetterFoo = () → Null => let final has-declared-initializer Null #t1 = null in let final void #t2 = foo::FooExtension|set#setterFoo(null, #t1, $creationLocationd_0dea112b090073317d4: #C54) in #t1;
- static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C58);
+ static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C57);
static field foo::Foo extensionStaticFoo = foo::FooExtension|staticFoo();
}
constants {
@@ -150,24 +150,24 @@
#C11 = 10.0
#C12 = 9.0
#C13 = wid::_Location {file:#C3, line:#C11, column:#C12, name:#C6, parameterLocations:#C1}
- #C14 = 15.0
- #C15 = 7.0
+ #C14 = 14.0
+ #C15 = 3.0
#C16 = "FooExtension|foo"
#C17 = wid::_Location {file:#C3, line:#C14, column:#C15, name:#C16, parameterLocations:#C1}
- #C18 = 18.0
+ #C18 = 17.0
#C19 = "FooExtension|bar"
#C20 = wid::_Location {file:#C3, line:#C18, column:#C15, name:#C19, parameterLocations:#C1}
#C21 = 21.0
#C22 = 20.0
#C23 = wid::_Location {file:#C3, line:#C21, column:#C22, name:#C6, parameterLocations:#C1}
- #C24 = 24.0
+ #C24 = 23.0
#C25 = "FooExtension|boz"
#C26 = wid::_Location {file:#C3, line:#C24, column:#C15, name:#C25, parameterLocations:#C1}
#C27 = 27.0
#C28 = wid::_Location {file:#C3, line:#C27, column:#C27, name:#C6, parameterLocations:#C1}
#C29 = foo::Foo {_location:#C28}
#C30 = "FooExtension|constFoo"
- #C31 = wid::_Location {file:#C3, line:#C27, column:#C15, name:#C30, parameterLocations:#C1}
+ #C31 = wid::_Location {file:#C3, line:#C5, column:#C15, name:#C30, parameterLocations:#C1}
#C32 = 39.0
#C33 = 33.0
#C34 = wid::_Location {file:#C3, line:#C32, column:#C33, name:#C6, parameterLocations:#C1}
@@ -175,8 +175,8 @@
#C36 = 2.0
#C37 = 30.0
#C38 = wid::_Location {file:#C35, line:#C36, column:#C37, name:#C30, parameterLocations:#C1}
- #C39 = 3.0
- #C40 = wid::_Location {file:#C35, line:#C39, column:#C18, name:#C6, parameterLocations:#C1}
+ #C39 = 18.0
+ #C40 = wid::_Location {file:#C35, line:#C15, column:#C39, name:#C6, parameterLocations:#C1}
#C41 = static-tearoff foo::Foo::_#new#tearOff
#C42 = 6.0
#C43 = 25.0
@@ -191,10 +191,9 @@
#C52 = 37.0
#C53 = "FooExtension|set#setterFoo"
#C54 = wid::_Location {file:#C35, line:#C51, column:#C52, name:#C53, parameterLocations:#C1}
- #C55 = 14.0
- #C56 = 28.0
- #C57 = "FooExtension|unary-"
- #C58 = wid::_Location {file:#C35, line:#C55, column:#C56, name:#C57, parameterLocations:#C1}
+ #C55 = 28.0
+ #C56 = "FooExtension|unary-"
+ #C57 = wid::_Location {file:#C35, line:#C14, column:#C55, name:#C56, parameterLocations:#C1}
}
diff --git a/pkg/pkg.status b/pkg/pkg.status
index 9542b63..cc9a9fc8 100644
--- a/pkg/pkg.status
+++ b/pkg/pkg.status
@@ -125,6 +125,7 @@
vm_service/test/coverage_closure_call_after_optimization_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
vm_service/test/coverage_closure_call_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
vm_service/test/coverage_const_field_async_closure_test: SkipByDesign # Debugger is disabled in AOT mode.
+vm_service/test/coverage_extension_methods_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
vm_service/test/coverage_instance_call_after_optimization_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
vm_service/test/coverage_leaf_function_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/coverage_optimized_function_test: SkipByDesign # Debugger is disabled in AOT mode.
diff --git a/pkg/vm_service/test/coverage_extension_methods_test.dart b/pkg/vm_service/test/coverage_extension_methods_test.dart
new file mode 100644
index 0000000..a545e05
--- /dev/null
+++ b/pkg/vm_service/test/coverage_extension_methods_test.dart
@@ -0,0 +1,107 @@
+// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:developer';
+
+import 'package:test/test.dart';
+import 'package:vm_service/vm_service.dart';
+
+import 'common/service_test_common.dart';
+import 'common/test_helper.dart';
+
+// AUTOGENERATED START
+//
+// Update these constants by running:
+//
+// dart pkg/vm_service/test/update_line_numbers.dart pkg/vm_service/test/coverage_extension_methods_test.dart
+//
+const OFFSET_GET_FROM = 0782;
+const OFFSET_BAZ_START = 0830;
+const OFFSET_PRINT = 0866;
+const OFFSET_BAZ_END = 0905;
+const OFFSET_GET_TO = 0928;
+// AUTOGENERATED END
+
+class Bar {}
+
+/* OFFSET_GET_FROM */ extension Foo on Bar {
+ /* OFFSET_BAZ_START */ void baz() {
+ /* OFFSET_PRINT */ print('hello'); /* OFFSET_BAZ_END */
+ }
+} /* OFFSET_GET_TO */
+
+void testFunction() {
+ debugger();
+ final bar = Bar();
+ bar.baz();
+ debugger();
+}
+
+Future<({Set<int> hits, Set<int> misses})> verifyLocationAndGetHitsAndMisses(
+ VmService service,
+ IsolateRef isolateRef,
+) async {
+ final isolateId = isolateRef.id!;
+ final stack = await service.getStack(isolateId);
+
+ // Make sure we are in the right place.
+ final frames = stack.frames!;
+ expect(frames.length, greaterThanOrEqualTo(1));
+ expect(frames[0].function!.name, 'testFunction');
+
+ final location = frames[0].function!.location!;
+ final report = await service.getSourceReport(
+ isolateId,
+ [SourceReportKind.kCoverage],
+ scriptId: location.script!.id!,
+ tokenPos: OFFSET_GET_FROM,
+ endTokenPos: OFFSET_GET_TO,
+ forceCompile: true,
+ );
+
+ final scripts = report.scripts!;
+ final ranges = report.ranges!;
+ final hits = <int>{};
+ final misses = <int>{};
+ // We have more ranges because there are several methods:
+ // The actual method, a tearoff and a closure inside the tearoff.
+ for (final range in ranges) {
+ hits.addAll(range.coverage!.hits!);
+ misses.addAll(range.coverage!.misses!);
+ expect(range.startPos, OFFSET_BAZ_START);
+ expect(range.endPos, OFFSET_BAZ_END);
+ expect(range.scriptIndex, 0);
+ }
+ misses.removeAll(hits);
+ expect(
+ scripts[0].uri,
+ endsWith('coverage_extension_methods_test.dart'),
+ );
+ return (hits: hits, misses: misses);
+}
+
+var tests = <IsolateTest>[
+ hasStoppedAtBreakpoint,
+ (VmService service, IsolateRef isolateRef) async {
+ final (:hits, :misses) =
+ await verifyLocationAndGetHitsAndMisses(service, isolateRef);
+ expect(hits, isEmpty);
+ expect(misses, {OFFSET_BAZ_START, OFFSET_PRINT});
+ },
+ resumeIsolate,
+ hasStoppedAtBreakpoint,
+ (VmService service, IsolateRef isolateRef) async {
+ final (:hits, :misses) =
+ await verifyLocationAndGetHitsAndMisses(service, isolateRef);
+ expect(hits, {OFFSET_BAZ_START, OFFSET_PRINT});
+ expect(misses, isEmpty);
+ },
+];
+
+void main(List<String> args) => runIsolateTests(
+ args,
+ tests,
+ 'coverage_extension_methods_test.dart',
+ testeeConcurrent: testFunction,
+ );
