Skip to content

Commit 8521d4f

Browse files
jensjohaCommit Queue
jensjoha
authored and
Commit Queue
committed
[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 <[email protected]> Reviewed-by: Slava Egorov <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 2d83ca1 commit 8521d4f

File tree

6 files changed

+174
-60
lines changed

6 files changed

+174
-60
lines changed

‎pkg/front_end/lib/src/fragment/method.dart‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,7 @@ mixin _ExtensionInstanceMethodEncodingMixin implements _MethodEncoding {
750750
Procedure procedure, NameScheme nameScheme, Reference? tearOffReference) {
751751
_extensionTearOffParameterMap = {};
752752

753+
int fileStartOffset = _fragment.startOffset;
753754
int fileOffset = _fragment.nameOffset;
754755
int fileEndOffset = _fragment.endOffset;
755756

@@ -837,7 +838,10 @@ mixin _ExtensionInstanceMethodEncodingMixin implements _MethodEncoding {
837838
procedure,
838839
new Arguments(closurePositionalArguments,
839840
types: typeArguments, named: closureNamedArguments))
840-
..fileOffset = fileOffset)
841+
// We need to use the fileStartOffset on the StaticInvocation to
842+
// avoid a possible "fake coverage miss" on the name of the
843+
// extension method.
844+
..fileOffset = fileStartOffset)
841845
..fileOffset = fileOffset;
842846

843847
FunctionExpression closure = new FunctionExpression(
@@ -850,7 +854,10 @@ mixin _ExtensionInstanceMethodEncodingMixin implements _MethodEncoding {
850854
returnType: closureReturnType)
851855
..fileOffset = fileOffset
852856
..fileEndOffset = fileEndOffset)
853-
..fileOffset = fileOffset;
857+
// We need to use the fileStartOffset on the FunctionExpression to
858+
// avoid a possible "fake coverage miss" on the name of the
859+
// extension method.
860+
..fileOffset = fileStartOffset;
854861

