[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>
diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart
index 43364f2..1bfdd5c 100644
--- a/pkg/compiler/lib/src/dart2js.dart
+++ b/pkg/compiler/lib/src/dart2js.dart
@@ -11,6 +11,7 @@
// 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 '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';
diff --git a/pkg/compiler/pubspec.yaml b/pkg/compiler/pubspec.yaml
index e170571..e31c7c0 100644
--- a/pkg/compiler/pubspec.yaml
+++ b/pkg/compiler/pubspec.yaml
@@ -21,6 +21,7 @@
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.
diff --git a/pkg/dart2wasm/lib/dart2wasm.dart b/pkg/dart2wasm/lib/dart2wasm.dart
index 9cb8c5a..9138d76 100644
--- a/pkg/dart2wasm/lib/dart2wasm.dart
+++ b/pkg/dart2wasm/lib/dart2wasm.dart
@@ -68,7 +68,7 @@
"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) =>
diff --git a/pkg/dart2wasm/lib/option.dart b/pkg/dart2wasm/lib/option.dart
index b0e8da4..89ddb5b 100644
--- a/pkg/dart2wasm/lib/option.dart
+++ b/pkg/dart2wasm/lib/option.dart
@@ -78,10 +78,12 @@
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 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> {
diff --git a/pkg/dart2wasm/tool/compile_benchmark b/pkg/dart2wasm/tool/compile_benchmark
index b2aaa93..e9dab8c 100755
--- a/pkg/dart2wasm/tool/compile_benchmark
+++ b/pkg/dart2wasm/tool/compile_benchmark
@@ -133,7 +133,7 @@
;;
--extra-compiler-option=*)
- DART2WASM_ARGS+=(${1#--extra-compiler-option=})
+ DART2WASM_ARGS+=("${1#--extra-compiler-option=}")
shift
;;
@@ -219,7 +219,7 @@
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 @@
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
diff --git a/pkg/dartdev/lib/src/commands/compile.dart b/pkg/dartdev/lib/src/commands/compile.dart
index ce98b11..2829f7e 100644
--- a/pkg/dartdev/lib/src/commands/compile.dart
+++ b/pkg/dartdev/lib/src/commands/compile.dart
@@ -799,6 +799,7 @@
help: defineOption.help,
abbr: defineOption.abbr,
valueHelp: defineOption.valueHelp,
+ splitCommas: false,
)
..addExperimentalFlags(verbose: verbose);
}
diff --git a/pkg/dev_compiler/lib/ddc.dart b/pkg/dev_compiler/lib/ddc.dart
index 3526364..55c739a 100755
--- a/pkg/dev_compiler/lib/ddc.dart
+++ b/pkg/dev_compiler/lib/ddc.dart
@@ -14,6 +14,7 @@
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 @@
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 {
diff --git a/pkg/dev_compiler/pubspec.yaml b/pkg/dev_compiler/pubspec.yaml
index 89a6240..fe69c37 100644
--- a/pkg/dev_compiler/pubspec.yaml
+++ b/pkg/dev_compiler/pubspec.yaml
@@ -18,6 +18,7 @@
js_shared: any
kernel: any
path: any
+ shell_arg_splitter: any
source_maps: any
source_span: any
diff --git a/pkg/shell_arg_splitter/analysis_options.yaml b/pkg/shell_arg_splitter/analysis_options.yaml
new file mode 100644
index 0000000..572dd23
--- /dev/null
+++ b/pkg/shell_arg_splitter/analysis_options.yaml
@@ -0,0 +1 @@
+include: package:lints/recommended.yaml
diff --git a/pkg/compiler/lib/src/util/command_line.dart b/pkg/shell_arg_splitter/lib/shell_arg_splitter.dart
similarity index 94%
rename from pkg/compiler/lib/src/util/command_line.dart
rename to pkg/shell_arg_splitter/lib/shell_arg_splitter.dart
index 4bb7923..4248b29 100644
--- a/pkg/compiler/lib/src/util/command_line.dart
+++ b/pkg/shell_arg_splitter/lib/shell_arg_splitter.dart
@@ -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 @@
i++;
c = line[i];
- String mapped = escapeMapping[c] ?? c;
+ String mapped = _escapeMapping[c] ?? c;
buffer.write(mapped);
continue;
}
diff --git a/pkg/shell_arg_splitter/pubspec.yaml b/pkg/shell_arg_splitter/pubspec.yaml
new file mode 100644
index 0000000..d21c97b
--- /dev/null
+++ b/pkg/shell_arg_splitter/pubspec.yaml
@@ -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
diff --git a/pkg/compiler/test/end_to_end/command_line_split_test.dart b/pkg/shell_arg_splitter/test/command_line_split_test.dart
similarity index 87%
rename from pkg/compiler/test/end_to_end/command_line_split_test.dart
rename to pkg/shell_arg_splitter/test/command_line_split_test.dart
index 6b82827..feedb3d 100644
--- a/pkg/compiler/test/end_to_end/command_line_split_test.dart
+++ b/pkg/shell_arg_splitter/test/command_line_split_test.dart
@@ -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 @@
"''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",
diff --git a/pkg/test_runner/lib/src/process_queue.dart b/pkg/test_runner/lib/src/process_queue.dart
index 2ab2eb8..0851d59 100644
--- a/pkg/test_runner/lib/src/process_queue.dart
+++ b/pkg/test_runner/lib/src/process_queue.dart
@@ -1058,6 +1058,7 @@
}
String _createArgumentsLine(List<String> arguments, int timeout) {
+ arguments = arguments.map(escapeCommandLineArgument).toList();
if (_useJson) {
return "${jsonEncode(arguments)}\n";
} else {
diff --git a/pkg/test_runner/lib/src/test_file.dart b/pkg/test_runner/lib/src/test_file.dart
index adcc01f..163a135 100644
--- a/pkg/test_runner/lib/src/test_file.dart
+++ b/pkg/test_runner/lib/src/test_file.dart
@@ -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 @@
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 @@
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 @@
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>[]);
diff --git a/pkg/test_runner/pubspec.yaml b/pkg/test_runner/pubspec.yaml
index 15c4089..57f20b9 100644
--- a/pkg/test_runner/pubspec.yaml
+++ b/pkg/test_runner/pubspec.yaml
@@ -17,6 +17,7 @@
package_config: any
path: any
pool: any
+ shell_arg_splitter: any
smith: any
status_file: any
webdriver: any
diff --git a/tests/web/wasm_js_shared/env_defines_test.dart b/tests/web/wasm_js_shared/env_defines_test.dart
new file mode 100644
index 0000000..c78dec2
--- /dev/null
+++ b/tests/web/wasm_js_shared/env_defines_test.dart
@@ -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"));
+}
