Skip to content

Commit

Permalink
[stable][ddc] Add ddc-options to compiler configuration
Browse files Browse the repository at this point in the history
Use explicit ddc-options to pass `--canary` flag to the compiler instead
of sniffing for a builder tag.

This just feels more expected and makes more sense if you are trying
to understand how the canary configurations compile tests differently
from the stable configurations.

Issue: #51481
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/280783
Change-Id: I6cc7c6a0a573b6f397b8b183dff13758f816fd33
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284542
Reviewed-by: Alexander Thomas <athom@google.com>
  • Loading branch information
nshahan authored and athomas committed Feb 28, 2023
1 parent 360c9b7 commit 0ec8740
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 5 deletions.
9 changes: 9 additions & 0 deletions pkg/smith/lib/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ class Configuration {
genKernelOptions: stringListOption("gen-kernel-options"),
vmOptions: stringListOption("vm-options"),
dart2jsOptions: stringListOption("dart2js-options"),
ddcOptions: stringListOption("ddc-options"),
experiments: stringListOption("enable-experiment"),
timeout: intOption("timeout"),
enableAsserts: boolOption("enable-asserts"),
Expand Down Expand Up @@ -300,6 +301,8 @@ class Configuration {

final List<String> dart2jsOptions;

final List<String> ddcOptions;

/// The names of the experiments to enable while running tests.
///
/// A test may *require* an experiment to always be enabled by containing a
Expand Down Expand Up @@ -347,6 +350,7 @@ class Configuration {
List<String>? genKernelOptions,
List<String>? vmOptions,
List<String>? dart2jsOptions,
List<String>? ddcOptions,
List<String>? experiments,
int? timeout,
bool? enableAsserts,
Expand All @@ -368,6 +372,7 @@ class Configuration {
genKernelOptions = genKernelOptions ?? <String>[],
vmOptions = vmOptions ?? <String>[],
dart2jsOptions = dart2jsOptions ?? <String>[],
ddcOptions = ddcOptions ?? <String>[],
experiments = experiments ?? <String>[],
timeout = timeout ?? -1,
enableAsserts = enableAsserts ?? false,
Expand Down Expand Up @@ -403,6 +408,7 @@ class Configuration {
_listsEqual(genKernelOptions, other.genKernelOptions) &&
_listsEqual(vmOptions, other.vmOptions) &&
_listsEqual(dart2jsOptions, other.dart2jsOptions) &&
_listsEqual(ddcOptions, other.ddcOptions) &&
_listsEqual(experiments, other.experiments) &&
timeout == other.timeout &&
enableAsserts == other.enableAsserts &&
Expand Down Expand Up @@ -453,6 +459,7 @@ class Configuration {
genKernelOptions.join(" & ").hashCode ^
vmOptions.join(" & ").hashCode ^
dart2jsOptions.join(" & ").hashCode ^
ddcOptions.join(" & ").hashCode ^
experiments.join(" & ").hashCode ^
timeout.hashCode ^
_toBinary([
Expand Down Expand Up @@ -495,6 +502,7 @@ class Configuration {
stringListField("gen-kernel-options", genKernelOptions);
stringListField("vm-options", vmOptions);
stringListField("dart2js-options", dart2jsOptions);
stringListField("ddc-options", ddcOptions);
stringListField("enable-experiment", experiments);
if (timeout > 0) fields.add("timeout: $timeout");
if (enableAsserts) fields.add("enable-asserts");
Expand Down Expand Up @@ -551,6 +559,7 @@ class Configuration {
"gen-kernel-options", genKernelOptions, other.genKernelOptions);
stringListField("vm-options", vmOptions, other.vmOptions);
stringListField("dart2js-options", dart2jsOptions, other.dart2jsOptions);
stringListField("ddc-options", ddcOptions, other.ddcOptions);
stringListField("experiments", experiments, other.experiments);
fields.add("timeout: $timeout ${other.timeout}");
boolField("enable-asserts", enableAsserts, other.enableAsserts);
Expand Down
4 changes: 4 additions & 0 deletions pkg/smith/test/configuration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ void main() {
builderTag: "a tag",
vmOptions: ["vm a1", "vm a2"],
dart2jsOptions: ["dart2js a1", "dart2js a2"],
ddcOptions: ["ddc a1", "ddc a2"],
experiments: ["experiment a1", "experiment a2"],
timeout: 1);

Expand All @@ -373,6 +374,7 @@ void main() {
builderTag: "b tag",
vmOptions: ["vm b1", "vm b2"],
dart2jsOptions: ["dart2js b1", "dart2js b2"],
ddcOptions: ["ddc b1", "ddc b2"],
experiments: ["experiment b1", "experiment b2"],
timeout: 2,
enableAsserts: true,
Expand Down Expand Up @@ -402,6 +404,7 @@ architecture: ia32 x64
builder-tag: a tag b tag
vm-options: [vm a1, vm a2] [vm b1, vm b2]
dart2js-options: [dart2js a1, dart2js a2] [dart2js b1, dart2js b2]
ddc-options: [ddc a1, ddc a2] [ddc b1, ddc b2]
experiments: [experiment a1, experiment a2] [experiment b1, experiment b2]
timeout: 1 2
enable-asserts: false true
Expand Down Expand Up @@ -432,6 +435,7 @@ architecture: ia32 ia32
builder-tag: a tag a tag
vm-options: [vm a1, vm a2] [vm a1, vm a2]
dart2js-options: [dart2js a1, dart2js a2] [dart2js a1, dart2js a2]
ddc-options: [ddc a1, ddc a2] [ddc a1, ddc a2]
experiments: [experiment a1, experiment a2] [experiment a1, experiment a2]
timeout: 1 1
"""));
Expand Down
9 changes: 6 additions & 3 deletions pkg/test_runner/lib/src/compiler_configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -602,11 +602,14 @@ class DevCompilerConfiguration extends CompilerConfiguration {
TestFile testFile, List<String> vmOptions, List<String> args) {
return [
...testFile.sharedOptions,
...testFile.ddcOptions,
..._configuration.sharedOptions,
..._configuration.ddcOptions,
..._experimentsArgument(_configuration, testFile),
...testFile.ddcOptions,
if (_configuration.nnbdMode == NnbdMode.strong) '--sound-null-safety',
if (_configuration.configuration.builderTag == 'canary') '--canary',
if (_configuration.nnbdMode == NnbdMode.strong)
'--sound-null-safety'
else
'--no-sound-null-safety',
// The file being compiled is the last argument.
args.last
];
Expand Down
3 changes: 3 additions & 0 deletions pkg/test_runner/lib/src/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ class TestConfiguration {
/// Extra dart2js options passed to the testing script.
List<String> get dart2jsOptions => configuration.dart2jsOptions;

/// Extra ddc options passed to the testing script.
List<String> get ddcOptions => configuration.ddcOptions;

/// Extra gen_kernel options passed to the testing script.
List<String> get genKernelOptions => configuration.genKernelOptions;

Expand Down
6 changes: 6 additions & 0 deletions pkg/test_runner/lib/src/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ options. Used to be able to make sane updates to the status files.''',
aliases: ['dart2js_options'],
hide: true,
help: 'Extra options for dart2js compilation step.')
..addMultiOption('ddc-options',
aliases: ['ddc_options'],
hide: true,
help: 'Extra command line options passed to the DDC compiler.')
..addMultiOption('shared-options',
aliases: ['shared_options'], hide: true, help: 'Extra shared options.')
..addMultiOption('enable-experiment',
Expand Down Expand Up @@ -628,6 +632,7 @@ has been specified on the command line.''')
}

var dart2jsOptions = listOption("dart2js-options");
var ddcOptions = listOption("ddc-options");
var vmOptions = listOption("vm-options");
var sharedOptions = listOption("shared-options");
var experiments = data["enable-experiment"] as List<String>?;
Expand Down Expand Up @@ -819,6 +824,7 @@ has been specified on the command line.''')
isMinified: data["minified"] as bool,
vmOptions: vmOptions,
dart2jsOptions: dart2jsOptions,
ddcOptions: ddcOptions,
experiments: experiments,
babel: data['babel'] as String?,
builderTag: data["builder-tag"] as String?,
Expand Down
10 changes: 8 additions & 2 deletions tools/bots/test_matrix.json
Original file line number Diff line number Diff line change
Expand Up @@ -928,15 +928,21 @@
"builder-tag": "canary",
"checked": true,
"use-sdk": true,
"enable-asserts": true
"enable-asserts": true,
"ddc-options": [
"--canary"
]
}
},
"dartdevc-canary-weak-(linux|mac|win)-release-(chrome|firefox)-(x64|arm64)": {
"options": {
"builder-tag": "canary",
"checked": true,
"use-sdk": true,
"enable-asserts": true
"enable-asserts": true,
"ddc-options": [
"--canary"
]
}
},
"cfe-(linux|mac|win)": {
Expand Down

0 comments on commit 0ec8740

Please sign in to comment.