[dart2js] Annotation for exception stacks with shorter prefix

Dart's `throw` expression does not correspond exactly to JavaScript's `throw` statement. To implement Dart semantics, and to reduce code size, dart2js uses the functions `wrapException` and `throwExpression` from the dart2js runtime. As a result, these functions appear in the stack trace (one or both).

A consequence of this is that a fixed prefix of an error stack is less useful, as potentially interesting frames are forced out of the prefix by these 'noise' frames.

This change ensures only one of the helpers on the stack trace.

It is also possible to get rid of the helper frame by an annotation.

The annotation `@pragma('dart2js:stack-starts-at-throw')` causes the stack to be captured in the current JavaScript function rather than in a helper. The cost is more code at the call site, about 12 bytes per `throw` expression in minified code. The annotation can be placed on a method, class or library, and applies to all `throw` expression in the scope of the annotated element.

This change uses the annotation to remove noise frames from type errors and some other errors in the runtime.

Change-Id: If15184a5963fb054199177bb4526b32f25e53fe9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406684
Reviewed-by: Mayank Patke <[email protected]>
Reviewed-by: Nate Biggs <[email protected]>

Author: rakudrama

pkg/compiler/doc/pragmas.md

@@ -12,6 +12,7 @@
 | `dart2js:noElision` | Disables an optimization whereby unused fields or unused parameters are removed |
 | `dart2js:load-priority` | [Affects deferred library loading](#load-priority) |
 | `dart2js:resource-identifier` | [Collects data references to resources](resource_identifiers.md) |
+| `dart2js:stack-starts-at-throw` | [Affects stack trace from `throw` expressions](#stack-statrs-at-throw) |
 | `weak-tearoff-reference` | [Declaring a static weak reference intrinsic method.](#declaring-a-static-weak-reference-intrinsic-method) |
 
 ## Unsafe pragmas for general use
@@ -214,6 +215,21 @@ In the future this annotation might be extended to apply to `late` local
 variables, static variables, and top-level variables.
 
 
+#### Stack starts at throw
+
+In order to generate smaller code, `throw` expressions are usually implemented
+by calling a helper method to capture the stack trace and perform the actual
+throw. This adds one (or occasionally two) frames to the stack trace showing the
+helper functions. It is possible for the stack trace to be captured directly in
+the method containing the `throw` expression. This takes a little extra code, so
+is opt-in via the following annotation.
+
+```dart
+@pragma('dart2js:stack-starts-at-throw')
+```
+
+This annotation can be placed on a method, class or library.
+
 ### Annotations related to deferred library loading
 
 #### Load priority

pkg/compiler/lib/src/common/elements.dart

@@ -898,6 +898,9 @@ abstract class CommonElements {
 
   FunctionEntity get stringInterpolationHelper => _findHelperFunction('S');
 
+  FunctionEntity get initializeExceptionWrapper =>
+      _findHelperFunction('initializeExceptionWrapper');
+
   FunctionEntity get wrapExceptionHelper =>
       _findHelperFunction('wrapException');
 

pkg/compiler/lib/src/js_backend/annotations.dart

@@ -91,7 +91,9 @@ enum PragmaAnnotation {
   lateCheck('late:check'),
 
   loadLibraryPriority('load-priority', hasOption: true),
-  resourceIdentifier('resource-identifier');
+  resourceIdentifier('resource-identifier'),
+
+  throwWithoutHelperFrame('stack-starts-at-throw');
 
   final String name;
   final bool forFunctionsOnly;
@@ -357,6 +359,11 @@ abstract class AnnotationsData {
 
   /// Determines whether [member] is annotated as a resource identifier.
   bool methodIsResourceIdentifier(FunctionEntity member);
+
+  /// Is this node in a context requesting that the captured stack in a `throw`
+  /// expression generates extra code to avoid having a runtime helper on the
+  /// stack?
+  bool throwWithoutHelperFrame(ir.TreeNode node);
 }
 
 class AnnotationsDataImpl implements AnnotationsData {
@@ -645,6 +652,22 @@ class AnnotationsDataImpl implements AnnotationsData {
     }
     return false;
   }
+
+  @override
+  bool throwWithoutHelperFrame(ir.TreeNode node) {
+    return _throwWithoutHelperFrame(_findContext(node));
+  }
+
+  bool _throwWithoutHelperFrame(DirectivesContext? context) {
+    while (context != null) {
+      EnumSet<PragmaAnnotation>? annotations = context.annotations;
+      if (annotations.contains(PragmaAnnotation.throwWithoutHelperFrame)) {
+        return true;
+      }
+      context = context.parent;
+    }
+    return false;
+  }
 }
 
 class AnnotationsDataBuilder {

pkg/compiler/lib/src/ssa/builder.dart

@@ -2529,7 +2529,14 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
       final sourceInformation = _sourceInformationBuilder.buildThrow(
         node.expression,
       );
-      _closeAndGotoExit(HThrow(pop(), sourceInformation));
+      _closeAndGotoExit(
+        HThrow(
+          pop(),
+          sourceInformation,
+          withoutHelperFrame: closedWorld.annotationsData
+              .throwWithoutHelperFrame(node),
+        ),
+      );
     } else {
       expression.accept(this);
       pop();
@@ -7759,6 +7766,8 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
           pop(),
           _abstractValueDomain.emptyType,
           sourceInformation,
+          withoutHelperFrame: closedWorld.annotationsData
+              .throwWithoutHelperFrame(node),
         ),
       );
       _isReachable = false;

pkg/compiler/lib/src/ssa/codegen.dart

@@ -2994,13 +2994,24 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
       pushStatement(js.Throw(pop()).withSourceInformation(sourceInformation));
     } else {
       use(node.inputs[0]);
-      _pushCallStatic(_commonElements.wrapExceptionHelper, [
-        pop(),
-      ], sourceInformation);
+      if (node.withoutHelperFrame) {
+        _pushCallStatic(_commonElements.initializeExceptionWrapper, [
+          pop(),
+          _newErrorObject(sourceInformation),
+        ], sourceInformation);
+      } else {
+        _pushCallStatic(_commonElements.wrapExceptionHelper, [
+          pop(),
+        ], sourceInformation);
+      }
       pushStatement(js.Throw(pop()).withSourceInformation(sourceInformation));
     }
   }
 
+  js.Expression _newErrorObject(SourceInformation? sourceInformation) {
+    return js.js('new Error()').withSourceInformation(sourceInformation);
+  }
+
   @override
   void visitAwait(HAwait node) {
     use(node.inputs[0]);
@@ -3185,6 +3196,7 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
     use(node.inputs[0]);
     _pushCallStatic(_commonElements.throwExpressionHelper, [
       pop(),
+      if (node.withoutHelperFrame) _newErrorObject(node.sourceInformation),
     ], node.sourceInformation);
   }
 

pkg/compiler/lib/src/ssa/nodes.dart

@@ -3417,11 +3417,13 @@ class HReturn extends HControlFlow {
 }
 
 class HThrowExpression extends HInstruction {
+  final bool withoutHelperFrame;
   HThrowExpression(
     super.value,
     super.type,
-    SourceInformation? sourceInformation,
-  ) : super._oneInput() {
+    SourceInformation? sourceInformation, {
+    this.withoutHelperFrame = false,
+  }) : super._oneInput() {
     this.sourceInformation = sourceInformation;
   }
   @override
@@ -3466,10 +3468,12 @@ class HYield extends HInstruction {
 
 class HThrow extends HControlFlow {
   final bool isRethrow;
+  final bool withoutHelperFrame;
   HThrow(
     HInstruction value,
     SourceInformation? sourceInformation, {
     this.isRethrow = false,
+    this.withoutHelperFrame = false,
   }) {
     inputs.add(value);
     this.sourceInformation = sourceInformation;

pkg/compiler/test/impact/data/expressions.dart

@@ -358,8 +358,11 @@ testAsGenericRaw(dynamic o) => o as GenericClass;
 testAsGenericDynamic(dynamic o) => o as GenericClass<dynamic, dynamic>;
 
 /*member: testThrow:
- static=[throwExpression(1),wrapException(1)],
- type=[inst:JSString]*/
+ static=[
+  throwExpression(2),
+  wrapException(1)],
+ type=[inst:JSString]
+*/
 testThrow() => throw '';
 
 /*member: testIfNotNull:

pkg/compiler/test/impact/data/statements.dart

@@ -424,7 +424,7 @@ testTryFinally() {
 
 /*member: testSwitchWithoutFallthrough:
  static=[
-  throwExpression(1),
+  throwExpression(2),
   wrapException(1)],
  type=[
   inst:JSInt,

pkg/compiler/test/sourcemaps/stacktrace_test.dart

@@ -124,6 +124,7 @@ Future runTest(
 const List<LineException> beforeExceptions = const [
   const LineException('wrapException', 'js_helper.dart'),
   const LineException('throwExpression', 'js_helper.dart'),
+  const LineException('throw_', 'js_helper.dart'),
 ];
 
 /// Lines allowed after the intended stack trace. Typically from the event

sdk/lib/_internal/js_runtime/lib/async_patch.dart

@@ -12,7 +12,6 @@ import 'dart:_js_helper'
         getTraceFromException,
         Primitives,
         requiresPreamble,
-        wrapException,
         unwrapException;
 
 import 'dart:_foreign_helper'

sdk/lib/_internal/js_runtime/lib/core_patch.dart

@@ -23,7 +23,7 @@ import 'dart:_js_helper'
         quoteStringForRegExp,
         getTraceFromException,
         RuntimeError,
-        wrapException,
+        initializeExceptionWrapper,
         wrapZoneUnaryCallback,
         TrustedGetRuntimeType;
 
@@ -273,11 +273,11 @@ class Error {
   StackTrace? get stackTrace => Primitives.extractStackTrace(this);
 
   @patch
+  @pragma('dart2js:never-inline')
   static Never _throw(Object error, StackTrace stackTrace) {
-    error = wrapException(error);
+    error = initializeExceptionWrapper(error, JS('', 'new Error()'));
     JS('void', '#.stack = #', error, stackTrace.toString());
-    JS('', 'throw #', error);
-    throw "unreachable";
+    JS<Never>('', 'throw #', error);
   }
 }
 

sdk/lib/_internal/js_runtime/lib/js_helper.dart

@@ -1100,7 +1100,8 @@ class Primitives {
   static void trySetStackTrace(Error error, StackTrace stackTrace) {
     var jsError = JS('', r'#.$thrownJsError', error);
     if (jsError == null) {
-      jsError = wrapException(error);
+      jsError = JS('', 'new Error()');
+      initializeExceptionWrapper(error, jsError);
       JS('', r'#.$thrownJsError = #', error, jsError);
       JS('void', '#.stack = #', jsError, stackTrace.toString());
     }
@@ -1200,18 +1201,23 @@ String checkString(value) {
   return value;
 }
 
-/// Wrap the given Dart object as a JS `Error` that can carry a stack trace.
+/// Wrap the given Dart object as a JavaScript `Error` that can carry a stack
+/// trace.
 ///
 /// The code in [unwrapException] deals with getting the original Dart
 /// object out of the wrapper again.
 @pragma('dart2js:never-inline')
 wrapException(ex) {
   final wrapper = JS('', 'new Error()');
-  return initializeExceptionWrapper(wrapper, ex);
+  return initializeExceptionWrapper(ex, wrapper);
 }
 
+/// Wrap the given Dart object with the recorded stack trace.
+///
+/// The code in [unwrapException] deals with getting the original Dart
+/// object out of the wrapper again. For convenience, returns [wrapper].
 @pragma('dart2js:never-inline')
-initializeExceptionWrapper(wrapper, ex) {
+initializeExceptionWrapper(ex, wrapper) {
   if (ex == null) ex = TypeError();
   // [unwrapException] looks for the property 'dartException'.
   JS('void', '#.dartException = #', wrapper, ex);
@@ -1240,18 +1246,14 @@ toStringWrapper() {
   return JS('', r'this.dartException').toString();
 }
 
-/// This wraps the exception and does the throw.  It is possible to call this in
-/// a JS expression context, where the throw statement is not allowed.  Helpers
-/// are never inlined, so we don't risk inlining the throw statement into an
-/// expression context.
+/// This wraps the exception and does the throw.  Since this is a function, it
+/// is possible to call this in a JavaScript expression context, where the throw
+/// statement is not allowed.  The wrapper may be passed in, but usually
+/// [throwExpression] is called with one argument, leaving `wrapper` undefined.
 @pragma('dart2js:never-inline')
-Never throwExpression(ex) {
-  // TODO(sra): Manually inline `wrapException` to remove one stack frame.
-  JS<Never>('', 'throw #', wrapException(ex));
-}
-
-Never throwExpressionWithWrapper(ex, wrapper) {
-  JS<Never>('', 'throw #', initializeExceptionWrapper(wrapper, ex));
+Never throwExpression(ex, [wrapper]) {
+  wrapper ??= JS('', 'new Error()');
+  JS<Never>('', 'throw #', initializeExceptionWrapper(ex, wrapper));
 }
 
 throwUnsupportedError(message) {
@@ -1272,8 +1274,7 @@ Never throwUnsupportedOperation(Object o, [Object? operation, Object? verb]) {
   operation ??= 0;
   verb ??= 0;
   final wrapper = JS('', 'Error()');
-  throwExpressionWithWrapper(
-      _diagnoseUnsupportedOperation(o, operation, verb), wrapper);
+  throwExpression(_diagnoseUnsupportedOperation(o, operation, verb), wrapper);
 }
 
 @pragma('dart2js:never-inline')

sdk/lib/_internal/js_runtime/lib/late_helper.dart

@@ -5,43 +5,42 @@
 library _late_helper;
 
 import 'dart:_internal' show LateError, createSentinel, isSentinel;
-import 'dart:_js_helper' show throwExpressionWithWrapper;
 import 'dart:_foreign_helper' show JS;
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwLateFieldNI(String fieldName) {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldNI(fieldName), wrapper);
+  throw LateError.fieldNI(fieldName);
 }
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwLateFieldAI(String fieldName) {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldAI(fieldName), wrapper);
+  throw LateError.fieldAI(fieldName);
 }
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwLateFieldADI(String fieldName) {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldADI(fieldName), wrapper);
+  throw LateError.fieldADI(fieldName);
 }
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwUnnamedLateFieldNI() {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldNI(''), wrapper);
+  throw LateError.fieldNI('');
 }
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwUnnamedLateFieldAI() {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldAI(''), wrapper);
+  throw LateError.fieldAI('');
 }
 
 @pragma('dart2js:never-inline')
+@pragma('dart2js:stack-starts-at-throw')
 void throwUnnamedLateFieldADI() {
-  final wrapper = JS('', 'new Error()');
-  throwExpressionWithWrapper(LateError.fieldADI(''), wrapper);
+  throw LateError.fieldADI('');
 }
 
 /// A boxed variable used for lowering uninitialized `late` variables when they