[vm/aot] Keep toString methods on exception classes in toString transformer

Flutter in google3 uses --delete-tostring-package-uri compiler option
to remove toString methods in package:flutter and dart:ui to reduce
size in release mode.
This has unfortunate effect of removing toString methods from exception
classes which may provide valuable information for investigating
problems seen in the wild.

This change adds a new @pragma('flutter:keep-to-string-in-subtypes')
on classes to keep toString methods on all subtypes of the annotated
classes. This pragma is now used on Exception and Error classes in
dart:core.

TEST=pkg/vm/test/transformations/to_string_transformer_test.dart

Issue: flutter/flutter#61562
Change-Id: Ib739c83cdf6b539208f705ba198e63b8bc54fa61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/227920
Reviewed-by: Dan Field <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

pkg/vm/lib/transformations/to_string_transformer.dart

@@ -19,6 +19,8 @@ class ToStringVisitor extends RecursiveVisitor {
   /// 'package:flutter/foundation.dart'.
   final Set<String> _packageUris;
 
+  final Map<Class, bool> _inheritedKeepAnnotations = {};
+
   /// Turn 'dart:ui' into 'dart:ui', or
   /// 'package:flutter/src/semantics_event.dart' into 'package:flutter'.
   String _importUriToPackage(Uri importUri) =>
@@ -29,7 +31,18 @@ class ToStringVisitor extends RecursiveVisitor {
         .contains(_importUriToPackage(node.enclosingLibrary.importUri));
   }
 
-  bool _hasKeepAnnotation(Procedure node) {
+  bool _hasKeepAnnotation(Procedure node) =>
+      _hasPragma(node, 'flutter:keep-to-string');
+
+  bool _hasKeepAnnotationOnClass(Class node) =>
+      _hasPragma(node, 'flutter:keep-to-string-in-subtypes');
+
+  bool _hasInheritedKeepAnnotation(Class node) =>
+      _inheritedKeepAnnotations[node] ??= (_hasKeepAnnotationOnClass(node) ||
+          node.supers
+              .any((Supertype t) => _hasInheritedKeepAnnotation(t.classNode)));
+
+  bool _hasPragma(Annotatable node, String pragma) {
     for (ConstantExpression expression
         in node.annotations.whereType<ConstantExpression>()) {
       if (expression.constant is! InstanceConstant) {
@@ -43,8 +56,7 @@ class ToStringVisitor extends RecursiveVisitor {
         for (var fieldRef in constant.fieldValues.keys) {
           if (fieldRef.asField.name.text == 'name') {
             Constant? name = constant.fieldValues[fieldRef];
-            return name is StringConstant &&
-                name.value == 'flutter:keep-to-string';
+            return name is StringConstant && name.value == pragma;
           }
         }
         return false;
@@ -61,7 +73,8 @@ class ToStringVisitor extends RecursiveVisitor {
         !node.isAbstract &&
         !node.enclosingClass!.isEnum &&
         _isInTargetPackage(node) &&
-        !_hasKeepAnnotation(node)) {
+        !_hasKeepAnnotation(node) &&
+        !_hasInheritedKeepAnnotation(node.enclosingClass!)) {
       node.function.body!.replaceWith(
         ReturnStatement(
           SuperMethodInvocation(

pkg/vm/testcases/transformations/to_string_transformer/lib/main.dart

@@ -5,6 +5,7 @@
 import 'dart:convert';
 
 const keepToString = pragma('flutter:keep-to-string');
+const keepToStringInSubtypes = pragma('flutter:keep-to-string-in-subtypes');
 
 String toString() => 'I am static';
 
@@ -26,9 +27,28 @@ class Keep {
   String toString() => 'I am a Keep';
 }
 
+@keepToStringInSubtypes
+class Base1 {}
+
+class Base2 extends Base1 {}
+
+class Base3 extends Object with Base2 {}
+
+class KeepInherited implements Base3 {
+  @override
+  String toString() => 'Heir';
+}
+
+class MyException implements Exception {
+  @override
+  String toString() => 'A very detailed message';
+}
+
 void main() {
   final IFoo foo = Foo();
   print(foo.toString());
   print(Keep().toString());
   print(FooEnum.B.toString());
+  print(KeepInherited().toString());
+  print(MyException().toString());
 }

pkg/vm/testcases/transformations/to_string_transformer/not_transformed.expect

@@ -39,14 +39,54 @@ class Keep extends core::Object {
   method toString() → core::String
     return "I am a Keep";
 }
+@#C16
+class Base1 extends core::Object {
+  synthetic constructor •() → self::Base1
+    : super core::Object::•()
+    ;
+}
+class Base2 extends self::Base1 {
+  synthetic constructor •() → self::Base2
+    : super self::Base1::•()
+    ;
+}
+abstract class _Base3&Object&Base2 extends core::Object implements self::Base2 /*isAnonymousMixin,isEliminatedMixin,hasConstConstructor*/  {
+  const synthetic constructor •() → self::_Base3&Object&Base2
+    : super core::Object::•()
+    ;
+}
+class Base3 extends self::_Base3&Object&Base2 {
+  synthetic constructor •() → self::Base3
+    : super self::_Base3&Object&Base2::•()
+    ;
+}
+class KeepInherited extends core::Object implements self::Base3 {
+  synthetic constructor •() → self::KeepInherited
+    : super core::Object::•()
+    ;
+  @#C1
+  method toString() → core::String
+    return "Heir";
+}
+class MyException extends core::Object implements core::Exception {
+  synthetic constructor •() → self::MyException
+    : super core::Object::•()
+    ;
+  @#C1
+  method toString() → core::String
+    return "A very detailed message";
+}
 static const field core::pragma keepToString = #C14;
+static const field core::pragma keepToStringInSubtypes = #C16;
 static method toString() → core::String
   return "I am static";
 static method main() → void {
   final self::IFoo foo = new self::Foo::•();
   core::print(foo.{self::IFoo::toString}(){() → core::String});
   core::print(new self::Keep::•().{self::Keep::toString}(){() → core::String});
   core::print(#C7.{self::FooEnum::toString}(){() → core::String});
+  core::print(new self::KeepInherited::•().{self::KeepInherited::toString}(){() → core::String});
+  core::print(new self::MyException::•().{self::MyException::toString}(){() → core::String});
 }
 constants  {
   #C1 = core::_Override {}
@@ -63,4 +103,6 @@ constants  {
   #C12 = "flutter:keep-to-string"
   #C13 = null
   #C14 = core::pragma {name:#C12, options:#C13}
+  #C15 = "flutter:keep-to-string-in-subtypes"
+  #C16 = core::pragma {name:#C15, options:#C13}
 }

pkg/vm/testcases/transformations/to_string_transformer/transformed.expect

@@ -39,14 +39,54 @@ class Keep extends core::Object {
   method toString() → core::String
     return "I am a Keep";
 }
+@#C16
+class Base1 extends core::Object {
+  synthetic constructor •() → self::Base1
+    : super core::Object::•()
+    ;
+}
+class Base2 extends self::Base1 {
+  synthetic constructor •() → self::Base2
+    : super self::Base1::•()
+    ;
+}
+abstract class _Base3&Object&Base2 extends core::Object implements self::Base2 /*isAnonymousMixin,isEliminatedMixin,hasConstConstructor*/  {
+  const synthetic constructor •() → self::_Base3&Object&Base2
+    : super core::Object::•()
+    ;
+}
+class Base3 extends self::_Base3&Object&Base2 {
+  synthetic constructor •() → self::Base3
+    : super self::_Base3&Object&Base2::•()
+    ;
+}
+class KeepInherited extends core::Object implements self::Base3 {
+  synthetic constructor •() → self::KeepInherited
+    : super core::Object::•()
+    ;
+  @#C1
+  method toString() → core::String
+    return "Heir";
+}
+class MyException extends core::Object implements core::Exception {
+  synthetic constructor •() → self::MyException
+    : super core::Object::•()
+    ;
+  @#C1
+  method toString() → core::String
+    return "A very detailed message";
+}
 static const field core::pragma keepToString = #C14;
+static const field core::pragma keepToStringInSubtypes = #C16;
 static method toString() → core::String
   return "I am static";
 static method main() → void {
   final self::IFoo foo = new self::Foo::•();
   core::print(foo.{self::IFoo::toString}(){() → core::String});
   core::print(new self::Keep::•().{self::Keep::toString}(){() → core::String});
   core::print(#C7.{self::FooEnum::toString}(){() → core::String});
+  core::print(new self::KeepInherited::•().{self::KeepInherited::toString}(){() → core::String});
+  core::print(new self::MyException::•().{self::MyException::toString}(){() → core::String});
 }
 constants  {
   #C1 = core::_Override {}
@@ -63,4 +103,6 @@ constants  {
   #C12 = "flutter:keep-to-string"
   #C13 = null
   #C14 = core::pragma {name:#C12, options:#C13}
+  #C15 = "flutter:keep-to-string-in-subtypes"
+  #C16 = core::pragma {name:#C15, options:#C13}
 }

runtime/docs/pragmas.md

@@ -41,3 +41,14 @@ Additionally, they are categorized into "safe" and "unsafe" forms: "safe" pragma
 | Pragma | Meaning |
 | --- | --- |
 | `vm:testing.unsafe.trace-entrypoints-fn` | [Observing which flow-graph-level entry-point was used when a function was called](compiler/frontend/testing_trace_entrypoints_pragma.md) |
+
+## Flutter toString transformer pragmas
+
+These pragmas are useful to exclude certain toString methods from toString transformation,
+which is enabled with `--delete-tostring-package-uri` option in kernel compilers and
+used by Flutter to remove certain toString methods in release mode to reduce size.
+
+| Pragma | Meaning |
+| --- | --- |
+| `flutter:keep-to-string` | Avoid transforming the annotated toString method. |
+| `flutter:keep-to-string-in-subtypes` | Avoid transforming toString methods in all subtypes of the annotated class. |

sdk/lib/core/errors.dart

@@ -64,6 +64,7 @@ part of dart.core;
 /// For example, the [String.contains] method will use a [RangeError]
 /// if its `startIndex` isn't in the range `0..length`,
 /// which is easily created by `RangeError.range(startIndex, 0, length)`.
+@pragma('flutter:keep-to-string-in-subtypes')
 class Error {
   Error(); // Prevent use as mixin.
 

sdk/lib/core/exceptions.dart

@@ -16,6 +16,7 @@ part of dart.core;
 /// is discouraged in library code since it doesn't give users a precise
 /// type they can catch. It may be reasonable to use instances of this
 /// class in tests or during development.
+@pragma('flutter:keep-to-string-in-subtypes')
 abstract class Exception {
   factory Exception([var message]) => _Exception(message);
 }