Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit deca91c

Browse files
jensjohacommit-bot@chromium.org
authored andcommitted
[CFE] Record const constructor coverage when evaluating constants
This CL records which const constructors were evaluated (i.e. contributes coverage) but doesn't actually save the coverage into the dill file. runtimes for constant evaluation: dart2js: No difference proven at 95.0% confidence flutter gallery: No difference proven at 95.0% confidence big internal app: No difference proven at 95.0% confidence sdk sizes: vm: no change dart2js: no change flutter: no change compile sizes: dart2js: no change flutter gallery: no change big internal app: no change Change-Id: I2567446c1b78180d73fd7613bc9e780e1a067f5b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170435 Commit-Queue: Jens Johansen <jensj@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com>
1 parent 08f3b03 commit deca91c

13 files changed

+577
-17
lines changed

pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ Component transformComponent(
104104
return component;
105105
}
106106

107-
void transformLibraries(
107+
ConstantCoverage transformLibraries(
108108
List<Library> libraries,
109109
ConstantsBackend backend,
110110
Map<String, String> environmentDefines,
@@ -132,6 +132,7 @@ void transformLibraries(
132132
for (final Library library in libraries) {
133133
constantsTransformer.convertLibrary(library);
134134
}
135+
return constantsTransformer.constantEvaluator.getConstantCoverage();
135136
}
136137

137138
void transformProcedure(
@@ -1010,6 +1011,18 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
10101011
return lower(constant, backend.lowerMapConstant(constant));
10111012
}
10121013

1014+
Map<Uri, Set<Reference>> _constructorCoverage = {};
1015+
1016+
ConstantCoverage getConstantCoverage() {
1017+
return new ConstantCoverage(_constructorCoverage);
1018+
}
1019+
1020+
void _recordConstructorCoverage(Constructor constructor, TreeNode caller) {
1021+
Uri currentUri = getFileUri(caller);
1022+
Set<Reference> uriCoverage = _constructorCoverage[currentUri] ??= {};
1023+
uriCoverage.add(constructor.reference);
1024+
}
1025+
10131026
/// Evaluate [node] and possibly cache the evaluation result.
10141027
///
10151028
/// Returns [_AbortDueToErrorConstant] or
@@ -1344,13 +1357,13 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
13441357
if (shouldBeUnevaluated) {
13451358
enterLazy();
13461359
AbortConstant error = handleConstructorInvocation(
1347-
constructor, typeArguments, positionals, named);
1360+
constructor, typeArguments, positionals, named, node);
13481361
if (error != null) return error;
13491362
leaveLazy();
13501363
return unevaluated(node, instanceBuilder.buildUnevaluatedInstance());
13511364
}
13521365
AbortConstant error = handleConstructorInvocation(
1353-
constructor, typeArguments, positionals, named);
1366+
constructor, typeArguments, positionals, named, node);
13541367
if (error != null) return error;
13551368
if (shouldBeUnevaluated) {
13561369
return unevaluated(node, instanceBuilder.buildUnevaluatedInstance());
@@ -1547,11 +1560,15 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
15471560
Constructor constructor,
15481561
List<DartType> typeArguments,
15491562
List<Constant> positionalArguments,
1550-
Map<String, Constant> namedArguments) {
1563+
Map<String, Constant> namedArguments,
1564+
TreeNode caller) {
15511565
return withNewEnvironment(() {
15521566
final Class klass = constructor.enclosingClass;
15531567
final FunctionNode function = constructor.function;
15541568

1569+
// Mark in file of the caller that we evaluate this specific constructor.
1570+
_recordConstructorCoverage(constructor, caller);
1571+
15551572
// We simulate now the constructor invocation.
15561573

15571574
// Step 1) Map type arguments and normal arguments from caller to
@@ -1626,8 +1643,8 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
16261643
}
16271644
assert(_gotError == null);
16281645
assert(namedArguments != null);
1629-
error = handleConstructorInvocation(
1630-
init.target, types, positionalArguments, namedArguments);
1646+
error = handleConstructorInvocation(init.target, types,
1647+
positionalArguments, namedArguments, constructor);
16311648
if (error != null) return error;
16321649
} else if (init is RedirectingInitializer) {
16331650
// Since a redirecting constructor targets a constructor of the same
@@ -1654,8 +1671,8 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
16541671
assert(_gotError == null);
16551672
assert(namedArguments != null);
16561673

1657-
error = handleConstructorInvocation(
1658-
init.target, typeArguments, positionalArguments, namedArguments);
1674+
error = handleConstructorInvocation(init.target, typeArguments,
1675+
positionalArguments, namedArguments, constructor);
16591676
if (error != null) return error;
16601677
} else if (init is AssertInitializer) {
16611678
AbortConstant error = checkAssert(init.statement);
@@ -2824,6 +2841,12 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
28242841
}
28252842
}
28262843

