Skip to content

Commit

Permalink
[dartdevc] Reduce the number of SDK libraries in the platform
Browse files Browse the repository at this point in the history
We are temporarily building a smaller SDK platform to make progress building
applications against the forked sdk_nnbd sources. This is because some of the
libraries have not yet been ported and they don't compose well.

I needed to thread the status of the non-nullable experiment through the front
end entry point and into the target options, and native types.

Also includes some standardization of the flag name in DDC.

#39698

Change-Id: I4bdd503be694b28d7f95828d4af28d1d5fb3691f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127486
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
nshahan authored and commit-bot@chromium.org committed Dec 9, 2019
1 parent b296d55 commit 7974f3f
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 91 deletions.
2 changes: 1 addition & 1 deletion pkg/dev_compiler/lib/src/analyzer/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ class CodeGenerator extends Object
// * There isn't an obvious place in dart:_runtime were we could place a
// method that adds these type tests (similar to addTypeTests()) because
// in the bootstrap ordering the Future class hasn't been defined yet.
if (options.nonNullableEnabled) {
if (options.enableNullSafety) {
// TODO(nshahan) Update FutureOr type tests for NNBD.
var typeParamT =
getLegacyTypeParameterType(classElem.typeParameters[0]);
Expand Down
2 changes: 1 addition & 1 deletion pkg/dev_compiler/lib/src/compiler/shared_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class SharedCompilerOptions {
}

// TODO(nshahan) Cleanup when NNBD graduates experimental status.
bool get nonNullableEnabled => experiments['non-nullable'] ?? false;
bool get enableNullSafety => experiments['non-nullable'] ?? false;
}

/// Finds explicit module names of the form `path=name` in [summaryPaths],
Expand Down
10 changes: 6 additions & 4 deletions pkg/dev_compiler/lib/src/kernel/command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ Future<CompilerResult> _compile(List<String> args,
sourcePathToUri(packageFile),
sourcePathToUri(librarySpecPath),
inputSummaries,
DevCompilerTarget(
TargetFlags(trackWidgetCreation: trackWidgetCreation)),
DevCompilerTarget(TargetFlags(
trackWidgetCreation: trackWidgetCreation,
enableNullSafety: options.enableNullSafety)),
fileSystem: fileSystem,
experiments: experiments,
environmentDefines: declaredVariables);
Expand Down Expand Up @@ -302,8 +303,9 @@ Future<CompilerResult> _compile(List<String> args,
sourcePathToUri(librarySpecPath),
inputSummaries,
inputDigests,
DevCompilerTarget(
TargetFlags(trackWidgetCreation: trackWidgetCreation)),
DevCompilerTarget(TargetFlags(
trackWidgetCreation: trackWidgetCreation,
enableNullSafety: options.enableNullSafety)),
fileSystem: fileSystem,
experiments: experiments,
environmentDefines: declaredVariables,
Expand Down
5 changes: 3 additions & 2 deletions pkg/dev_compiler/lib/src/kernel/compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
coreTypes ??= CoreTypes(component);
var types = TypeEnvironment(coreTypes, hierarchy);
var constants = DevCompilerConstants();
var nativeTypes = NativeTypeSet(coreTypes, constants);
var nativeTypes = NativeTypeSet(coreTypes, constants,
enableNullSafety: options.enableNullSafety);
var jsTypeRep = JSTypeRep(types, hierarchy);
var staticTypeContext = StatefulStaticTypeContext.stacked(types);
return ProgramCompiler._(
Expand Down Expand Up @@ -1003,7 +1004,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
// * There isn't an obvious place in dart:_runtime were we could place a
// method that adds these type tests (similar to addTypeTests()) because
// in the bootstrap ordering the Future class hasn't been defined yet.
if (_options.nonNullableEnabled) {
if (_options.enableNullSafety) {
// TODO(nshahan) Update FutureOr type tests for NNBD
var typeParam =
TypeParameterType(c.typeParameters[0], Nullability.legacy);
Expand Down
19 changes: 12 additions & 7 deletions pkg/dev_compiler/lib/src/kernel/native_types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class NativeTypeSet {
final _nativeTypes = HashSet<Class>.identity();
final _pendingLibraries = HashSet<Library>.identity();

NativeTypeSet(this.coreTypes, this.constants) {
NativeTypeSet(this.coreTypes, this.constants,
{bool enableNullSafety = false}) {
// First, core types:
// TODO(vsm): If we're analyzing against the main SDK, those
// types are not explicitly annotated.
Expand All @@ -61,12 +62,16 @@ class NativeTypeSet {
// listed below).

// Second, html types - these are only searched if we use dart:html, etc.:
_addPendingExtensionTypes(sdk.getLibrary('dart:html'));
_addPendingExtensionTypes(sdk.getLibrary('dart:indexed_db'));
_addPendingExtensionTypes(sdk.getLibrary('dart:svg'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_audio'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_gl'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_sql'));
// TODO(39698) Remove this check and the named parameter once we don't have
// to exclude libraries from the forked NNBD sdk.
if (!enableNullSafety) {
_addPendingExtensionTypes(sdk.getLibrary('dart:html'));
_addPendingExtensionTypes(sdk.getLibrary('dart:indexed_db'));
_addPendingExtensionTypes(sdk.getLibrary('dart:svg'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_audio'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_gl'));
_addPendingExtensionTypes(sdk.getLibrary('dart:web_sql'));
}
}

void _addExtensionType(Class c, [bool mustBeNative = false]) {
Expand Down
104 changes: 58 additions & 46 deletions pkg/dev_compiler/lib/src/kernel/target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,62 @@ import 'kernel_helpers.dart';

/// A kernel [Target] to configure the Dart Front End for dartdevc.
class DevCompilerTarget extends Target {
DevCompilerTarget(this.flags);
// TODO(39698) Turn these back into const lists returned from the getters
// once we don't have to exclude libraries from the forked NNBD sdk.
List<String> _extraRequiredLibraries;
List<String> _extraIndexedLibraries;

DevCompilerTarget(this.flags)
: _extraRequiredLibraries = [
'dart:_runtime',
'dart:_debugger',
'dart:_foreign_helper',
'dart:_interceptors',
'dart:_internal',
'dart:_isolate_helper',
'dart:_js_helper',
'dart:_js_mirrors',
'dart:_js_primitives',
'dart:_metadata',
'dart:_native_typed_data',
'dart:async',
'dart:collection',
'dart:convert',
if (!flags.enableNullSafety) ...[
'dart:developer',
'dart:io',
'dart:isolate'
],
'dart:js',
'dart:js_util',
'dart:math',
'dart:mirrors',
'dart:typed_data',
if (!flags.enableNullSafety) ...[
'dart:indexed_db',
'dart:html',
'dart:html_common',
'dart:svg',
'dart:web_audio',
'dart:web_gl',
'dart:web_sql'
]
],
_extraIndexedLibraries = [
'dart:async',
'dart:collection',
if (!flags.enableNullSafety) ...['dart:html', 'dart:indexed_db'],
'dart:math',
if (!flags.enableNullSafety) ...[
'dart:svg',
'dart:web_audio',
'dart:web_gl',
'dart:web_sql'
],
'dart:_interceptors',
'dart:_js_helper',
'dart:_native_typed_data',
];

final TargetFlags flags;

Expand All @@ -30,54 +85,11 @@ class DevCompilerTarget extends Target {
String get name => 'dartdevc';

@override
List<String> get extraRequiredLibraries => const [
'dart:_runtime',
'dart:_debugger',
'dart:_foreign_helper',
'dart:_interceptors',
'dart:_internal',
'dart:_isolate_helper',
'dart:_js_helper',
'dart:_js_mirrors',
'dart:_js_primitives',
'dart:_metadata',
'dart:_native_typed_data',
'dart:async',
'dart:collection',
'dart:convert',
'dart:developer',
'dart:io',
'dart:isolate',
'dart:js',
'dart:js_util',
'dart:math',
'dart:mirrors',
'dart:typed_data',
'dart:indexed_db',
'dart:html',
'dart:html_common',
'dart:svg',
'dart:web_audio',
'dart:web_gl',
'dart:web_sql'
];
List<String> get extraRequiredLibraries => _extraRequiredLibraries;

// The libraries required to be indexed via CoreTypes.
@override
List<String> get extraIndexedLibraries => const [
'dart:async',
'dart:collection',
'dart:html',
'dart:indexed_db',
'dart:math',
'dart:svg',
'dart:web_audio',
'dart:web_gl',
'dart:web_sql',
'dart:_interceptors',
'dart:_js_helper',
'dart:_native_typed_data',
];
List<String> get extraIndexedLibraries => _extraIndexedLibraries;

@override
bool mayDefineRestrictedType(Uri uri) =>
Expand Down
15 changes: 9 additions & 6 deletions pkg/front_end/tool/_fasta/command_line.dart
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,16 @@ ProcessedOptions analyzeCommandLine(

final String targetName = options["--target"] ?? "vm";

Map<ExperimentalFlag, bool> experimentalFlags = parseExperimentalFlags(
parseExperimentalArguments(options["--enable-experiment"]),
onError: throwCommandLineProblem,
onWarning: print);

final TargetFlags flags = new TargetFlags(
forceLateLoweringForTesting: options["--force-late-lowering"]);
forceLateLoweringForTesting: options["--force-late-lowering"],
enableNullSafety:
experimentalFlags.containsKey(ExperimentalFlag.nonNullable) &&
experimentalFlags[ExperimentalFlag.nonNullable]);

final Target target = getTarget(targetName, flags);
if (target == null) {
Expand Down Expand Up @@ -356,11 +364,6 @@ ProcessedOptions analyzeCommandLine(
});
}

Map<ExperimentalFlag, bool> experimentalFlags = parseExperimentalFlags(
parseExperimentalArguments(options["--enable-experiment"]),
onError: throwCommandLineProblem,
onWarning: print);

if (programName == "compile_platform") {
if (arguments.length != 5) {
return throw new CommandLineProblem.deprecated(
Expand Down
4 changes: 3 additions & 1 deletion pkg/kernel/lib/target/targets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ final List<String> targetNames = targets.keys.toList();
class TargetFlags {
final bool trackWidgetCreation;
final bool forceLateLoweringForTesting;
final bool enableNullSafety;

TargetFlags(
{this.trackWidgetCreation = false,
this.forceLateLoweringForTesting = false});
this.forceLateLoweringForTesting = false,
this.enableNullSafety = false});
}

typedef Target _TargetBuilder(TargetFlags flags);
Expand Down
41 changes: 18 additions & 23 deletions utils/dartdevc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,25 @@ create_timestamp_file("dartdevc_sdk_patch_stamp") {
group("dartdevc_test_kernel_pkg") {
deps = [
":async_helper_js",
":collection_js",
":expect_js",
":js_js",
":matcher_js",
":meta_js",
":path_js",
":stack_trace_js",
":unittest_js",
]

# TODO(38701): Cleanup after merging the forked SDK into mainline.
if (!use_nnbd) {
# TODO(sigmund): Merge this list into above. We turned this off temporarily
# while the migration of libraries is in flux, this step otherwise fails
# with many nnbd-related compile-time errors because the packages are
# assumed to be nnbd compilant.
deps += [
":collection_js",
":matcher_js",
":path_js",
":stack_trace_js",
":unittest_js",
]
}
}

template("dartdevc_kernel_compile") {
Expand All @@ -232,10 +242,6 @@ template("dartdevc_kernel_compile") {
# * package_dependencies: the name of other packages this package depends
# on. When providing `name`, a separate `dartdevc_kernel_compile` target
# named `${name}_js` must exist.
# * nnbd_disabled: whether to disable the non-nullable experiment under the
# NNBD build. This is used temporariy until we either migrate the packages
# or add support to read the language version information from the package
# itself.
# * args: additional args to pass to dartdevc

prebuilt_dart_action(target_name) {
Expand Down Expand Up @@ -288,15 +294,9 @@ template("dartdevc_kernel_compile") {
args += invoker.args
}

# TODO(sigmund): remove nnbd_disabled. We turned this off temporarily while
# the migration of libraries is in flux, this step otherwise fails with many
# nnbd-related compile-time errors because the packages are assumed to be
# nnbd compilant.
if (!defined(invoker.nnbd_disabled) || !invoker.nnbd_disabled) {
# TODO(38701): Cleanup after merging the forked SDK into mainline.
if (use_nnbd) {
args += [ "--enable-experiment=non-nullable" ]
}
# TODO(38701): Cleanup after merging the forked SDK into mainline.
if (use_nnbd) {
args += [ "--enable-experiment=non-nullable" ]
}
}
}
Expand All @@ -307,7 +307,6 @@ dartdevc_kernel_compile("async_helper_js") {

dartdevc_kernel_compile("collection_js") {
package = "collection"
nnbd_disabled = true
}

dartdevc_kernel_compile("expect_js") {
Expand All @@ -323,7 +322,6 @@ dartdevc_kernel_compile("js_js") {
dartdevc_kernel_compile("matcher_js") {
package = "matcher"
package_dependencies = [ "stack_trace" ]
nnbd_disabled = true
}

dartdevc_kernel_compile("meta_js") {
Expand All @@ -332,13 +330,11 @@ dartdevc_kernel_compile("meta_js") {

dartdevc_kernel_compile("path_js") {
package = "path"
nnbd_disabled = true
}

dartdevc_kernel_compile("stack_trace_js") {
package = "stack_trace"
package_dependencies = [ "path" ]
nnbd_disabled = true
}

# TODO(rnystrom): Remove this when unittest is no longer used. Also remove
Expand All @@ -354,7 +350,6 @@ dartdevc_kernel_compile("unittest_js") {
"html_individual_config",
"html_enhanced_config",
]
nnbd_disabled = true
}

compile_platform("dartdevc_platform") {
Expand Down

0 comments on commit 7974f3f

Please sign in to comment.