[dart2wasm] Fix handling of --define/-D
When parsing `--define` or `-D` arguments don't split the the value by
commas.
This is consistent with how dart2js handles `-D`, but inconsistent with
how VM handles it.
Example:
void main() {
print(const String.fromEnvironment("FOO"));
}
When compiled with `dart compile js -DFOO="a, b"` and run, dart2js
prints
a, b
VM prints (when compiled to exe)
a
Between these two, I think dart2js' behavior is more common, so we
follow dart2js.
Also update compile_benchmark to avoid splitting a single argument "a b"
into "a" and "b" when parsing the arguments and then splicing them back
before calling `dart2wasm`.
Also update the test runner and ddc batch mode argument parser to handle
splitting quoted arguments in `// dart2jsOption = ...` and the same
options for ddc and dart2wasm, by moving dart2js's `splitLine` to a new
library and reusing it in the test runner and ddc.
Fixes https://github.com/flutter/flutter/issues/164873.
See also https://github.com/dart-lang/sdk/issues/60341 for relevant
future work.
Change-Id: Idbdf69072fa212c8e4a390990577eb5a57b49e8a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415280
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
This commit is contained in:
committed by
Commit Queue
parent
367474d576
commit
f4b41f0902
@@ -11,6 +11,7 @@ import 'dart:isolate' show Isolate;
|
||||
|
||||
// ignore: implementation_imports
|
||||
import 'package:front_end/src/api_unstable/dart2js.dart' as fe;
|
||||
import 'package:shell_arg_splitter/shell_arg_splitter.dart';
|
||||
|
||||
import '../compiler_api.dart' as api;
|
||||
import 'commandline_options.dart';
|
||||
@@ -20,7 +21,6 @@ import 'io/mapped_file.dart';
|
||||
import 'options.dart'
|
||||
show CompilerOptions, CompilerStage, DumpInfoFormat, FeatureOptions;
|
||||
import 'source_file_provider.dart';
|
||||
import 'util/command_line.dart';
|
||||
import 'util/util.dart' show stackTraceFilePrefix;
|
||||
|
||||
const String _defaultSpecificationUri = '../../../../sdk/lib/libraries.json';
|
||||
|
||||
@@ -21,6 +21,7 @@ dependencies:
|
||||
kernel: any
|
||||
meta: any
|
||||
mmap: any
|
||||
shell_arg_splitter: any
|
||||
vm_service: any
|
||||
|
||||
# Use 'any' constraints here; we get our versions from the DEPS file.
|
||||
|
||||
@@ -68,7 +68,7 @@ final List<Option> options = [
|
||||
"watch", (o, values) => o.translatorOptions.watchPoints = values),
|
||||
StringMultiOption(
|
||||
"define", (o, values) => o.environment.addAll(processEnvironment(values)),
|
||||
abbr: "D"),
|
||||
abbr: "D", splitCommas: false),
|
||||
StringMultiOption(
|
||||
"enable-experiment",
|
||||
(o, values) =>
|
||||
|
||||
@@ -78,10 +78,12 @@ class MultiValueOption<T> extends Option<List<T>> {
|
||||
void Function(WasmCompilerOptions o, List<T> v) applyToOptions,
|
||||
T Function(dynamic v) converter,
|
||||
{Iterable<String>? defaultsTo,
|
||||
String? abbr})
|
||||
String? abbr,
|
||||
bool splitCommas = true})
|
||||
: super(
|
||||
name,
|
||||
(a) => a.addMultiOption(name, abbr: abbr, defaultsTo: defaultsTo),
|
||||
(a) => a.addMultiOption(name,
|
||||
abbr: abbr, defaultsTo: defaultsTo, splitCommas: splitCommas),
|
||||
applyToOptions,
|
||||
(vs) => vs.map(converter).cast<T>().toList());
|
||||
}
|
||||
@@ -97,9 +99,9 @@ class IntMultiOption extends MultiValueOption<int> {
|
||||
class StringMultiOption extends MultiValueOption<String> {
|
||||
StringMultiOption(String name,
|
||||
void Function(WasmCompilerOptions o, List<String> v) applyToOptions,
|
||||
{String? abbr, Iterable<String>? defaultsTo})
|
||||
{String? abbr, Iterable<String>? defaultsTo, bool splitCommas = true})
|
||||
: super(name, applyToOptions, (v) => v,
|
||||
abbr: abbr, defaultsTo: defaultsTo);
|
||||
abbr: abbr, defaultsTo: defaultsTo, splitCommas: splitCommas);
|
||||
}
|
||||
|
||||
class UriMultiOption extends MultiValueOption<Uri> {
|
||||
|
||||
@@ -133,7 +133,7 @@ while [ $# -gt 0 ]; do
|
||||
;;
|
||||
|
||||
--extra-compiler-option=*)
|
||||
DART2WASM_ARGS+=(${1#--extra-compiler-option=})
|
||||
DART2WASM_ARGS+=("${1#--extra-compiler-option=}")
|
||||
shift
|
||||
;;
|
||||
|
||||
@@ -219,7 +219,7 @@ fi
|
||||
binaryen_command=("$BINARYEN" "${BINARYEN_FLAGS[@]}" "$WASM_FILE" -o "$WASM_FILE")
|
||||
|
||||
if [ -n "$COMPILE_BENCHMARK_BASE_NAME" ]; then
|
||||
measure ${dart2wasm_command[@]}
|
||||
measure "${dart2wasm_command[@]}"
|
||||
COMPILER_TIME=$TIME
|
||||
COMPILER_MEMORY=$MEMORY
|
||||
|
||||
@@ -262,6 +262,6 @@ if [ -n "$COMPILE_BENCHMARK_BASE_NAME" ]; then
|
||||
echo "$COMPILE_BENCHMARK_BASE_NAME.MemoryUse.Dart2Wasm(MemoryUse): $COMPILER_MEMORY bytes"
|
||||
run_if_binaryen_enabled echo "$COMPILE_BENCHMARK_BASE_NAME.MemoryUse.Wasm2WasmOpt(MemoryUse): $BINARYEN_MEMORY bytes"
|
||||
else
|
||||
${dart2wasm_command[@]}
|
||||
"${dart2wasm_command[@]}"
|
||||
run_if_binaryen_enabled ${binaryen_command[@]}
|
||||
fi
|
||||
|
||||
@@ -799,6 +799,7 @@ class CompileWasmCommand extends CompileSubcommandCommand {
|
||||
help: defineOption.help,
|
||||
abbr: defineOption.abbr,
|
||||
valueHelp: defineOption.valueHelp,
|
||||
splitCommas: false,
|
||||
)
|
||||
..addExperimentalFlags(verbose: verbose);
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ import 'dart:io';
|
||||
import 'dart:isolate';
|
||||
import 'package:bazel_worker/bazel_worker.dart';
|
||||
import 'package:kernel/ast.dart' show clearDummyTreeNodesParentPointer;
|
||||
import 'package:shell_arg_splitter/shell_arg_splitter.dart';
|
||||
|
||||
import 'src/command/arguments.dart';
|
||||
import 'src/command/command.dart';
|
||||
@@ -129,7 +130,7 @@ class _BatchHelper {
|
||||
|
||||
Future<void> _doIteration(ParsedArguments batchArgs, String line) async {
|
||||
totalTests++;
|
||||
var args = batchArgs.merge(line.split(RegExp(r'\s+')));
|
||||
var args = batchArgs.merge(splitLine(line));
|
||||
|
||||
String outcome;
|
||||
try {
|
||||
|
||||
@@ -18,6 +18,7 @@ dependencies:
|
||||
js_shared: any
|
||||
kernel: any
|
||||
path: any
|
||||
shell_arg_splitter: any
|
||||
source_maps: any
|
||||
source_span: any
|
||||
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
include: package:lints/recommended.yaml
|
||||
+2
-4
@@ -2,12 +2,10 @@
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
library;
|
||||
|
||||
/// The accepted escapes in the input of the --batch processor.
|
||||
///
|
||||
/// Contrary to Dart strings it does not contain hex escapes (\u or \x).
|
||||
const Map<String, String> escapeMapping = {
|
||||
const Map<String, String> _escapeMapping = {
|
||||
'n': '\n',
|
||||
'r': '\r',
|
||||
't': '\t',
|
||||
@@ -57,7 +55,7 @@ List<String> splitLine(String line, {bool windows = false}) {
|
||||
i++;
|
||||
|
||||
c = line[i];
|
||||
String mapped = escapeMapping[c] ?? c;
|
||||
String mapped = _escapeMapping[c] ?? c;
|
||||
buffer.write(mapped);
|
||||
continue;
|
||||
}
|
||||
@@ -0,0 +1,10 @@
|
||||
name: shell_arg_splitter
|
||||
# This package is not intended for consumption on pub.dev. DO NOT publish.
|
||||
publish_to: none
|
||||
|
||||
environment:
|
||||
sdk: '^3.7.0'
|
||||
|
||||
dev_dependencies:
|
||||
expect: any
|
||||
lints: any
|
||||
+17
-7
@@ -3,9 +3,9 @@
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import "package:expect/expect.dart";
|
||||
import 'package:compiler/src/util/command_line.dart';
|
||||
import "package:shell_arg_splitter/shell_arg_splitter.dart";
|
||||
|
||||
main() {
|
||||
void main() {
|
||||
Expect.listEquals(["foo", "bar"], splitLine("foo bar"));
|
||||
Expect.listEquals(["foo", "bar"], splitLine("foo bar", windows: true));
|
||||
|
||||
@@ -28,13 +28,23 @@ main() {
|
||||
"''bar",
|
||||
], splitLine(r"""foo"'" '"'bar" """, windows: true));
|
||||
|
||||
Expect.listEquals(["foo", "bar"], splitLine("'f''o''o' " + '"b""a""r"'));
|
||||
Expect.listEquals(
|
||||
["foo", "bar"],
|
||||
splitLine(
|
||||
"'f''o''o' "
|
||||
'"b""a""r"',
|
||||
),
|
||||
);
|
||||
// TODO(johnniwinther): This is not actual Windows behavior: "b""a" is
|
||||
// interpreted as b"a but "b""a""r" is interpreted as b"ar.
|
||||
Expect.listEquals([
|
||||
"'f''o''o'",
|
||||
"bar",
|
||||
], splitLine("'f''o''o' " + '"b""a""r"', windows: true));
|
||||
Expect.listEquals(
|
||||
["'f''o''o'", "bar"],
|
||||
splitLine(
|
||||
"'f''o''o' "
|
||||
'"b""a""r"',
|
||||
windows: true,
|
||||
),
|
||||
);
|
||||
|
||||
Expect.listEquals([
|
||||
"\n",
|
||||
@@ -1058,6 +1058,7 @@ class BatchRunnerProcess {
|
||||
}
|
||||
|
||||
String _createArgumentsLine(List<String> arguments, int timeout) {
|
||||
arguments = arguments.map(escapeCommandLineArgument).toList();
|
||||
if (_useJson) {
|
||||
return "${jsonEncode(arguments)}\n";
|
||||
} else {
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:shell_arg_splitter/shell_arg_splitter.dart';
|
||||
|
||||
import 'feature.dart';
|
||||
import 'path.dart';
|
||||
import 'static_error.dart';
|
||||
@@ -22,9 +24,6 @@ final _localFileRegExp = RegExp(
|
||||
r"""(?:(?:show|hide)\s+\w+\s*(?:,\s*\w+\s*))*;""",
|
||||
multiLine: true);
|
||||
|
||||
List<String> _splitWords(String s) =>
|
||||
s.split(' ')..removeWhere((s) => s.isEmpty);
|
||||
|
||||
List<T> _parseOption<T>(
|
||||
String filePath, String contents, String name, T Function(String) convert,
|
||||
{bool allowMultiple = false}) {
|
||||
@@ -37,7 +36,7 @@ List<T> _parseOption<T>(
|
||||
|
||||
var options = <T>[];
|
||||
for (var match in matches) {
|
||||
for (var option in _splitWords(match[1]!)) {
|
||||
for (var option in splitLine(match[1]!)) {
|
||||
options.add(convert(option));
|
||||
}
|
||||
}
|
||||
@@ -228,7 +227,7 @@ class TestFile extends _TestFileBase {
|
||||
var vmOptions = <List<String>>[];
|
||||
var matches = _vmOptionsRegExp.allMatches(contents);
|
||||
for (var match in matches) {
|
||||
vmOptions.add(_splitWords(match[1]!));
|
||||
vmOptions.add(splitLine(match[1]!));
|
||||
}
|
||||
if (vmOptions.isEmpty) vmOptions.add(<String>[]);
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ dependencies:
|
||||
package_config: any
|
||||
path: any
|
||||
pool: any
|
||||
shell_arg_splitter: any
|
||||
smith: any
|
||||
status_file: any
|
||||
webdriver: any
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
// dart2jsOptions="-DFOO=a, b, c=123" --define=BAR=hi
|
||||
// ddcOptions="-DFOO=a, b, c=123" --define=BAR=hi
|
||||
// dart2wasmOptions="--extra-compiler-option=-DFOO=a, b, c=123" --define=BAR=hi
|
||||
|
||||
import 'package:expect/expect.dart';
|
||||
|
||||
void main() {
|
||||
Expect.equals("a, b, c=123", const String.fromEnvironment("FOO"));
|
||||
Expect.equals("hi", const String.fromEnvironment("BAR"));
|
||||
}
|
||||
Reference in New Issue
Block a user