Skip to content

Commit

Permalink
[gardening] Fix arguments test for app_jit(k).
Browse files Browse the repository at this point in the history
The fix for #35960 broke
on app_jit(k) runs, because these expect the arguments to be
passed when creating the jit snapshot. Pass dartOptions along to
computeCompilerArguments as well, so that the
AppJitCompilerConfiguration can add them in appropriately.

Change-Id: I8c7b5a3a1689943db4e6c3785ccaf8bfdb839c2d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97516
Reviewed-by: Stevie Strickland <sstrickl@google.com>
Commit-Queue: Stevie Strickland <sstrickl@google.com>
  • Loading branch information
sstrickl authored and commit-bot@chromium.org committed Mar 25, 2019
1 parent 580f44a commit ab8510a
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 10 deletions.
26 changes: 21 additions & 5 deletions tools/testing/dart/compiler_configuration.dart
Expand Up @@ -144,6 +144,7 @@ abstract class CompilerConfiguration {
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
Expand Down Expand Up @@ -235,6 +236,7 @@ class VMKernelCompilerConfiguration extends CompilerConfiguration
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
Expand Down Expand Up @@ -356,7 +358,12 @@ class ComposedCompilerConfiguration extends CompilerConfiguration {
}

List<String> computeCompilerArguments(
vmOptions, sharedOptions, dart2jsOptions, ddcOptions, args) {
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
// The result will be passed as an input to [extractArguments]
// (i.e. the arguments to the [PipelineCommand]).
return <String>[]
Expand Down Expand Up @@ -457,6 +464,7 @@ class Dart2jsCompilerConfiguration extends Dart2xCompilerConfiguration {
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
Expand Down Expand Up @@ -509,6 +517,7 @@ class DevCompilerConfiguration extends CompilerConfiguration {
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
Expand Down Expand Up @@ -809,6 +818,7 @@ class PrecompilerCompilerConfiguration extends CompilerConfiguration
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> originalArguments) {
Expand Down Expand Up @@ -889,7 +899,12 @@ class AppJitCompilerConfiguration extends CompilerConfiguration {
}

List<String> computeCompilerArguments(
vmOptions, sharedOptions, dart2jsOptions, ddcOptions, originalArguments) {
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> originalArguments) {
var args = <String>[];
if (_useEnableAsserts) {
args.add('--enable_asserts');
Expand All @@ -898,7 +913,8 @@ class AppJitCompilerConfiguration extends CompilerConfiguration {
..addAll(vmOptions)
..addAll(sharedOptions)
..addAll(_configuration.sharedOptions)
..addAll(originalArguments);
..addAll(originalArguments)
..addAll(dartOptions);
}

List<String> computeRuntimeArguments(
Expand All @@ -913,13 +929,12 @@ class AppJitCompilerConfiguration extends CompilerConfiguration {
if (_useEnableAsserts) {
args.add('--enable_asserts');
}
args
return args
..addAll(vmOptions)
..addAll(sharedOptions)
..addAll(_configuration.sharedOptions)
..addAll(_replaceDartFiles(originalArguments, artifact.filename))
..addAll(dartOptions);
return args;
}
}

Expand Down Expand Up @@ -1190,6 +1205,7 @@ class FastaCompilerConfiguration extends CompilerConfiguration {
List<String> computeCompilerArguments(
List<String> vmOptions,
List<String> sharedOptions,
List<String> dartOptions,
List<String> dart2jsOptions,
List<String> ddcOptions,
List<String> args) {
Expand Down
15 changes: 10 additions & 5 deletions tools/testing/dart/test_suite.dart
Expand Up @@ -821,14 +821,23 @@ class StandardTestSuite extends TestSuite {
var commands = <Command>[];
var compilerConfiguration = configuration.compilerConfiguration;
var sharedOptions = info.optionsFromFile['sharedOptions'] as List<String>;
var dartOptions = info.optionsFromFile['dartOptions'] as List<String>;
var dart2jsOptions = info.optionsFromFile['dart2jsOptions'] as List<String>;
var ddcOptions = info.optionsFromFile['ddcOptions'] as List<String>;

var isMultitest = info.optionsFromFile["isMultitest"] as bool;
assert(!isMultitest || dartOptions.isEmpty);

var compileTimeArguments = <String>[];
String tempDir;
if (compilerConfiguration.hasCompiler) {
compileTimeArguments = compilerConfiguration.computeCompilerArguments(
vmOptions, sharedOptions, dart2jsOptions, ddcOptions, args);
vmOptions,
sharedOptions,
dartOptions,
dart2jsOptions,
ddcOptions,
args);
// Avoid doing this for analyzer.
var path = info.filePath;
if (vmOptionsVariant != 0) {
Expand Down Expand Up @@ -862,10 +871,6 @@ class StandardTestSuite extends TestSuite {
return commands;
}

var isMultitest = info.optionsFromFile["isMultitest"] as bool;
var dartOptions = info.optionsFromFile['dartOptions'] as List<String>;
assert(!isMultitest || dartOptions.isEmpty);

List<String> runtimeArguments =
compilerConfiguration.computeRuntimeArguments(
configuration.runtimeConfiguration,
Expand Down

0 comments on commit ab8510a

Please sign in to comment.