From a8e51c58681342daf39a02beba2221e8c6ddae32 Mon Sep 17 00:00:00 2001 From: Johnni Winther Date: Tue, 26 Mar 2019 13:18:24 +0000 Subject: [PATCH] Bypass use of dart2js_platform.dill when experimental flags are non-default Change-Id: I95e654c6c2a430f7aebb30ced92b33cd35aadca3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97634 Reviewed-by: Aske Simon Christensen Reviewed-by: Sigmund Cherem Commit-Queue: Johnni Winther --- pkg/compiler/lib/src/dart2js.dart | 2 +- pkg/compiler/lib/src/kernel/loader.dart | 9 ++++- pkg/compiler/lib/src/options.dart | 23 +++++++++-- pkg/compiler/lib/src/ssa/builder_kernel.dart | 40 +++++++++++++------ pkg/compiler/lib/src/util/util.dart | 10 +++++ .../lib/src/api_unstable/dart2js.dart | 6 ++- .../src/fasta/kernel/constant_evaluator.dart | 4 +- .../kernel/kernel_procedure_builder.dart | 2 +- .../lib/src/fasta/kernel/kernel_target.dart | 20 +++++++--- .../lib/src/fasta/rewrite_severity.dart | 14 ------- .../dart2js/end_to_end/no_platform_test.dart | 13 ++++-- .../dart2js/helpers/memory_compiler.dart | 6 +-- 12 files changed, 97 insertions(+), 52 deletions(-) diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart index 3d64064a4a78c..85abe2ac8ec18 100644 --- a/pkg/compiler/lib/src/dart2js.dart +++ b/pkg/compiler/lib/src/dart2js.dart @@ -127,7 +127,7 @@ Future compile(List argv, bool showHints; bool enableColors; int optimizationLevel = null; - Uri platformBinaries = fe.computePlatformBinariesLocation(); + Uri platformBinaries; Map environment = new Map(); CompilationStrategy compilationStrategy = CompilationStrategy.direct; diff --git a/pkg/compiler/lib/src/kernel/loader.dart b/pkg/compiler/lib/src/kernel/loader.dart index f73fba4ce5e79..bfdf6ee5bfebb 100644 --- a/pkg/compiler/lib/src/kernel/loader.dart +++ b/pkg/compiler/lib/src/kernel/loader.dart @@ -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); } @@ -90,7 +92,10 @@ class KernelLoaderTask extends CompilerTask { return null; } } else { - List dependencies = [_options.platformBinaries.resolve(platform)]; + List dependencies = []; + if (_options.platformBinaries != null) { + dependencies.add(_options.platformBinaries.resolve(platform)); + } if (_options.dillDependencies != null) { dependencies.addAll(_options.dillDependencies); } diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart index 8a6e462553be1..009ea93410f83 100644 --- a/pkg/compiler/lib/src/options.dart +++ b/pkg/compiler/lib/src/options.dart @@ -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 { @@ -320,6 +321,11 @@ class CompilerOptions implements DiagnosticOptions { /// Create an options object by parsing flags from [options]. static CompilerOptions parse(List options, {Uri librariesSpecificationUri, Uri platformBinaries}) { + Map languageExperiments = + _extractExperiments(options); + if (equalMaps(languageExperiments, defaultExperimentalFlags)) { + platformBinaries ??= fe.computePlatformBinariesLocation(); + } return new CompilerOptions() ..librariesSpecificationUri = librariesSpecificationUri ..allowMockCompilation = _hasOption(options, Flags.allowMockCompilation) @@ -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) @@ -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}"); } } @@ -553,8 +560,18 @@ List _extractUriListOption(List options, String flag) { Map _extractExperiments(List options) { List experiments = _extractOptionalCsvOption(options, Flags.enableLanguageExperiments); - return fe.parseExperimentalFlags( + Map 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 defaultExperimentalFlags = { + fe.ExperimentalFlag.constantUpdate2018: false, + fe.ExperimentalFlag.controlFlowCollections: false, + fe.ExperimentalFlag.spreadCollections: false, +}; + const String _UNDETERMINED_BUILD_ID = "build number could not be determined"; diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart index 3d0c9588fa6b4..884240fc426d7 100644 --- a/pkg/compiler/lib/src/ssa/builder_kernel.dart +++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart @@ -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, + [], + 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, - [], - 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( diff --git a/pkg/compiler/lib/src/util/util.dart b/pkg/compiler/lib/src/util/util.dart index 3dfee4b37c3ad..92be403258569 100644 --- a/pkg/compiler/lib/src/util/util.dart +++ b/pkg/compiler/lib/src/util/util.dart @@ -114,6 +114,16 @@ bool equalSets(Set a, Set b) { return a.length == b.length && a.containsAll(b) && b.containsAll(a); } +bool equalMaps(Map a, Map 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; diff --git a/pkg/front_end/lib/src/api_unstable/dart2js.dart b/pkg/front_end/lib/src/api_unstable/dart2js.dart index 37129d018d5c4..e397849cddc75 100644 --- a/pkg/front_end/lib/src/api_unstable/dart2js.dart +++ b/pkg/front_end/lib/src/api_unstable/dart2js.dart @@ -112,7 +112,8 @@ InitializedCompilerState initializeCompiler( List linkedDependencies, Uri packagesFileUri, {List dependencies, - Map experimentalFlags}) { + Map experimentalFlags, + bool verify: false}) { bool mapEqual(Map a, Map b) { if (a == null || b == null) return a == b; if (a.length != b.length) return false; @@ -146,7 +147,8 @@ InitializedCompilerState initializeCompiler( ..linkedDependencies = linkedDependencies ..librariesSpecificationUri = librariesSpecificationUri ..packagesFileUri = packagesFileUri - ..experimentalFlags = experimentalFlags; + ..experimentalFlags = experimentalFlags + ..verify = verify; ProcessedOptions processedOpts = new ProcessedOptions(options: options); diff --git a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart index 600ed94e5dd0a..79aa7f510e8eb 100644 --- a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart +++ b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart @@ -236,7 +236,7 @@ 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) { @@ -244,7 +244,7 @@ class ConstantsTransformer extends Transformer { if (variable.initializer != null) { variable.initializer = tryEvaluateAndTransformWithContext(variable, variable.initializer) - ..parent = node; + ..parent = variable; } } if (node.body != null) { diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart index ae0e19743f6ef..2434d096e66a5 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_procedure_builder.dart @@ -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; diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart index a8166dbbd115e..126123c7d1126 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart @@ -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); + } } } } diff --git a/pkg/front_end/lib/src/fasta/rewrite_severity.dart b/pkg/front_end/lib/src/fasta/rewrite_severity.dart index 5d521bbee6be4..40f5b8c1fa4d8 100644 --- a/pkg/front_end/lib/src/fasta/rewrite_severity.dart +++ b/pkg/front_end/lib/src/fasta/rewrite_severity.dart @@ -9,20 +9,6 @@ import 'messages.dart' as msg; Severity rewriteSeverity( Severity severity, msg.Code 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; } diff --git a/tests/compiler/dart2js/end_to_end/no_platform_test.dart b/tests/compiler/dart2js/end_to_end/no_platform_test.dart index 5a562a8442150..c6c11da4b1ef2 100644 --- a/tests/compiler/dart2js/end_to_end/no_platform_test.dart +++ b/tests/compiler/dart2js/end_to_end/no_platform_test.dart @@ -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 experimentalFlags) async { fe.InitializedCompilerState initializedCompilerState = fe.initializeCompiler( null, @@ -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) { @@ -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}); }); } diff --git a/tests/compiler/dart2js/helpers/memory_compiler.dart b/tests/compiler/dart2js/helpers/memory_compiler.dart index ce4eda754724b..4f7b01f84d753 100644 --- a/tests/compiler/dart2js/helpers/memory_compiler.dart +++ b/tests/compiler/dart2js/helpers/memory_compiler.dart @@ -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'; @@ -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) { @@ -134,8 +131,7 @@ CompilerImpl compilerFor( } CompilerOptions compilerOptions = CompilerOptions.parse(options, - librariesSpecificationUri: librariesSpecificationUri, - platformBinaries: platformBinaries) + librariesSpecificationUri: librariesSpecificationUri) ..entryPoint = entryPoint ..packageRoot = packageRoot ..environment = {}