Skip to content

Commit 7c7160f

Browse files
sigmundchcommit-bot@chromium.org
authored andcommitted
Move isRedirectingFactory to CFE and add unit test for it.
Eventually it should be handled by Procedure.isRedirectingFactoryConstructor, but that is currently broken for patch files. It appears the VM has no patches of that kind, but dart2js does. Change-Id: I0b64379f16089ad0d98ecd4df1ed9282ed6bc0e7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98523 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Sigmund Cherem <[email protected]>
1 parent e656760 commit 7c7160f

4 files changed

Lines changed: 114 additions & 13 deletions

File tree

pkg/compiler/lib/src/kernel/env.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
library dart2js.kernel.env;
66

7-
import 'package:front_end/src/api_unstable/dart2js.dart' as ir
8-
show RedirectingFactoryBody;
7+
import 'package:front_end/src/api_unstable/dart2js.dart'
8+
show isRedirectingFactory;
99

1010
import 'package:kernel/ast.dart' as ir;
1111
import 'package:kernel/clone.dart';
@@ -415,7 +415,7 @@ class KClassEnvImpl implements KClassEnv {
415415
var name = member.name.name;
416416
assert(!name.contains('#'));
417417
if (member.kind == ir.ProcedureKind.Factory) {
418-
if (member.function.body is ir.RedirectingFactoryBody) {
418+
if (isRedirectingFactory(member)) {
419419
// Don't include redirecting factories.
420420
return;
421421
}

pkg/front_end/lib/src/api_unstable/dart2js.dart

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import 'dart:async' show Future;
66

77
import 'package:kernel/kernel.dart' show Component;
88

9+
import 'package:kernel/ast.dart' as ir;
10+
911
import 'package:kernel/target/targets.dart' show Target;
1012

1113
import '../api_prototype/compiler_options.dart' show CompilerOptions;
@@ -30,6 +32,8 @@ import '../kernel_generator_impl.dart' show generateKernelInternal;
3032

3133
import '../fasta/scanner.dart' show ErrorToken, StringToken, Token;
3234

35+
import '../fasta/kernel/redirecting_factory_body.dart' as redirecting;
36+
3337
import 'compiler_state.dart' show InitializedCompilerState;
3438

3539
import 'util.dart' show equalLists, equalMaps;
@@ -61,9 +65,6 @@ export '../compute_platform_binaries_location.dart'
6165

6266
export '../fasta/fasta_codes.dart' show LocatedMessage;
6367

64-
export '../fasta/kernel/redirecting_factory_body.dart'
65-
show RedirectingFactoryBody;
66-
6768
export '../fasta/operator.dart' show operatorFromString;
6869

6970
export '../fasta/parser/async_modifier.dart' show AsyncModifier;
@@ -212,3 +213,23 @@ Iterable<String> getSupportedLibraryNames(
212213
.where((l) => l.isSupported)
213214
.map((l) => l.name);
214215
}
216+
217+
/// Desugar API to determine whether [member] is a redirecting factory
218+
/// constructor.
219+
// TODO(sigmund): Delete this API once `member.isRedirectingFactoryConstructor`
220+
// is implemented correctly for patch files (Issue #33495).
221+
bool isRedirectingFactory(ir.Procedure member) {
222+
if (member.kind == ir.ProcedureKind.Factory) {
223+
var body = member.function.body;
224+
if (body is redirecting.RedirectingFactoryBody) return true;
225+
if (body is ir.ExpressionStatement) {
226+
ir.Expression expression = body.expression;
227+
if (expression is ir.Let) {
228+
if (expression.variable.name == redirecting.letName) {
229+
return true;
230+
}
231+
}
232+
}
233+
}
234+
return false;
235+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) 2019, 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+
/// Test to ensure that desugaring APIs used by clients like dart2js are
6+
/// always up to date.
7+
///
8+
/// Desugaring APIs are methods that inspect the kernel output to find
9+
/// structures that are assumed to always be generated by the CFE. If the CFE
10+
/// changes its invariants, the desugaring APIs will change together to hide
11+
/// these changes from clients.
12+
13+
import 'dart:io';
14+
15+
import 'package:async_helper/async_helper.dart';
16+
import 'package:expect/expect.dart';
17+
import 'package:front_end/src/api_unstable/dart2js.dart' as api;
18+
import 'package:front_end/src/compute_platform_binaries_location.dart';
19+
import 'package:front_end/src/fasta/kernel/utils.dart' show serializeComponent;
20+
import 'package:front_end/src/testing/compiler_common.dart';
21+
import 'package:kernel/ast.dart' as ir;
22+
import 'package:kernel/binary/ast_from_binary.dart' show BinaryBuilder;
23+
24+
main() async {
25+
await asyncTest(() async {
26+
await testRedirectingFactoryDirect();
27+
await testRedirectingFactorySerialized();
28+
await testRedirectingFactoryPatchFile();
29+
});
30+
}
31+
32+
testRedirectingFactoryDirect() async {
33+
var component = await compileUnit(['a.dart'], {'a.dart': aSource});
34+
checkIsRedirectingFactory(component, 'a.dart', 'A', 'foo');
35+
checkIsRedirectingFactory(component, 'core', 'Uri', 'file');
36+
}
37+
38+
testRedirectingFactorySerialized() async {
39+
var component = await compileUnit(['a.dart'], {'a.dart': aSource});
40+
var bytes = serializeComponent(component);
41+
component = new ir.Component();
42+
new BinaryBuilder(bytes).readComponent(component);
43+
checkIsRedirectingFactory(component, 'a.dart', 'A', 'foo');
44+
checkIsRedirectingFactory(component, 'core', 'Uri', 'file');
45+
}
46+
47+
// regression test: redirecting factories from patch files don't have the
48+
// redirecting-factory flag stored in kernel.
49+
testRedirectingFactoryPatchFile() async {
50+
var componentUri =
51+
computePlatformBinariesLocation().resolve('dart2js_platform.dill');
52+
var component = new ir.Component();
53+
new BinaryBuilder(new File.fromUri(componentUri).readAsBytesSync())
54+
.readComponent(component);
55+
checkIsRedirectingFactory(component, 'collection', 'HashMap', 'identity',
56+
isPatch: true);
57+
}
58+
59+
void checkIsRedirectingFactory(ir.Component component, String uriPath,
60+
String className, String constructorName,
61+
{bool isPatch: false}) {
62+
var lib =
63+
component.libraries.firstWhere((l) => l.importUri.path.endsWith(uriPath));
64+
var cls = lib.classes.firstWhere((c) => c.name == className);
65+
ir.Procedure member =
66+
cls.members.firstWhere((m) => m.name.name == constructorName);
67+
Expect.isTrue(
68+
member.kind == ir.ProcedureKind.Factory, "$member is not a factory");
69+
Expect.isTrue(api.isRedirectingFactory(member));
70+
// TODO: this should always be true. Issue #33495
71+
Expect.equals(!isPatch, member.isRedirectingFactoryConstructor);
72+
}
73+
74+
const aSource = '''
75+
class A {
76+
factory A.foo(int x) = _B;
77+
}
78+
79+
class _B implements A {
80+
_B(int x);
81+
}
82+
''';

tests/compiler/dart2js/analyses/analysis_helper.dart

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ import 'package:compiler/src/kernel/dart2js_target.dart';
1919
import 'package:compiler/src/kernel/loader.dart';
2020
import 'package:compiler/src/util/uri_extras.dart';
2121
import 'package:expect/expect.dart';
22-
import 'package:front_end/src/api_unstable/dart2js.dart' as ir
23-
show RedirectingFactoryBody;
2422
import 'package:front_end/src/api_prototype/constant_evaluator.dart' as ir;
23+
import 'package:front_end/src/api_unstable/dart2js.dart'
24+
show isRedirectingFactory;
2525
import 'package:kernel/ast.dart' as ir;
2626
import 'package:kernel/class_hierarchy.dart' as ir;
2727
import 'package:kernel/core_types.dart' as ir;
@@ -97,11 +97,9 @@ class StaticTypeVisitorBase extends StaticTypeVisitor {
9797

9898
@override
9999
Null visitProcedure(ir.Procedure node) {
100-
if (node.kind == ir.ProcedureKind.Factory) {
101-
if (node.function.body is ir.RedirectingFactoryBody) {
102-
// Don't visit redirecting factories.
103-
return;
104-
}
100+
if (node.kind == ir.ProcedureKind.Factory && isRedirectingFactory(node)) {
101+
// Don't visit redirecting factories.
102+
return;
105103
}
106104
if (node.name.name.contains('#')) {
107105
// Skip synthetic .dill members.

0 commit comments

Comments
 (0)