2844+
class ConstantCoverage {
2845+
final Map<Uri, Set<Reference>> constructorCoverage;
2846+
2847+
ConstantCoverage(this.constructorCoverage);
2848+
}
2849+
28272850
/// Holds the necessary information for a constant object, namely
28282851
/// * the [klass] being instantiated
28292852
/// * the [typeArguments] used for the instantiation

pkg/front_end/lib/src/fasta/kernel/kernel_target.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ import '../source/source_loader.dart' show SourceLoader;
105105
import '../target_implementation.dart' show TargetImplementation;
106106
import '../uri_translator.dart' show UriTranslator;
107107
import 'constant_evaluator.dart' as constants
108-
show EvaluationMode, transformLibraries, transformProcedure;
108+
show
109+
EvaluationMode,
110+
transformLibraries,
111+
transformProcedure,
112+
ConstantCoverage;
109113
import 'kernel_constants.dart' show KernelConstantErrorReporter;
110114
import 'metadata_collector.dart' show MetadataCollector;
111115
import 'verifier.dart' show verifyComponent, verifyGetStaticType;
@@ -126,6 +130,10 @@ class KernelTarget extends TargetImplementation {
126130

127131
Component component;
128132

133+
/// Temporary field meant for testing only. Follow-up CLs should get rid of
134+
/// this and integrate coverage properly.
135+
constants.ConstantCoverage constantCoverageForTesting;
136+
129137
// 'dynamic' is always nullable.
130138
// TODO(johnniwinther): Why isn't this using a FixedTypeBuilder?
131139
final TypeBuilder dynamicType = new NamedTypeBuilder(
@@ -1231,7 +1239,7 @@ class KernelTarget extends TargetImplementation {
12311239
new TypeEnvironment(loader.coreTypes, loader.hierarchy);
12321240
constants.EvaluationMode evaluationMode = _getConstantEvaluationMode();
12331241

1234-
constants.transformLibraries(
1242+
constants.ConstantCoverage coverage = constants.transformLibraries(
12351243
loader.libraries,
12361244
backendTarget.constantsBackend(loader.coreTypes),
12371245
environmentDefines,
@@ -1244,6 +1252,7 @@ class KernelTarget extends TargetImplementation {
12441252
isExperimentEnabledGlobally(ExperimentalFlag.tripleShift),
12451253
errorOnUnevaluatedConstant: errorOnUnevaluatedConstant);
12461254
ticker.logMs("Evaluated constants");
1255+
constantCoverageForTesting = coverage;
12471256

12481257
if (loader.target.context.options
12491258
.isExperimentEnabledGlobally(ExperimentalFlag.valueClass)) {

pkg/front_end/test/fasta/testing/suite.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,16 +1115,16 @@ class Outline extends Step<TestDescription, ComponentResult, FastaContext> {
11151115
await instrumentation.fixSource(description.uri, false);
11161116
} else {
11171117
return new Result<ComponentResult>(
1118-
new ComponentResult(
1119-
description, p, userLibraries, options, sourceTarget),
1118+
new ComponentResult(description, p, userLibraries, options,
1119+
sourceTarget, sourceTarget.constantCoverageForTesting),
11201120
context.expectationSet["InstrumentationMismatch"],
11211121
instrumentation.problemsAsString,
11221122
autoFixCommand: '${UPDATE_COMMENTS}=true');
11231123
}
11241124
}
11251125
}
1126-
return pass(new ComponentResult(
1127-
description, p, userLibraries, options, sourceTarget));
1126+
return pass(new ComponentResult(description, p, userLibraries, options,
1127+
sourceTarget, sourceTarget.constantCoverageForTesting));
11281128
});
11291129
}
11301130

pkg/front_end/test/spell_checking_list_code.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ corrections
245245
cosmetic
246246
counters
247247
covariances
248+
coverage
248249
cr
249250
creator
250251
criterion
@@ -547,6 +548,7 @@ inspired
547548
inst
548549
instanceof
549550
instantiator
551+
integrate
550552
intentionally
551553
interim
552554
interior

pkg/front_end/test/utils/kernel_chain.dart

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import 'package:front_end/src/compute_platform_binaries_location.dart'
2323
import 'package:front_end/src/fasta/compiler_context.dart' show CompilerContext;
2424

2525
import 'package:front_end/src/fasta/fasta_codes.dart' show templateUnspecified;
26+
import 'package:front_end/src/fasta/kernel/constant_evaluator.dart'
27+
show ConstantCoverage;
2628

2729
import 'package:front_end/src/fasta/kernel/kernel_target.dart'
2830
show KernelTarget;
@@ -293,6 +295,24 @@ class MatchExpectation
293295
buffer.writeln(extraConstantString);
294296
}
295297
}
298+
// TODO(jensj): Don't comment this out. Will be done in a follow-up-CL.
299+
// if (result.constantCoverage != null) {
300+
// ConstantCoverage constantCoverage = result.constantCoverage;
301+
// if (constantCoverage.constructorCoverage.isNotEmpty) {
302+
// buffer.writeln("");
303+
// buffer.writeln("");
304+
// buffer.writeln("Constructor coverage from constants:");
305+
// for (MapEntry<Uri, Set<Reference>> entry
306+
// in constantCoverage.constructorCoverage.entries) {
307+
// buffer.writeln("${entry.key}:");
308+
// for (Reference reference in entry.value) {
309+
// buffer.writeln(
310+
// "- ${reference.node} (from ${reference.node.location})");
311+
// }
312+
// buffer.writeln("");
313+
// }
314+
// }
315+
// }
296316

