Skip to content

Commit

Permalink
Bypass use of dart2js_platform.dill when experimental flags are non-d…
Browse files Browse the repository at this point in the history
…efault

Change-Id: I95e654c6c2a430f7aebb30ced92b33cd35aadca3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97634
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
johnniwinther authored and commit-bot@chromium.org committed Mar 26, 2019
1 parent b2a2c08 commit a8e51c5
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 52 deletions.
2 changes: 1 addition & 1 deletion pkg/compiler/lib/src/dart2js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Future<api.CompilationResult> compile(List<String> argv,
bool showHints;
bool enableColors;
int optimizationLevel = null;
Uri platformBinaries = fe.computePlatformBinariesLocation();
Uri platformBinaries;
Map<String, String> environment = new Map<String, String>();
CompilationStrategy compilationStrategy = CompilationStrategy.direct;

Expand Down
9 changes: 7 additions & 2 deletions pkg/compiler/lib/src/kernel/loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ class KernelLoaderTask extends CompilerTask {
if (_options.dillDependencies != null) {
// Modular compiles do not include the platform on the input dill
// either.
await read(_options.platformBinaries.resolve(platform));
if (_options.platformBinaries != null) {
await read(_options.platformBinaries.resolve(platform));
}
for (Uri dependency in _options.dillDependencies) {
await read(dependency);
}
Expand All @@ -90,7 +92,10 @@ class KernelLoaderTask extends CompilerTask {
return null;
}
} else {
List<Uri> dependencies = [_options.platformBinaries.resolve(platform)];
List<Uri> dependencies = [];
if (_options.platformBinaries != null) {
dependencies.add(_options.platformBinaries.resolve(platform));
}
if (_options.dillDependencies != null) {
dependencies.addAll(_options.dillDependencies);
}
Expand Down
23 changes: 20 additions & 3 deletions pkg/compiler/lib/src/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ library dart2js.src.options;
import 'package:front_end/src/api_unstable/dart2js.dart' as fe;

import 'commandline_options.dart' show Flags;
import 'util/util.dart';

/// Options used for controlling diagnostic messages.
abstract class DiagnosticOptions {
Expand Down Expand Up @@ -320,6 +321,11 @@ class CompilerOptions implements DiagnosticOptions {
/// Create an options object by parsing flags from [options].
static CompilerOptions parse(List<String> options,
{Uri librariesSpecificationUri, Uri platformBinaries}) {
Map<fe.ExperimentalFlag, bool> languageExperiments =
_extractExperiments(options);
if (equalMaps(languageExperiments, defaultExperimentalFlags)) {
platformBinaries ??= fe.computePlatformBinariesLocation();
}
return new CompilerOptions()
..librariesSpecificationUri = librariesSpecificationUri
..allowMockCompilation = _hasOption(options, Flags.allowMockCompilation)
Expand All @@ -338,7 +344,7 @@ class CompilerOptions implements DiagnosticOptions {
..suppressHints = _hasOption(options, Flags.suppressHints)
..shownPackageWarnings =
_extractOptionalCsvOption(options, Flags.showPackageWarnings)
..languageExperiments = _extractExperiments(options)
..languageExperiments = languageExperiments
..disableInlining = _hasOption(options, Flags.disableInlining)
..disableProgramSplit = _hasOption(options, Flags.disableProgramSplit)
..disableTypeInference = _hasOption(options, Flags.disableTypeInference)
Expand Down Expand Up @@ -413,7 +419,8 @@ class CompilerOptions implements DiagnosticOptions {
if (packageRoot != null && !packageRoot.path.endsWith("/")) {
throw new ArgumentError("[packageRoot] must end with a /");
}
if (platformBinaries == null) {
if (platformBinaries == null &&
equalMaps(languageExperiments, defaultExperimentalFlags)) {
throw new ArgumentError("Missing required ${Flags.platformBinaries}");
}
}
Expand Down Expand Up @@ -553,8 +560,18 @@ List<Uri> _extractUriListOption(List<String> options, String flag) {
Map<fe.ExperimentalFlag, bool> _extractExperiments(List<String> options) {
List<String> experiments =
_extractOptionalCsvOption(options, Flags.enableLanguageExperiments);
return fe.parseExperimentalFlags(
Map<fe.ExperimentalFlag, bool> flags = fe.parseExperimentalFlags(
experiments, (String error) => throw new ArgumentError(error));
for (fe.ExperimentalFlag flag in defaultExperimentalFlags.keys) {
flags[flag] ??= defaultExperimentalFlags[flag];
}
return flags;
}

const Map<fe.ExperimentalFlag, bool> defaultExperimentalFlags = {
fe.ExperimentalFlag.constantUpdate2018: false,
fe.ExperimentalFlag.controlFlowCollections: false,
fe.ExperimentalFlag.spreadCollections: false,
};

const String _UNDETERMINED_BUILD_ID = "build number could not be determined";
40 changes: 27 additions & 13 deletions pkg/compiler/lib/src/ssa/builder_kernel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4025,27 +4025,41 @@ class KernelSsaGraphBuilder extends ir.Visitor

ir.Expression closure = invocation.arguments.positional.single;
String problem = 'requires a static method or top-level method';

bool handleTarget(ir.Procedure procedure) {
ir.FunctionNode function = procedure.function;
if (function != null &&
function.requiredParameterCount ==
function.positionalParameters.length &&
function.namedParameters.isEmpty) {
push(new HForeignCode(
js.js.expressionTemplateYielding(
emitter.staticFunctionAccess(_elementMap.getMethod(procedure))),
abstractValueDomain.dynamicType,
<HInstruction>[],
nativeBehavior: NativeBehavior.PURE,
foreignFunction: _elementMap.getMethod(procedure)));
return true;
}
problem = 'does not handle a closure with optional parameters';
return false;
}

if (closure is ir.StaticGet) {
ir.Member staticTarget = closure.target;
if (staticTarget is ir.Procedure) {
if (staticTarget.kind == ir.ProcedureKind.Method) {
ir.FunctionNode function = staticTarget.function;
if (function != null &&
function.requiredParameterCount ==
function.positionalParameters.length &&
function.namedParameters.isEmpty) {
push(new HForeignCode(
js.js.expressionTemplateYielding(emitter
.staticFunctionAccess(_elementMap.getMethod(staticTarget))),
abstractValueDomain.dynamicType,
<HInstruction>[],
nativeBehavior: NativeBehavior.PURE,
foreignFunction: _elementMap.getMethod(staticTarget)));
if (handleTarget(staticTarget)) {
return;
}
problem = 'does not handle a closure with optional parameters';
}
}
} else if (closure is ir.ConstantExpression &&
closure.constant is ir.TearOffConstant) {
ir.TearOffConstant tearOff = closure.constant;
if (handleTarget(tearOff.procedure)) {
return;
}
}

reporter.reportErrorMessage(
Expand Down
10 changes: 10 additions & 0 deletions pkg/compiler/lib/src/util/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ bool equalSets<E>(Set<E> a, Set<E> b) {
return a.length == b.length && a.containsAll(b) && b.containsAll(a);
}

bool equalMaps<K, V>(Map<K, V> a, Map<K, V> b) {
if (identical(a, b)) return true;
if (a == null || b == null) return false;
if (a.length != b.length) return false;
for (K key in a.keys) {
if (a[key] != b[key]) return false;
}
return true;
}

/// File name prefix used to shorten the file name in stack traces printed by
/// [trace].
String stackTraceFilePrefix = null;
Expand Down
6 changes: 4 additions & 2 deletions pkg/front_end/lib/src/api_unstable/dart2js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ InitializedCompilerState initializeCompiler(
List<Uri> linkedDependencies,
Uri packagesFileUri,
{List<Uri> dependencies,
Map<ExperimentalFlag, bool> experimentalFlags}) {
Map<ExperimentalFlag, bool> experimentalFlags,
bool verify: false}) {
bool mapEqual(Map<ExperimentalFlag, bool> a, Map<ExperimentalFlag, bool> b) {
if (a == null || b == null) return a == b;
if (a.length != b.length) return false;
Expand Down Expand Up @@ -146,7 +147,8 @@ InitializedCompilerState initializeCompiler(
..linkedDependencies = linkedDependencies
..librariesSpecificationUri = librariesSpecificationUri
..packagesFileUri = packagesFileUri
..experimentalFlags = experimentalFlags;
..experimentalFlags = experimentalFlags
..verify = verify;

ProcessedOptions processedOpts = new ProcessedOptions(options: options);

Expand Down
4 changes: 2 additions & 2 deletions pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,15 @@ class ConstantsTransformer extends Transformer {
if (variable.initializer != null) {
variable.initializer =
tryEvaluateAndTransformWithContext(variable, variable.initializer)
..parent = node;
..parent = variable;
}
}
for (final VariableDeclaration variable in node.namedParameters) {
transformAnnotations(variable.annotations, variable);
if (variable.initializer != null) {
variable.initializer =
tryEvaluateAndTransformWithContext(variable, variable.initializer)
..parent = node;
..parent = variable;
}
}
if (node.body != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ class KernelConstructorBuilder extends KernelFunctionBuilder {

origin.constructor.isExternal = constructor.isExternal;
origin.constructor.function = constructor.function;
origin.constructor.function.parent = constructor.function;
origin.constructor.function.parent = origin.constructor;
origin.constructor.initializers = constructor.initializers;
setParents(origin.constructor.initializers, origin.constructor);
return 1;
Expand Down
20 changes: 15 additions & 5 deletions pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -711,11 +711,21 @@ class KernelTarget extends TargetImplementation {
field.initializer = new NullLiteral()..parent = field;
if (field.isFinal &&
(cls.constructors.isNotEmpty || cls.isMixinDeclaration)) {
builder.library.addProblem(
templateFinalFieldNotInitialized.withArguments(field.name.name),
field.fileOffset,
field.name.name.length,
field.fileUri);
String uri = '${field.enclosingLibrary.importUri}';
String file = field.fileUri.pathSegments.last;
if (uri == 'dart:html' ||
uri == 'dart:svg' ||
uri == 'dart:_native_typed_data' ||
uri == 'dart:_interceptors' && file == 'js_string.dart') {
// TODO(johnniwinther): Use external getters instead of final
// fields. See https://github.com/dart-lang/sdk/issues/33762
} else {
builder.library.addProblem(
templateFinalFieldNotInitialized.withArguments(field.name.name),
field.fileOffset,
field.name.name.length,
field.fileUri);
}
}
}
}
Expand Down
14 changes: 0 additions & 14 deletions pkg/front_end/lib/src/fasta/rewrite_severity.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,6 @@ import 'messages.dart' as msg;
Severity rewriteSeverity(
Severity severity, msg.Code<Object> code, Uri fileUri) {
if (severity != Severity.ignored) {
if (code == msg.codeFinalFieldNotInitialized) {
// TODO(johnniwinther): Use external getters instead of final fields.
// See https://github.com/dart-lang/sdk/issues/33762
for (String path in [
"/pkg/dev_compiler/tool/input_sdk/private/js_string.dart",
"/sdk/lib/html/dart2js/html_dart2js.dart",
"/sdk/lib/svg/dart2js/svg_dart2js.dart",
"/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart"
]) {
if (fileUri.path.endsWith(path)) {
return Severity.ignored;
}
}
}
return severity;
}

Expand Down
13 changes: 9 additions & 4 deletions tests/compiler/dart2js/end_to_end/no_platform_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import 'package:front_end/src/api_prototype/standard_file_system.dart' as fe;
import 'package:front_end/src/api_unstable/dart2js.dart' as fe;

main() {
asyncTest(() async {
runTest(Map<fe.ExperimentalFlag, bool> experimentalFlags) async {
fe.InitializedCompilerState initializedCompilerState =
fe.initializeCompiler(
null,
Expand All @@ -21,9 +21,8 @@ main() {
.resolve('sdk/lib/libraries.json'), // librariesSpecificationUri
[], // linkedDependencies
Uri.base.resolve('.packages'), // packagesFileUri
experimentalFlags: const {
fe.ExperimentalFlag.constantUpdate2018: true
});
experimentalFlags: experimentalFlags,
verify: true);
ir.Component component = await fe.compile(
initializedCompilerState, false, fe.StandardFileSystem.instance,
(fe.DiagnosticMessage message) {
Expand All @@ -33,5 +32,11 @@ main() {
Uri.base.resolve(
'tests/compiler/dart2js/end_to_end/data/hello_world.dart'));
Expect.isNotNull(new ir.CoreTypes(component).futureClass);
}

asyncTest(() async {
await runTest(const {});
await runTest(const {fe.ExperimentalFlag.constantUpdate2018: true});
await runTest(const {fe.ExperimentalFlag.spreadCollections: true});
});
}
6 changes: 1 addition & 5 deletions tests/compiler/dart2js/helpers/memory_compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import 'package:compiler/src/null_compiler_output.dart' show NullCompilerOutput;
import 'package:compiler/src/options.dart' show CompilerOptions;

import 'package:front_end/src/api_unstable/dart2js.dart' as fe;
import 'package:front_end/src/compute_platform_binaries_location.dart'
show computePlatformBinariesLocation;

import 'memory_source_file_helper.dart';

Expand Down Expand Up @@ -111,7 +109,6 @@ CompilerImpl compilerFor(
Uri packageConfig}) {
retainDataForTesting = true;
librariesSpecificationUri ??= Uri.base.resolve('sdk/lib/libraries.json');
Uri platformBinaries = computePlatformBinariesLocation();

if (packageRoot == null && packageConfig == null) {
if (Platform.packageConfig != null) {
Expand All @@ -134,8 +131,7 @@ CompilerImpl compilerFor(
}

CompilerOptions compilerOptions = CompilerOptions.parse(options,
librariesSpecificationUri: librariesSpecificationUri,
platformBinaries: platformBinaries)
librariesSpecificationUri: librariesSpecificationUri)
..entryPoint = entryPoint
..packageRoot = packageRoot
..environment = {}
Expand Down

0 comments on commit a8e51c5

Please sign in to comment.