Skip to content

Commit d56b9b3

Browse files
nshahanCommit Queue
authored and
Commit Queue
committed
[ddc] Remove unsound .dill files from SDK
Dart 3 requires all code to be null safe. Compiling with --no-sound-null safety requires manually passing an unsound SDK outline. This is no longer provided by default and is not packaged with the SDK at all. Fixes: #50700 Change-Id: Ic4bd7dc5e96b42c7c10a29f273371c6a680936f1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278999 Reviewed-by: William Hesse <[email protected]> Commit-Queue: Nicholas Shahan <[email protected]>
1 parent f48a1fb commit d56b9b3

File tree

10 files changed

+122
-73
lines changed

10 files changed

+122
-73
lines changed

pkg/dev_compiler/lib/src/kernel/command.dart

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,27 @@ Future<CompilerResult> _compile(List<String> args,
140140

141141
var options = SharedCompilerOptions.fromArguments(argResults);
142142

143+
Uri toCustomUri(Uri uri) {
144+
if (!uri.hasScheme) {
145+
return Uri(scheme: options.multiRootScheme, path: '/${uri.path}');
146+
}
147+
return uri;
148+
}
149+
150+
// TODO(jmesserly): this is a workaround for the CFE, which does not
151+
// understand relative URIs, and we'd like to avoid absolute file URIs
152+
// being placed in the summary if possible.
153+
// TODO(jmesserly): investigate if Analyzer has a similar issue.
154+
Uri sourcePathToCustomUri(String source) {
155+
return toCustomUri(sourcePathToRelativeUri(source));
156+
}
157+
158+
// Compile SDK module directly from a provided .dill file.
159+
var inputs = [for (var arg in argResults.rest) sourcePathToCustomUri(arg)];
160+
if (inputs.length == 1 && inputs.single.path.endsWith('.dill')) {
161+
return compileSdkFromDill(args);
162+
}
163+
143164
// To make the output .dill agnostic of the current working directory,
144165
// we use a custom-uri scheme for all app URIs (these are files outside the
145166
// lib folder). The following [FileSystem] will resolve those references to
@@ -162,44 +183,36 @@ Future<CompilerResult> _compile(List<String> args,
162183

163184
var fileSystem = MultiRootFileSystem(
164185
options.multiRootScheme, multiRootPaths, fe.StandardFileSystem.instance);
165-
166-
Uri toCustomUri(Uri uri) {
167-
if (!uri.hasScheme) {
168-
return Uri(scheme: options.multiRootScheme, path: '/${uri.path}');
169-
}
170-
return uri;
171-
}
172-
173-
// TODO(jmesserly): this is a workaround for the CFE, which does not
174-
// understand relative URIs, and we'd like to avoid absolute file URIs
175-
// being placed in the summary if possible.
176-
// TODO(jmesserly): investigate if Analyzer has a similar issue.
177-
Uri sourcePathToCustomUri(String source) {
178-
return toCustomUri(sourcePathToRelativeUri(source));
179-
}
180-
181186
var summaryPaths = options.summaryModules.keys.toList();
182187
var summaryModules = Map.fromIterables(
183188
summaryPaths.map(sourcePathToUri).cast<Uri>(),
184189
options.summaryModules.values);
185190
var sdkSummaryPath = argResults['dart-sdk-summary'] as String?;
186191
var librarySpecPath = argResults['libraries-file'] as String?;
192+
var compileSdk = argResults['compile-sdk'] == true;
187193
if (sdkSummaryPath == null) {
188-
sdkSummaryPath =
189-
defaultSdkSummaryPath(soundNullSafety: options.soundNullSafety);
190-
librarySpecPath ??= defaultLibrarySpecPath;
194+
if (!compileSdk) {
195+
if (!options.soundNullSafety) {
196+
// Technically if you can produce an SDK outline .dill and pass it
197+
// this error can be avoided and the compile will still work for now.
198+
// If you are reading this comment be warned, this loophole will be
199+
// removed without warning in the future.
200+
print('Dart 3 only supports sound null safety, '
201+
'see https://dart.dev/null-safety.\n');
202+
return CompilerResult(64);
203+
}
204+
sdkSummaryPath = defaultSdkSummaryPath;
205+
librarySpecPath ??= defaultLibrarySpecPath;
206+
}
207+
// Compiling without manually passing or getting a default SDK summary is
208+
// only allowed when `compileSdk` is true.
191209
}
192210
var invalidSummary = summaryPaths.any((s) => !s.endsWith('.dill')) ||
193-
!sdkSummaryPath.endsWith('.dill');
211+
(sdkSummaryPath != null && !sdkSummaryPath.endsWith('.dill'));
194212
if (invalidSummary) {
195213
throw StateError('Non-dill file detected in input: $summaryPaths');
196214
}
197215

198-
var inputs = [for (var arg in argResults.rest) sourcePathToCustomUri(arg)];
199-
if (inputs.length == 1 && inputs.single.path.endsWith('.dill')) {
200-
return compileSdkFromDill(args);
201-
}
202-
203216
if (librarySpecPath == null) {
204217
// TODO(jmesserly): the `isSupported` bit should be included in the SDK
205218
// summary, but front_end requires a separate file, so we have to work
@@ -210,10 +223,12 @@ Future<CompilerResult> _compile(List<String> args,
210223
// (if the user is doing something custom).
211224
//
212225
// Another option: we could make an in-memory file with the relevant info.
213-
librarySpecPath =
214-
p.join(p.dirname(p.dirname(sdkSummaryPath)), 'libraries.json');
226+
librarySpecPath = p.join(
227+
p.dirname(p.dirname(sdkSummaryPath ?? defaultSdkSummaryPath)),
228+
'libraries.json');
215229
if (!File(librarySpecPath).existsSync()) {
216-
librarySpecPath = p.join(p.dirname(sdkSummaryPath), 'libraries.json');
230+
librarySpecPath = p.join(
231+
p.dirname(sdkSummaryPath ?? defaultSdkSummaryPath), 'libraries.json');
217232
}
218233
}
219234

@@ -245,8 +260,6 @@ Future<CompilerResult> _compile(List<String> args,
245260
onError: stderr.writeln, onWarning: print);
246261

247262
var trackWidgetCreation = argResults['track-widget-creation'] as bool;
248-
249-
var compileSdk = argResults['compile-sdk'] == true;
250263
var oldCompilerState = compilerState;
251264
var recordUsedInputs = argResults['used-inputs-file'] != null;
252265
var additionalDills = summaryModules.keys.toList();
@@ -262,7 +275,7 @@ Future<CompilerResult> _compile(List<String> args,
262275
oldCompilerState,
263276
compileSdk,
264277
sourcePathToUri(getSdkPath()),
265-
compileSdk ? null : sourcePathToUri(sdkSummaryPath),
278+
compileSdk ? null : sourcePathToUri(sdkSummaryPath!),
266279
packageFile != null ? sourcePathToUri(packageFile) : null,
267280
sourcePathToUri(librarySpecPath),
268281
additionalDills,
@@ -284,7 +297,7 @@ Future<CompilerResult> _compile(List<String> args,
284297
oldCompilerState = null;
285298

286299
if (!compileSdk) {
287-
inputDigests[sourcePathToUri(sdkSummaryPath)] = const [0];
300+
inputDigests[sourcePathToUri(sdkSummaryPath!)] = const [0];
288301
}
289302
for (var uri in summaryModules.keys) {
290303
inputDigests[uri] = const [0];
@@ -302,7 +315,7 @@ Future<CompilerResult> _compile(List<String> args,
302315
doneAdditionalDills,
303316
compileSdk,
304317
sourcePathToUri(getSdkPath()),
305-
compileSdk ? null : sourcePathToUri(sdkSummaryPath),
318+
compileSdk ? null : sourcePathToUri(sdkSummaryPath!),
306319
packageFile != null ? sourcePathToUri(packageFile) : null,
307320
sourcePathToUri(librarySpecPath),
308321
additionalDills,
@@ -317,8 +330,9 @@ Future<CompilerResult> _compile(List<String> args,
317330
nnbdMode:
318331
options.soundNullSafety ? fe.NnbdMode.Strong : fe.NnbdMode.Weak);
319332
var incrementalCompiler = compilerState.incrementalCompiler!;
320-
var cachedSdkInput =
321-
compilerState.workerInputCache![sourcePathToUri(sdkSummaryPath)];
333+
var cachedSdkInput = compileSdk
334+
? null
335+
: compilerState.workerInputCache![sourcePathToUri(sdkSummaryPath!)];
322336
compilerState.options.onDiagnostic = diagnosticMessageHandler;
323337
var incrementalCompilerResult = await incrementalCompiler.computeDelta(
324338
entryPoints: inputs,
@@ -819,13 +833,9 @@ Map<String, String> parseAndRemoveDeclaredVariables(List<String> args) {
819833
return declaredVariables;
820834
}
821835

822-
/// The default path of the kernel summary for the Dart SDK given the
823-
/// [soundNullSafety] mode.
824-
String defaultSdkSummaryPath({required bool soundNullSafety}) {
825-
var outlineDill =
826-
soundNullSafety ? 'ddc_outline.dill' : 'ddc_outline_unsound.dill';
827-
return p.join(getSdkPath(), 'lib', '_internal', outlineDill);
828-
}
836+
/// The default path of the kernel summary for the Dart SDK.
837+
final defaultSdkSummaryPath =
838+
p.join(getSdkPath(), 'lib', '_internal', 'ddc_outline.dill');
829839

830840
final defaultLibrarySpecPath = p.join(getSdkPath(), 'lib', 'libraries.json');
831841

pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_suite.dart

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,15 @@ class DevelopmentIncrementalCompiler extends fe.IncrementalCompiler {
5454

5555
class SetupCompilerOptions {
5656
static final sdkRoot = fe.computePlatformBinariesLocation();
57+
static final buildRoot =
58+
fe.computePlatformBinariesLocation(forceBuildDir: true);
59+
// Unsound .dill files are not longer in the released SDK so this file must be
60+
// read from the build output directory.
5761
static final sdkUnsoundSummaryPath =
58-
p.join(sdkRoot.toFilePath(), 'ddc_outline_unsound.dill');
62+
buildRoot.resolve('ddc_outline_unsound.dill').toFilePath();
63+
// Use the outline copied to the released SDK.
5964
static final sdkSoundSummaryPath =
60-
p.join(sdkRoot.toFilePath(), 'ddc_outline.dill');
65+
sdkRoot.resolve('ddc_outline.dill').toFilePath();
6166
static final librariesSpecificationUri =
6267
p.join(p.dirname(p.dirname(getSdkPath())), 'libraries.json');
6368

@@ -301,7 +306,6 @@ class TestDriver {
301306
this.setup = setup;
302307
this.source = source;
303308
testDir = chromeDir.createTempSync('ddc_eval_test');
304-
var buildDir = p.dirname(p.dirname(p.dirname(Platform.resolvedExecutable)));
305309
var scriptPath = Platform.script.normalizePath().toFilePath();
306310
var ddcPath = p.dirname(p.dirname(p.dirname(scriptPath)));
307311
output = testDir.uri.resolve('test.js');
@@ -340,14 +344,15 @@ class TestDriver {
340344

341345
switch (setup.moduleFormat) {
342346
case ModuleFormat.ddc:
343-
var dartSdkPath = escaped(p.join(
344-
buildDir,
345-
'gen',
346-
'utils',
347-
'dartdevc',
348-
setup.soundNullSafety ? 'sound' : 'kernel',
349-
'legacy',
350-
'dart_sdk.js'));
347+
var dartSdkPath = escaped(SetupCompilerOptions.buildRoot
348+
.resolve(p.join(
349+
'gen',
350+
'utils',
351+
'dartdevc',
352+
setup.soundNullSafety ? 'sound' : 'kernel',
353+
'legacy',
354+
'dart_sdk.js'))
355+
.toFilePath());
351356
if (!File(dartSdkPath).existsSync()) {
352357
throw Exception('Unable to find Dart SDK at $dartSdkPath');
353358
}
@@ -375,13 +380,17 @@ class TestDriver {
375380
''');
376381
break;
377382
case ModuleFormat.amd:
378-
var dartSdkPath = escaped(p.join(buildDir, 'gen', 'utils', 'dartdevc',
379-
setup.soundNullSafety ? 'sound' : 'kernel', 'amd', 'dart_sdk'));
383+
var dartSdkPath = escaped(SetupCompilerOptions.buildRoot
384+
.resolve(p.join('gen', 'utils', 'dartdevc',
385+
setup.soundNullSafety ? 'sound' : 'kernel', 'amd', 'dart_sdk'))
386+
.toFilePath());
380387
if (!File('$dartSdkPath.js').existsSync()) {
381388
throw Exception('Unable to find Dart SDK at $dartSdkPath.js');
382389
}
383-
var requirePath = escaped(p.join(
384-
buildDir, 'dart-sdk', 'lib', 'dev_compiler', 'amd', 'require.js'));
390+
var requirePath = escaped(SetupCompilerOptions.buildRoot
391+
.resolve(
392+
p.join('dart-sdk', 'lib', 'dev_compiler', 'amd', 'require.js'))
393+
.toFilePath());
385394
var outputPath = escaped(p.withoutExtension(output.toFilePath()));
386395
bootstrapFile.writeAsStringSync('''
387396
<script src='$requirePath'></script>

pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,9 +737,13 @@ class TestProjectConfiguration {
737737
Uri get packagesPath => root.resolve('package_config.json');
738738

739739
Uri get sdkRoot => computePlatformBinariesLocation();
740+
// Use the outline copied to the released SDK.
741+
// Unsound .dill files are not longer in the released SDK so this file must be
742+
// read from the build output directory.
740743
Uri get sdkSummaryPath => soundNullSafety
741744
? sdkRoot.resolve('ddc_outline.dill')
742-
: sdkRoot.resolve('ddc_outline_unsound.dill');
745+
: computePlatformBinariesLocation(forceBuildDir: true)
746+
.resolve('ddc_outline_unsound.dill');
743747
Uri get librariesPath => sdkRoot.resolve('lib/libraries.json');
744748

745749
List get inputUris => [
@@ -1139,7 +1143,7 @@ class DDCKernelGenerator {
11391143
for (var module in config.modules.values) {
11401144
exitCode = await _generateSummary(module);
11411145
expect(exitCode, 0,
1142-
reason: 'Failed to generate fsummary for ${module.moduleName}');
1146+
reason: 'Failed to generate summary dill for ${module.moduleName}');
11431147
}
11441148

11451149
// generate full dill

pkg/dev_compiler/test/expression_compiler/scope_offset_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import 'package:test/test.dart';
1414
bool get verbose => false;
1515

1616
Uri get sdkRoot => computePlatformBinariesLocation();
17-
Uri get sdkSummaryPath => sdkRoot.resolve('ddc_outline_unsound.dill');
17+
// Unsound .dill files are not longer in the released SDK so this file must be
18+
// read from the build output directory.
19+
Uri get sdkSummaryPath => computePlatformBinariesLocation(forceBuildDir: true)
20+
.resolve('ddc_outline_unsound.dill');
1821
Uri get librariesPath => sdkRoot.resolve('lib/libraries.json');
1922

2023
void main(List<String> args) {

pkg/dev_compiler/test/nullable_inference_test.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'package:dev_compiler/src/kernel/js_typerep.dart';
1111
import 'package:dev_compiler/src/kernel/nullable_inference.dart';
1212
import 'package:dev_compiler/src/kernel/target.dart';
1313
import 'package:front_end/src/api_unstable/ddc.dart' as fe;
14+
import 'package:front_end/src/compute_platform_binaries_location.dart';
1415
import 'package:kernel/class_hierarchy.dart';
1516
import 'package:kernel/core_types.dart';
1617
import 'package:kernel/kernel.dart';
@@ -657,8 +658,9 @@ Future<CompileResult> kernelCompile(String code) async {
657658
var sdkUri = Uri.file('/memory/ddc_outline_unsound.dill');
658659
var sdkFile = _fileSystem.entityForUri(sdkUri);
659660
if (!await sdkFile.exists()) {
661+
var buildRoot = computePlatformBinariesLocation(forceBuildDir: true);
660662
var outlineDill =
661-
p.join(getSdkPath(), 'lib', '_internal', 'ddc_outline_unsound.dill');
663+
buildRoot.resolve('ddc_outline_unsound.dill').toFilePath();
662664
sdkFile.writeAsBytesSync(File(outlineDill).readAsBytesSync());
663665
}
664666
var librariesUri = Uri.file('/memory/libraries.json');

pkg/dev_compiler/test/sdk_source_map_test.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ import 'package:front_end/src/compute_platform_binaries_location.dart';
99
import 'package:path/path.dart' as p;
1010
import 'package:source_maps/source_maps.dart' as sm;
1111

12+
/// Verifies that the compiled SDK modules used in the SDK test suites have
13+
/// source maps, and those mappings correctly point to the .dart source files.
14+
///
15+
/// There are no longer any precompiled SDK modules distributed with the Dart
16+
/// SDK so this test depends on built assets from the gen/utils/dartdevc
17+
/// directory.
1218
void main() async {
1319
// This test relies on source maps for the built SDK when working inside the
1420
// Dart SDK repo.

pkg/dev_compiler/test/shared_test_options.dart

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,13 @@ class DevelopmentIncrementalCompiler extends IncrementalCompiler {
4040

4141
class SetupCompilerOptions {
4242
static final sdkRoot = computePlatformBinariesLocation();
43+
// Unsound .dill files are not longer in the released SDK so this file must be
44+
// read from the build output directory.
4345
static final sdkUnsoundSummaryPath =
44-
p.join(sdkRoot.path, 'ddc_outline_unsound.dill');
45-
static final sdkSoundSummaryPath = p.join(sdkRoot.path, 'ddc_outline.dill');
46-
// TODO(46617) Call getSdkPath() from command.dart instead.
46+
computePlatformBinariesLocation(forceBuildDir: true)
47+
.resolve('ddc_outline_unsound.dill');
48+
// Use the outline copied to the released SDK.
49+
static final sdkSoundSummaryPath = sdkRoot.resolve('ddc_outline.dill');
4750
static final librariesSpecificationUri =
4851
p.join(p.dirname(p.dirname(getSdkPath())), 'libraries.json');
4952

@@ -54,8 +57,8 @@ class SetupCompilerOptions {
5457
..target = DevCompilerTarget(TargetFlags())
5558
..librariesSpecificationUri = Uri.base.resolve('sdk/lib/libraries.json')
5659
..omitPlatform = true
57-
..sdkSummary = sdkRoot.resolve(
58-
soundNullSafety ? sdkSoundSummaryPath : sdkUnsoundSummaryPath)
60+
..sdkSummary =
61+
soundNullSafety ? sdkSoundSummaryPath : sdkUnsoundSummaryPath
5962
..environmentDefines = const {}
6063
..nnbdMode = soundNullSafety ? NnbdMode.Strong : NnbdMode.Weak;
6164
return options;

pkg/test_runner/lib/src/compiler_configuration.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,12 @@ class DevCompilerConfiguration extends CompilerConfiguration {
626626
// the bootstrapping code, instead of a compiler option.
627627
var options = sharedOptions.toList();
628628
options.remove('--null-assertions');
629-
if (!_useSdk) {
629+
if (!_useSdk || _configuration.nnbdMode != NnbdMode.strong) {
630630
// If we're testing a built SDK, DDC will find its own summary.
631631
//
632+
// Unsound summary files are not longer bundled with the built SDK so they
633+
// must always be specified manually.
634+
//
632635
// For local development we don't have a built SDK yet, so point directly
633636
// at the built summary file location.
634637
var sdkSummaryFile = _configuration.nnbdMode == NnbdMode.strong

sdk/BUILD.gn

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ declare_args() {
3636
# ......dart or dart.exe (executable)
3737
# ......dart.lib (import library for VM native extensions on Windows)
3838
# ......dartaotruntime or dartaotruntime.exe (executable)
39-
# ......dart2js
40-
# ......dartdevc
4139
# ......utils/gen_snapshot or utils/gen_snapshot.exe (if not on ia32)
4240
# ......snapshots/
4341
# ........analysis_server.dart.snapshot
@@ -70,12 +68,20 @@ declare_args() {
7068
# ........dart2js_server_platform_unsound.dill
7169
# ........dart2wasm_platform.dill (if not on ia32)
7270
# ........dart2wasm_outline.dill (if not on ia32)
71+
# ........ddc_outline.dill
72+
# ........ddc_platform.dill
7373
# ........vm_platform_strong.dill
74-
# ........dev_compiler/
74+
# ........js_dev_runtime/
75+
# ........js_runtime/
76+
# ........js_shared/
7577
# ......async/
7678
# ......collection/
7779
# ......convert/
7880
# ......core/
81+
# ......dev_compiler/
82+
#.........amd/require.js
83+
#.........web/dart_stack_trace_mapper.js
84+
# ......developer/
7985
# ......html/
8086
# ......_http/
8187
# ......internal/
@@ -518,13 +524,10 @@ copy("copy_dev_compiler_dills") {
518524
deps = [
519525
":copy_libraries",
520526
"../utils/dartdevc:dartdevc_platform_sound",
521-
"../utils/dartdevc:dartdevc_platform_unsound",
522527
]
523528
sources = [
524529
"$root_out_dir/ddc_outline.dill",
525-
"$root_out_dir/ddc_outline_unsound.dill",
526530
"$root_out_dir/ddc_platform.dill",
527-
"$root_out_dir/ddc_platform_unsound.dill",
528531
]
529532
outputs =
530533
[ "$root_out_dir/$dart_sdk_output/lib/_internal/{{source_file_part}}" ]

0 commit comments

Comments
 (0)