297317
String actual = "$buffer";
298318
String binariesPath =
@@ -393,8 +413,14 @@ class WriteDill extends Step<ComponentResult, ComponentResult, ChainContext> {
393413
Uri uri = tmp.uri.resolve("generated.dill");
394414
File generated = new File.fromUri(uri);
395415
IOSink sink = generated.openWrite();
396-
result = new ComponentResult(result.description, result.component,
397-
result.userLibraries, result.options, result.sourceTarget, uri);
416+
result = new ComponentResult(
417+
result.description,
418+
result.component,
419+
result.userLibraries,
420+
result.options,
421+
result.sourceTarget,
422+
result.constantCoverage,
423+
uri);
398424
try {
399425
new BinaryPrinter(sink).writeComponentFile(component);
400426
} catch (e, s) {
@@ -493,9 +519,10 @@ class ComponentResult {
493519
final ProcessedOptions options;
494520
final KernelTarget sourceTarget;
495521
final List<String> extraConstantStrings = [];
522+
final ConstantCoverage constantCoverage;
496523

497524
ComponentResult(this.description, this.component, this.userLibraries,
498-
this.options, this.sourceTarget,
525+
this.options, this.sourceTarget, this.constantCoverage,
499526
[this.outputUri]);
500527

501528
bool isUserLibrary(Library library) {
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import "const_constructor_coverage_lib1.dart";
6+
import "const_constructor_coverage_lib2.dart";
7+
8+
const Foo foo1 = const Foo();
9+
const Foo foo2 = const Foo.named1();
10+
const Foo foo3 = const Foo.named2();
11+
const Foo foo4 = const Foo.named3();
12+
13+
main() {
14+
print(foo1);
15+
}
16+
17+
// This file in itself should mark the following as const-constructor-covered:
18+
// * "Foo", "Foo.named1", "Foo.named2" and "Foo.named3" from constant-evaluating
19+
// the const fields.
20+
21+
// Notice, that combined these 3 files should have coverage for:
22+
// * "Foo", "Foo.named1", "Foo.named2", "Foo.named3",
23+
// * "Bar", "Bar.named1", "Bar.named2", "Bar.named3",
24+
// * "Baz", "Baz.named1", "Baz.named4", "Baz.named5", "Baz.named6"
25+
// but NOT have any coverage for
26+
// * "Bar.named4",
27+
// * "Baz.named2" and "Baz.named3".

0 commit comments

Comments
 (0)