[ddc] Avoid collision of a top-level function name with parameter names.

Safari has a bug that makes it a syntax error for a function name
to overlap with names of parameters in functions with default
parameter values.

This CL changes DDC to generates code to circumvent this issue
when emitting top-level methods.  Fortunately, the Safari bug
doesn't trigger with ES6 methods, so we don't need to do anything
for Dart instance methods.

The only other occurrence of named functions in DDC are
`function*` generators that we emit. Those take no arguments,
so we don't need any additional renaming there.

Fixes #43520

Change-Id: I2e4588701a294a8f3c5b47956826ada4ed973e6c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198200
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>

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

@@ -2642,14 +2642,37 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     }
 
     var nameExpr = _emitTopLevelName(p);
+    var jsName = _safeFunctionNameForSafari(p.name.text, fn);
     body.add(js.statement('# = #',
-        [nameExpr, js_ast.NamedFunction(_emitTemporaryId(p.name.text), fn)]));
+        [nameExpr, js_ast.NamedFunction(_emitTemporaryId(jsName), fn)]));
 
     _currentUri = savedUri;
     _staticTypeContext.leaveMember(p);
     return js_ast.Statement.from(body);
   }
 
+  /// Choose a safe name for [fn].
+  ///
+  /// Most of the time we use [candidateName], except if the name collides
+  /// with a parameter name and the function contains default parameter values.
+  ///
+  /// In ES6, functions containing default parameter values, which DDC
+  /// generates when Dart uses positional optional parameters, cannot have
+  /// two parameters with the same name. Because we have a similar restriction
+  /// in Dart, this is not normally an issue we need to pay attention to.
+  /// However, a bug in Safari makes it a syntax error to have the function
+  /// name overlap with the parameter names as well. This rename works around
+  /// such bug (dartbug.com/43520).
+  static String _safeFunctionNameForSafari(
+      String candidateName, js_ast.Fun fn) {
+    if (fn.params.any((p) => p is js_ast.DestructuredVariable)) {
+      while (fn.params.any((a) => a.parameterName == candidateName)) {
+        candidateName = '$candidateName\$';
+      }
+    }
+    return candidateName;
+  }
+
   js_ast.Expression _emitFunctionTagged(js_ast.Expression fn, FunctionType type,
       {bool topLevel = false}) {
     var lazy = topLevel && !_canEmitTypeAtTopLevel(type);

tests/web/regress/43520_safari_test.dart

@@ -0,0 +1,20 @@
+// Copyright (c) 2021, 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.
+
+/// Regression test for dartbug.com/43520.
+///
+/// Safari has a bug that makes it a syntax error for a function name to overlap
+/// with names of parameters in functions with default parameter values.
+///
+/// DDC now generates code to circumvent this issue.
+
+import 'package:expect/expect.dart';
+
+String a(Object a, [String f = '3']) {
+  return "$a$f";
+}
+
+main() async {
+  Expect.equals('13', a(1));
+}

tests/web_2/regress/43520_safari_test.dart

@@ -0,0 +1,20 @@
+// Copyright (c) 2021, 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.
+
+/// Regression test for dartbug.com/43520.
+///
+/// Safari has a bug that makes it a syntax error for a function name to overlap
+/// with names of parameters in functions with default parameter values.
+///
+/// DDC now generates code to circumvent this issue.
+
+import 'package:expect/expect.dart';
+
+String a(Object a, [String f = '3']) {
+  return "$a$f";
+}
+
+main() async {
+  Expect.equals('13', a(1));
+}