[ddc] Update source mapping for static invocations

when they have been rewritten to include hot reload checks. Appears to
fix the issue detected in the dwds test failure.

Issue: dart-lang/webdev#2617.

The new mapping ensures breakpoints applied to the call site in the Dart
source are the call site instead of the the generation check.

Allows the tooling to set a breakpoint on the static method call site,
and then when paused, a step-into will action will advance the program
into the static method.

In the future we will need to reevaluate how we generate code where a
single branch of execution in Dart source produces multiple branches in
JavaScript. I believe right now the tooling will not support creating
breakpoints in both branches even when they both have a mapping to the
same Dart source location.

Change-Id: I1ea0548768d8213dc7eefc332814d3bda7c81535
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427567
Reviewed-by: Nate Biggs <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>
Reviewed-by: Srujan Gaddam <[email protected]>

Author: nshahan

pkg/dev_compiler/lib/src/js_ast/nodes.dart

@@ -1213,6 +1213,13 @@ class ArrayBindingPattern extends BindingPattern {
   ArrayBindingPattern _clone() => ArrayBindingPattern(variables);
 }
 
+/// Entirely transparent wrapper only used to identify the node as being a
+/// rewritten invocation that includes correctness checks in case of a hot
+/// reload at runtime.
+class InvocationWithHotReloadChecks extends Conditional {
+  InvocationWithHotReloadChecks(super.condition, super.then, super.otherwise);
+}
+
 class Conditional extends Expression {
   final Expression condition;
   final Expression then;

pkg/dev_compiler/lib/src/js_ast/printer.dart

@@ -44,6 +44,12 @@ abstract class JavaScriptPrintingContext {
   /// Callback after printing the last character representing [node].
   void exitNode(Node node) {}
 
+  // Callback before branching the flow of code in an expression.
+  void expressionBranch() {}
+
+  // Callback after returning to the original flow of code in an expression.
+  void expressionJoin() {}
+
   late Printer printer;
 }
 
@@ -757,11 +763,13 @@ class Printer implements NodeVisitor {
     // The then part is allowed to have an 'in'.
     visitNestedExpression(cond.then, ASSIGNMENT,
         newInForInit: false, newAtStatementBegin: false);
+    context.expressionBranch();
     spaceOut();
     out(':');
     spaceOut();
     visitNestedExpression(cond.otherwise, ASSIGNMENT,
         newInForInit: inForInit, newAtStatementBegin: false);
+    context.expressionJoin();
   }
 
   @override

pkg/dev_compiler/lib/src/js_ast/source_map_printer.dart

@@ -139,6 +139,22 @@ class SourceMapPrintingContext extends SimpleJavaScriptPrintingContext {
     }
   }
 
+  @override
+  void expressionBranch() {
+    // Only one branch of a conditional expression will be executed but the
+    // expressions could be mapped to the same source location. To ensure
+    // mappings are produced for both branches any pending locations should be
+    // flushed when switching output from one branch to the other.
+    _flushPendingMarks();
+  }
+
+  @override
+  void expressionJoin() {
+    // To ensure mappings get output for all branches when nesting conditional
+    // expressions they should be flushed after code flow joins after branching.
+    _flushPendingMarks();
+  }
+
   /// Marks that we entered a Dart node at this offset.
   ///
   /// Multiple nested Dart expressions will appear to start at the same Dart

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

@@ -4582,6 +4582,14 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         return _emitDebuggerCall(expr).toStatement();
       }
     }
+    var jsExpression = _visitExpression(expr);
+    if (jsExpression is js_ast.InvocationWithHotReloadChecks) {
+      // When the static invocation expression has been rewritten to include
+      // hot reload correctness checks the entire resulting node shouldn't
+      // get source-mapped to the original call. Instead the call sites in
+      // the sub-expressions will have the correct mapping applied.
+      return jsExpression.toStatement()..sourceInformation = continueSourceMap;
+    }
     return _visitExpression(expr).toStatement();
   }
 
@@ -6795,26 +6803,38 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
     var fn = _emitStaticTarget(target);
     var args = _emitArgumentList(node.arguments, target: target);
-    var staticCall = js_ast.Call(fn, args);
-    if (_inFunctionExpression &&
-        !usesJSInterop(node.target) &&
-        !_isBuildingSdk) {
+    var staticCall = js_ast.Call(fn, args)
+      ..sourceInformation = _nodeStart(node);
+    if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
+      var generationCheck = js.call('# === #', [
+        js.number(_hotReloadGeneration),
+        _runtimeCall('global.dartDevEmbedder.hotReloadGeneration')
+      ]);
       var checkedCall = _rewriteInvocationWithHotReloadChecks(
-          target, node.arguments, node.getStaticType(_staticTypeContext));
+          target,
+          node.arguments,
+          node.getStaticType(_staticTypeContext),
+          _nodeStart(node));
       // As an optimization, avoid extra checks when the invocation code was
       // compiled in the same generation that it is running.
-      return js.call('# === # ? # : #', [
-        js.number(_hotReloadGeneration),
-        _runtimeCall('global.dartDevEmbedder.hotReloadGeneration'),
-        staticCall,
-        checkedCall
-      ]);
+      return js_ast.InvocationWithHotReloadChecks(
+          generationCheck, staticCall, checkedCall)
+        ..sourceInformation = continueSourceMap;
     }
     return _isNullCheckableJsInterop(target)
         ? _wrapWithJsInteropNullCheck(staticCall)
         : staticCall;
   }
 
+  /// Returns `true` when an invocation of [target] should be rewritten to
+  /// include checks to preserve soundness in the presence of hot reloads at
+  /// runtime.
+  bool _shouldRewriteInvocationWithHotReloadChecks(Member target) =>
+      _inFunctionExpression &&
+      !_isBuildingSdk &&
+      !usesJSInterop(target) &&
+      target != _assertInteropMethod;
+
   /// Rewrites an invocation of [target] that is statically sound at compile
   /// time to include checks to preserve soundness in the presence of hot
   /// reloads at runtime.
@@ -6826,8 +6846,14 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   ///
   /// Invokes `noSuchMethod` if the check fails. If that also returns a value,
   /// it's cast to the [expectedReturnType].
+  ///
+  /// The resulting expression for the validated call site will receive the
+  /// [originalCallSiteSourceLocation].
   js_ast.Expression _rewriteInvocationWithHotReloadChecks(
-      Member target, Arguments arguments, DartType expectedReturnType) {
+      Member target,
+      Arguments arguments,
+      DartType expectedReturnType,
+      SourceLocation? originalCallSiteSourceLocation) {
     var hoistedPositionalVariables = <js_ast.Expression>[];
     var hoistedNamedVariables = <String, js_ast.Expression>{};
     js_ast.Expression? letAssignments;
@@ -6906,7 +6932,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
           for (var e in hoistedNamedVariables.entries)
             js_ast.Property(js.string(e.key), e.value)
         ])
-    ]);
+    ])
+      ..sourceInformation = originalCallSiteSourceLocation;
     // Cast the result of the checked call or the value returned from a
     // `NoSuchMethod` invocation.
     return js_ast.Binary(