[dart2js] Fix for #41890

- Use JS inline code instead of '%' in assertion (#41890)

Other changes help code work without inlining or type inference

- Use annotation to ensure isTopType has no entry check.
- Use JS_STRING_CONCAT instead of '+'.

Fixed: 41890
Change-Id: I1958f1ebbb6049a350fad07a975e8e851db2b115
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148006
Commit-Queue: Stephen Adams <[email protected]>
Reviewed-by: Mayank Patke <[email protected]>

Author: rakudrama

sdk_nnbd/lib/_internal/js_runtime/lib/rti.dart

@@ -14,6 +14,7 @@ import 'dart:_foreign_helper'
         JS_EMBEDDED_GLOBAL,
         JS_GET_FLAG,
         JS_GET_NAME,
+        JS_STRING_CONCAT,
         RAW_DART_FUNCTION_REF,
         TYPE_REF,
         LEGACY_TYPE_REF;
@@ -571,7 +572,7 @@ Object? _substituteNamed(
     Object? universe, Object? namedArray, Object? typeArguments, int depth) {
   bool changed = false;
   int length = _Utils.arrayLength(namedArray);
-  assert(length % 3 == 0);
+  assert(_Utils.isMultipleOf(length, 3));
   Object? result = JS('', '[]');
   for (int i = 0; i < length; i += 3) {
     String name = _Utils.asString(_Utils.arrayAt(namedArray, i));
@@ -1696,6 +1697,27 @@ class _Universe {
     return rti;
   }
 
+  // These helpers are used for creating canonical recipes. The key feature of
+  // the generated code is that it makes no reference to the constant pool,
+  // which does not exist when the type$ pool is created.
+  //
+  // The strange association is so that usage like
+  //
+  //     s = _recipeJoin3(s, a, b);
+  //
+  // associates as `s+=(a+b)` rather than `s=s+a+b`. As recipe fragments are
+  // small, this tends to create smaller cons-string trees.
+
+  static String _recipeJoin(String s1, String s2) => JS_STRING_CONCAT(s1, s2);
+  static String _recipeJoin3(String s1, String s2, String s3) =>
+      JS_STRING_CONCAT(s1, JS_STRING_CONCAT(s2, s3));
+  static String _recipeJoin4(String s1, String s2, String s3, String s4) =>
+      JS_STRING_CONCAT(s1, JS_STRING_CONCAT(JS_STRING_CONCAT(s2, s3), s4));
+  static String _recipeJoin5(
+          String s1, String s2, String s3, String s4, String s5) =>
+      JS_STRING_CONCAT(s1,
+          JS_STRING_CONCAT(JS_STRING_CONCAT(JS_STRING_CONCAT(s2, s3), s4), s5));
+
   // For each kind of Rti there are three methods:
   //
   // * `lookupXXX` which takes the component parts and returns an existing Rti
@@ -1714,19 +1736,19 @@ class _Universe {
   static String _canonicalRecipeOfDynamic() => Recipe.pushDynamicString;
   static String _canonicalRecipeOfVoid() => Recipe.pushVoidString;
   static String _canonicalRecipeOfNever() =>
-      Recipe.pushNeverExtensionString + Recipe.extensionOpString;
+      _recipeJoin(Recipe.pushNeverExtensionString, Recipe.extensionOpString);
   static String _canonicalRecipeOfAny() =>
-      Recipe.pushAnyExtensionString + Recipe.extensionOpString;
+      _recipeJoin(Recipe.pushAnyExtensionString, Recipe.extensionOpString);
 
   static String _canonicalRecipeOfStar(Rti baseType) =>
-      Rti._getCanonicalRecipe(baseType) + Recipe.wrapStarString;
+      _recipeJoin(Rti._getCanonicalRecipe(baseType), Recipe.wrapStarString);
   static String _canonicalRecipeOfQuestion(Rti baseType) =>
-      Rti._getCanonicalRecipe(baseType) + Recipe.wrapQuestionString;
+      _recipeJoin(Rti._getCanonicalRecipe(baseType), Recipe.wrapQuestionString);
   static String _canonicalRecipeOfFutureOr(Rti baseType) =>
-      Rti._getCanonicalRecipe(baseType) + Recipe.wrapFutureOrString;
+      _recipeJoin(Rti._getCanonicalRecipe(baseType), Recipe.wrapFutureOrString);
 
   static String _canonicalRecipeOfGenericFunctionParameter(int index) =>
-      '$index' + Recipe.genericFunctionTypeParameterIndexString;
+      _recipeJoin('$index', Recipe.genericFunctionTypeParameterIndexString);
 
   static Rti _lookupErasedRti(Object? universe) {
     return _lookupTerminalRti(
@@ -1888,7 +1910,7 @@ class _Universe {
     for (int i = 0; i < length; i++) {
       Rti argument = _Utils.asRti(_Utils.arrayAt(arguments, i));
       String subrecipe = Rti._getCanonicalRecipe(argument);
-      s += sep + subrecipe;
+      s = _recipeJoin3(s, sep, subrecipe);
       sep = Recipe.separatorString;
     }
     return s;
@@ -1897,17 +1919,16 @@ class _Universe {
   static String _canonicalRecipeJoinNamed(Object? arguments) {
     String s = '', sep = '';
     int length = _Utils.arrayLength(arguments);
-    assert(length % 3 == 0);
+    assert(_Utils.isMultipleOf(length, 3));
     for (int i = 0; i < length; i += 3) {
-      s += sep;
       String name = _Utils.asString(_Utils.arrayAt(arguments, i));
       bool isRequired = _Utils.asBool(_Utils.arrayAt(arguments, i + 1));
       String nameSep = isRequired
           ? Recipe.requiredNameSeparatorString
           : Recipe.nameSeparatorString;
       Rti type = _Utils.asRti(_Utils.arrayAt(arguments, i + 2));
       String subrecipe = Rti._getCanonicalRecipe(type);
-      s += name + nameSep + subrecipe;
+      s = _recipeJoin5(s, sep, name, nameSep, subrecipe);
       sep = Recipe.separatorString;
     }
     return s;
@@ -1918,9 +1939,8 @@ class _Universe {
     String s = _Utils.asString(name);
     int length = _Utils.arrayLength(arguments);
     if (length != 0) {
-      s += Recipe.startTypeArgumentsString +
-          _canonicalRecipeJoin(arguments) +
-          Recipe.endTypeArgumentsString;
+      s = _recipeJoin4(s, Recipe.startTypeArgumentsString,
+          _canonicalRecipeJoin(arguments), Recipe.endTypeArgumentsString);
     }
     return s;
   }
@@ -1954,13 +1974,13 @@ class _Universe {
           JS_GET_NAME(JsGetName.FUTURE_CLASS_TYPE_NAME), JS('', '[#]', base));
 
   static String _canonicalRecipeOfBinding(Rti base, Object? arguments) {
-    String s = Rti._getCanonicalRecipe(base);
-    s += Recipe
-        .toTypeString; // TODO(sra): Omit when base encoding is Rti without ToType.
-    s += Recipe.startTypeArgumentsString +
-        _canonicalRecipeJoin(arguments) +
-        Recipe.endTypeArgumentsString;
-    return s;
+    return _recipeJoin5(
+        Rti._getCanonicalRecipe(base),
+        // TODO(sra): Omit when base encoding is Rti without ToType:
+        Recipe.toTypeString,
+        Recipe.startTypeArgumentsString,
+        _canonicalRecipeJoin(arguments),
+        Recipe.endTypeArgumentsString);
   }
 
   /// [arguments] becomes owned by the created Rti.
@@ -1992,8 +2012,8 @@ class _Universe {
 
   static String _canonicalRecipeOfFunction(
           Rti returnType, _FunctionParameters parameters) =>
-      Rti._getCanonicalRecipe(returnType) +
-      _canonicalRecipeOfFunctionParameters(parameters);
+      _recipeJoin(Rti._getCanonicalRecipe(returnType),
+          _canonicalRecipeOfFunctionParameters(parameters));
 
   static String _canonicalRecipeOfFunctionParameters(
       _FunctionParameters parameters) {
@@ -2007,26 +2027,26 @@ class _Universe {
     int namedLength = _Utils.arrayLength(named);
     assert(optionalPositionalLength == 0 || namedLength == 0);
 
-    String recipe = Recipe.startFunctionArgumentsString +
-        _canonicalRecipeJoin(requiredPositional);
+    String recipe = _recipeJoin(Recipe.startFunctionArgumentsString,
+        _canonicalRecipeJoin(requiredPositional));
 
     if (optionalPositionalLength > 0) {
       String sep = requiredPositionalLength > 0 ? Recipe.separatorString : '';
-      recipe += sep +
-          Recipe.startOptionalGroupString +
-          _canonicalRecipeJoin(optionalPositional) +
-          Recipe.endOptionalGroupString;
+      recipe = _recipeJoin5(
+          recipe,
+          sep,
+          Recipe.startOptionalGroupString,
+          _canonicalRecipeJoin(optionalPositional),
+          Recipe.endOptionalGroupString);
     }
 
     if (namedLength > 0) {
       String sep = requiredPositionalLength > 0 ? Recipe.separatorString : '';
-      recipe += sep +
-          Recipe.startNamedGroupString +
-          _canonicalRecipeJoinNamed(named) +
-          Recipe.endNamedGroupString;
+      recipe = _recipeJoin5(recipe, sep, Recipe.startNamedGroupString,
+          _canonicalRecipeJoinNamed(named), Recipe.endNamedGroupString);
     }
 
-    return recipe + Recipe.endFunctionArgumentsString;
+    return _recipeJoin(recipe, Recipe.endFunctionArgumentsString);
   }
 
   static Rti _lookupFunctionRti(
@@ -2051,10 +2071,11 @@ class _Universe {
 
   static String _canonicalRecipeOfGenericFunction(
           Rti baseFunctionType, Object? bounds) =>
-      Rti._getCanonicalRecipe(baseFunctionType) +
-      Recipe.startTypeArgumentsString +
-      _canonicalRecipeJoin(bounds) +
-      Recipe.endTypeArgumentsString;
+      _recipeJoin4(
+          Rti._getCanonicalRecipe(baseFunctionType),
+          Recipe.startTypeArgumentsString,
+          _canonicalRecipeJoin(bounds),
+          Recipe.endTypeArgumentsString);
 
   static Rti _lookupGenericFunctionRti(
       Object? universe, Rti baseFunctionType, Object? bounds, bool normalize) {
@@ -2552,7 +2573,7 @@ class _Parser {
 
   static void toTypesNamed(Object? universe, Rti environment, Object? items) {
     int length = _Utils.arrayLength(items);
-    assert(length % 3 == 0);
+    assert(_Utils.isMultipleOf(length, 3));
     for (int i = 2; i < length; i += 3) {
       var item = _Utils.arrayAt(items, i);
       Rti type = toType(universe, environment, item);
@@ -2995,6 +3016,7 @@ bool isNullable(Rti t) {
       kind == Rti.kindFutureOr && isNullable(Rti._getFutureOrArgument(t));
 }
 
+@pragma('dart2js:parameter:trust')
 bool isTopType(Rti t) =>
     isStrongTopType(t) ||
     isLegacyObjectType(t) ||
@@ -3040,6 +3062,8 @@ class _Utils {
   static bool isNotIdentical(Object? s, Object? t) =>
       JS('bool', '# !== #', s, t);
 
+  static bool isMultipleOf(int n, int d) => JS('bool', '# % # === 0', n, d);
+
   static JSArray objectKeys(Object? o) =>
       JS('returns:JSArray;new:true;', 'Object.keys(#)', o);
 
@@ -3089,6 +3113,7 @@ class _Utils {
     JS('', '#.set(#, #)', cache, key, value);
   }
 }
+
 // -------- Entry points for testing -------------------------------------------
 
 String testingCanonicalRecipe(Rti rti) {

tests/language/regress/regress41890_test.dart

@@ -0,0 +1,74 @@
+// Copyright (c) 2020, 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=--enable-asserts --disable-inlining --disable-type-inference --no-minify
+
+import 'dart:async';
+import 'package:expect/expect.dart';
+
+// This test contains many different Rti instances that are initialized at
+// startup.
+
+typedef F1 = String Function(String, {int opt1});
+typedef F2 = String Function(String, {required int req1, int opt1});
+
+class Thingy {
+  const Thingy();
+}
+
+class Generic<AA> {
+  const Generic();
+}
+
+bool isObject(o) => o is Object;
+bool isF1(o) => o is F1;
+bool isF2(o) => o is F2;
+bool isThingy(o) => o is Thingy;
+bool isGenericF1(o) => o is Generic<F1>;
+bool isGenericF2Q(o) => o is Generic<F2?>;
+bool isG3(o) => o is FutureOr<AA> Function<AA>(AA);
+
+String foo1(String s) => s;
+String foo2(String s, {int opt1 = 0}) => '$s $opt1';
+String foo3(String s, {int opt1 = 0, required int req1}) => '$s $req1 $opt1';
+Never foo4() => throw 'never';
+
+// TODO(sra): Use 'isStrongMode' from package:expect.
+final bool strong = () {
+  try {
+    int i = null as dynamic;
+    return false;
+  } catch (e) {
+    return true;
+  }
+}();
+
+void test() {
+  var items = [foo1, foo2, foo3, foo4, Thingy(), Generic<F1>()];
+
+  void check(String answers, bool Function(dynamic) predicate) {
+    Expect.equals(items.length, answers.length);
+    for (int i = 0; i < items.length; i++) {
+      var item = items[i];
+      String code = answers[i];
+      bool expected =
+          code == 'T' || (code == 'S' && strong) || (code == 'W' && !strong);
+      Expect.equals(expected, predicate(item), "$predicate '$code' $item");
+    }
+  }
+
+  // T = true, S = true only in strong mode, W = true only in weak mode.
+  check('TTTTTT', isObject);
+  check('.TW...', isF1);
+  check('..T...', isF2);
+  check('....T.', isThingy);
+  check('.....T', isGenericF1);
+  check('......', isGenericF2Q);
+  check('......', isG3);
+}
+
+void main() {
+  test();
+  test();
+}