855862
FunctionNode function = new FunctionNode(
856863
new ReturnStatement(closure)..fileOffset = fileOffset,

‎pkg/front_end/test/coverage_suite_expected.dart‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
148148
hitCount: 97,
149149
missCount: 0,
150150
),
151-
// 99.88331388564761%.
151+
// 100.0%.
152152
"package:front_end/src/base/incremental_compiler.dart": (
153153
hitCount: 856,
154-
missCount: 1,
154+
missCount: 0,
155155
),
156156
// 100.0%.
157157
"package:front_end/src/base/incremental_serializer.dart": (
@@ -183,10 +183,10 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
183183
hitCount: 31,
184184
missCount: 0,
185185
),
186-
// 99.26470588235294%.
186+
// 100.0%.
187187
"package:front_end/src/base/modifiers.dart": (
188188
hitCount: 135,
189-
missCount: 1,
189+
missCount: 0,
190190
),
191191
// 100.0%.
192192
"package:front_end/src/base/name_space.dart": (
@@ -208,10 +208,10 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
208208
hitCount: 260,
209209
missCount: 0,
210210
),
211-
// 99.38837920489296%.
211+
// 100.0%.
212212
"package:front_end/src/base/scope.dart": (
213213
hitCount: 650,
214-
missCount: 4,
214+
missCount: 0,
215215
),
216216
// 100.0%.
217217
"package:front_end/src/base/ticker.dart": (
@@ -535,7 +535,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
535535
),
536536
// 100.0%.
537537
"package:front_end/src/fragment/method.dart": (
538-
hitCount: 771,
538+
hitCount: 773,
539539
missCount: 0,
540540
),
541541
// 100.0%.
@@ -920,7 +920,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
920920
),
921921
// 100.0%.
922922
"package:front_end/src/source/source_constructor_builder.dart": (
923-
hitCount: 1085,
923+
hitCount: 1084,
924924
missCount: 0,
925925
),
926926
// 100.0%.
@@ -941,13 +941,13 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
941941
),
942942
// 100.0%.
943943
"package:front_end/src/source/source_factory_builder.dart": (
944-
hitCount: 717,
944+
hitCount: 715,
945945
missCount: 0,
946946
),
947-
// 97.91666666666666%.
947+
// 100.0%.
948948
"package:front_end/src/source/source_function_builder.dart": (
949949
hitCount: 47,
950-
missCount: 1,
950+
missCount: 0,
951951
),
952952
// 100.0%.
953953
"package:front_end/src/source/source_library_builder.dart": (
@@ -991,7 +991,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
991991
),
992992
// 100.0%.
993993
"package:front_end/src/source/type_parameter_scope_builder.dart": (
994-
hitCount: 1445,
994+
hitCount: 1446,
995995
missCount: 0,
996996
),
997997
// 100.0%.
@@ -1109,10 +1109,10 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
11091109
hitCount: 20,
11101110
missCount: 0,
11111111
),
1112-
// 92.0%.
1112+
// 100.0%.
11131113
"package:front_end/src/util/local_stack.dart": (
11141114
hitCount: 23,
1115-
missCount: 2,
1115+
missCount: 0,
11161116
),
11171117
// 100.0%.
11181118
"package:front_end/src/util/parser_ast.dart": (

‎pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.1.expect‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -121,18 +121,18 @@ library from "org-dartlang-test:///main.dart" as main {
121121

122122
import "org-dartlang-test:///foo.dart";
123123

124-
static field foo::Foo newFoo = foo::Foo::•($creationLocationd_0dea112b090073317d4: #C37);
125-
static field () → foo::Foo newFooFunction = #C38;
124+
static field foo::Foo newFoo = foo::Foo::•($creationLocationd_0dea112b090073317d4: #C38);
125+
static field () → foo::Foo newFooFunction = #C39;
126126
static field foo::Foo newFooFunctionCall = main::newFooFunction(){() → foo::Foo};
127-
static field foo::Foo extensionFoo = foo::FooExtension|foo(null, $creationLocationd_0dea112b090073317d4: #C41);
127+
static field foo::Foo extensionFoo = foo::FooExtension|foo(null, $creationLocationd_0dea112b090073317d4: #C42);
128128
static field () → foo::Foo extensionFooFunction = foo::FooExtension|get#foo(null);
129129
static field foo::Foo extensionFooFunctionCall = main::extensionFooFunction(){() → foo::Foo};
130-
static field foo::Foo extensionBar = foo::FooExtension|bar(null, $creationLocationd_0dea112b090073317d4: #C42);
130+
static field foo::Foo extensionBar = foo::FooExtension|bar(null, $creationLocationd_0dea112b090073317d4: #C43);
131131
static field foo::Foo extensionBaz = foo::FooExtension|baz(null);
132-
static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C43);
133-
static field foo::Foo extensionConstFoo = foo::FooExtension|constFoo(null, $creationLocationd_0dea112b090073317d4: #C45);
134-
static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C49);
135-
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;
132+
static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C44);
133+
static field foo::Foo extensionConstFoo = foo::FooExtension|constFoo(null, $creationLocationd_0dea112b090073317d4: #C46);
134+
static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C50);
135+
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;
136136
static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C57);
137137
static field foo::Foo extensionStaticFoo = foo::FooExtension|staticFoo();
138138
}
@@ -150,50 +150,50 @@ constants {
150150
#C11 = 10.0
151151
#C12 = 9.0
152152
#C13 = wid::_Location {file:#C3, line:#C11, column:#C12, name:#C6, parameterLocations:#C1}
153-
#C14 = 15.0
154-
#C15 = 7.0
153+
#C14 = 14.0
154+
#C15 = 3.0
155155
#C16 = "FooExtension|foo"
156156
#C17 = wid::_Location {file:#C3, line:#C14, column:#C15, name:#C16, parameterLocations:#C1}
157-
#C18 = 18.0
157+
#C18 = 17.0
158158
#C19 = "FooExtension|bar"
159159
#C20 = wid::_Location {file:#C3, line:#C18, column:#C15, name:#C19, parameterLocations:#C1}
160160
#C21 = 21.0
161161
#C22 = 20.0
162162
#C23 = wid::_Location {file:#C3, line:#C21, column:#C22, name:#C6, parameterLocations:#C1}
163-
#C24 = 24.0
163+
#C24 = 23.0
164164
#C25 = "FooExtension|boz"
165165
#C26 = wid::_Location {file:#C3, line:#C24, column:#C15, name:#C25, parameterLocations:#C1}
166166
#C27 = 27.0
167167
#C28 = wid::_Location {file:#C3, line:#C27, column:#C27, name:#C6, parameterLocations:#C1}
168168
#C29 = foo::Foo {_location:#C28}
169169
#C30 = "FooExtension|constFoo"
170-
#C31 = wid::_Location {file:#C3, line:#C27, column:#C15, name:#C30, parameterLocations:#C1}
170+
#C31 = wid::_Location {file:#C3, line:#C5, column:#C15, name:#C30, parameterLocations:#C1}
171171
#C32 = 39.0
172172
#C33 = 33.0
173173
#C34 = wid::_Location {file:#C3, line:#C32, column:#C33, name:#C6, parameterLocations:#C1}
174174
#C35 = "org-dartlang-test:///main.dart"
175175
#C36 = 2.0
176-
#C37 = wid::_Location {file:#C35, line:#C36, column:#C18, name:#C6, parameterLocations:#C1}
177-
#C38 = static-tearoff foo::Foo::_#new#tearOff
178-
#C39 = 5.0
179-
#C40 = 25.0
180-
#C41 = wid::_Location {file:#C35, line:#C39, column:#C40, name:#C16, parameterLocations:#C1}
181-
#C42 = wid::_Location {file:#C35, line:#C4, column:#C40, name:#C19, parameterLocations:#C1}
182-
#C43 = wid::_Location {file:#C35, line:#C11, column:#C40, name:#C25, parameterLocations:#C1}
183-
#C44 = 30.0
184-
#C45 = wid::_Location {file:#C35, line:#C9, column:#C44, name:#C30, parameterLocations:#C1}
185-
#C46 = 12.0
186-
#C47 = 31.0
187-
#C48 = "FooExtension|get#getterFoo"
188-
#C49 = wid::_Location {file:#C35, line:#C46, column:#C47, name:#C48, parameterLocations:#C1}
189-
#C50 = 13.0
190-
#C51 = 37.0
191-
#C52 = "FooExtension|set#setterFoo"
192-
#C53 = wid::_Location {file:#C35, line:#C50, column:#C51, name:#C52, parameterLocations:#C1}
193-
#C54 = 14.0
176+
#C37 = 18.0
177+
#C38 = wid::_Location {file:#C35, line:#C36, column:#C37, name:#C6, parameterLocations:#C1}
178+
#C39 = static-tearoff foo::Foo::_#new#tearOff
179+
#C40 = 5.0
180+
#C41 = 25.0
181+
#C42 = wid::_Location {file:#C35, line:#C40, column:#C41, name:#C16, parameterLocations:#C1}
182+
#C43 = wid::_Location {file:#C35, line:#C4, column:#C41, name:#C19, parameterLocations:#C1}
183+
#C44 = wid::_Location {file:#C35, line:#C11, column:#C41, name:#C25, parameterLocations:#C1}
184+
#C45 = 30.0
185+
#C46 = wid::_Location {file:#C35, line:#C9, column:#C45, name:#C30, parameterLocations:#C1}
186+
#C47 = 12.0
187+
#C48 = 31.0
188+
#C49 = "FooExtension|get#getterFoo"
189+
#C50 = wid::_Location {file:#C35, line:#C47, column:#C48, name:#C49, parameterLocations:#C1}
190+
#C51 = 13.0
191+
#C52 = 37.0
192+
#C53 = "FooExtension|set#setterFoo"
193+
#C54 = wid::_Location {file:#C35, line:#C51, column:#C52, name:#C53, parameterLocations:#C1}
194194
#C55 = 28.0
195195
#C56 = "FooExtension|unary-"
196-
#C57 = wid::_Location {file:#C35, line:#C54, column:#C55, name:#C56, parameterLocations:#C1}
196+
#C57 = wid::_Location {file:#C35, line:#C14, column:#C55, name:#C56, parameterLocations:#C1}
197197
}
198198

199199

‎pkg/front_end/testcases/incremental/flutter_widget_factory.yaml.world.2.expect‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ library from "org-dartlang-test:///main.dart" as main {
133133
static field foo::Foo extensionBoz = foo::FooExtension|boz(null, $creationLocationd_0dea112b090073317d4: #C46);
134134
static field foo::Foo extensionGetterFoo = foo::FooExtension|get#getterFoo(null, $creationLocationd_0dea112b090073317d4: #C50);
135135
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;
136-
static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C58);
136+
static field foo::Foo extensionOperatorFoo = foo::FooExtension|unary-(null, $creationLocationd_0dea112b090073317d4: #C57);
137137
static field foo::Foo extensionStaticFoo = foo::FooExtension|staticFoo();
138138
}
139139
constants {
@@ -150,33 +150,33 @@ constants {
150150
#C11 = 10.0
151151
#C12 = 9.0
152152
#C13 = wid::_Location {file:#C3, line:#C11, column:#C12, name:#C6, parameterLocations:#C1}
153-
#C14 = 15.0
154-
#C15 = 7.0
153+
#C14 = 14.0
154+
#C15 = 3.0
155155
#C16 = "FooExtension|foo"
156156
#C17 = wid::_Location {file:#C3, line:#C14, column:#C15, name:#C16, parameterLocations:#C1}
157-
#C18 = 18.0
157+
#C18 = 17.0
158158
#C19 = "FooExtension|bar"
159159
#C20 = wid::_Location {file:#C3, line:#C18, column:#C15, name:#C19, parameterLocations:#C1}
160160
#C21 = 21.0
161161
#C22 = 20.0
162162
#C23 = wid::_Location {file:#C3, line:#C21, column:#C22, name:#C6, parameterLocations:#C1}
163-
#C24 = 24.0
163+
#C24 = 23.0
164164
#C25 = "FooExtension|boz"
165165
#C26 = wid::_Location {file:#C3, line:#C24, column:#C15, name:#C25, parameterLocations:#C1}
166166
#C27 = 27.0
167167
#C28 = wid::_Location {file:#C3, line:#C27, column:#C27, name:#C6, parameterLocations:#C1}
168168
#C29 = foo::Foo {_location:#C28}
169169
#C30 = "FooExtension|constFoo"
170-
#C31 = wid::_Location {file:#C3, line:#C27, column:#C15, name:#C30, parameterLocations:#C1}
170+
#C31 = wid::_Location {file:#C3, line:#C5, column:#C15, name:#C30, parameterLocations:#C1}
171171
#C32 = 39.0
172172
#C33 = 33.0
173173
#C34 = wid::_Location {file:#C3, line:#C32, column:#C33, name:#C6, parameterLocations:#C1}
174174
#C35 = "org-dartlang-test:///main.dart"
175175
#C36 = 2.0
176176
#C37 = 30.0
177177
#C38 = wid::_Location {file:#C35, line:#C36, column:#C37, name:#C30, parameterLocations:#C1}
178-
#C39 = 3.0
179-
#C40 = wid::_Location {file:#C35, line:#C39, column:#C18, name:#C6, parameterLocations:#C1}
178+
#C39 = 18.0
179+
#C40 = wid::_Location {file:#C35, line:#C15, column:#C39, name:#C6, parameterLocations:#C1}
180180
#C41 = static-tearoff foo::Foo::_#new#tearOff
181181
#C42 = 6.0
182182
#C43 = 25.0
@@ -191,10 +191,9 @@ constants {
191191
#C52 = 37.0
192192
#C53 = "FooExtension|set#setterFoo"
193193
#C54 = wid::_Location {file:#C35, line:#C51, column:#C52, name:#C53, parameterLocations:#C1}
194-
#C55 = 14.0
195-
#C56 = 28.0
196-
#C57 = "FooExtension|unary-"
197-
#C58 = wid::_Location {file:#C35, line:#C55, column:#C56, name:#C57, parameterLocations:#C1}
194+
#C55 = 28.0
195+
#C56 = "FooExtension|unary-"
196+
#C57 = wid::_Location {file:#C35, line:#C14, column:#C55, name:#C56, parameterLocations:#C1}
198197
}
199198

200199

‎pkg/pkg.status‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ vm_service/test/coverage_async_test: SkipByDesign # Debugger is disabled in AOT
125125
vm_service/test/coverage_closure_call_after_optimization_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
126126
vm_service/test/coverage_closure_call_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
127127
vm_service/test/coverage_const_field_async_closure_test: SkipByDesign # Debugger is disabled in AOT mode.
128+
vm_service/test/coverage_extension_methods_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
128129
vm_service/test/coverage_instance_call_after_optimization_test: SkipByDesign # Debugger and coverage are disabled in AOT mode.
129130
vm_service/test/coverage_leaf_function_test: SkipByDesign # Debugger is disabled in AOT mode.
130131
vm_service/test/coverage_optimized_function_test: SkipByDesign # Debugger is disabled in AOT mode.

0 commit comments

Comments
 (0)