Base JavaScript code position computation on JavaScript tracer.

tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart

Index: tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart
diff --git a/tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart b/tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart
index 9cb53e566d977cf1e567ef943f6dc7ee9507ba74..d4f176726c34a54df9c42906906944896d8a2b1b 100644
--- a/tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart
+++ b/tests/compiler/dart2js/sourcemaps/sourcemap_helper.dart
@@ -11,8 +11,10 @@ import 'package:compiler/src/null_compiler_output.dart' show NullSink;
 import 'package:compiler/src/elements/elements.dart';
 import 'package:compiler/src/helpers/helpers.dart';
 import 'package:compiler/src/filenames.dart';
+import 'package:compiler/src/io/code_output.dart';
 import 'package:compiler/src/io/source_file.dart';
 import 'package:compiler/src/io/source_information.dart';
+import 'package:compiler/src/io/position_information.dart';
 import 'package:compiler/src/js/js.dart' as js;
 import 'package:compiler/src/js/js_debug.dart';
 import 'package:compiler/src/js/js_source_mapping.dart';
@@ -21,15 +23,51 @@ import 'package:compiler/src/source_file_provider.dart';
 import '../memory_compiler.dart';
 import '../output_collector.dart';
 
+class SourceFileSink implements EventSink<String> {
+  final String filename;
+  StringBuffer sb = new StringBuffer();
+  SourceFile sourceFile;
+
+  SourceFileSink(this.filename);
+
+  @override
+  void add(String event) {
+    sb.write(event);
+  }
+
+  @override
+  void addError(errorEvent, [StackTrace stackTrace]) {
+    // Ignore.
+  }
+
+  @override
+  void close() {
+    sourceFile = new StringSourceFile.fromName(filename, sb.toString());
+  }
+}
+
 class OutputProvider implements CompilerOutput {
-  BufferedEventSink jsMapOutput;
+  Map<Uri, SourceFileSink> outputMap = <Uri, SourceFileSink>{};
+
+  SourceFile getSourceFile(Uri uri) {
+    SourceFileSink sink = outputMap[uri];
+    if (sink != null) {
+      return sink.sourceFile;
+    }
+    return null;
+  }
+
+  SourceFileSink createSourceFileSink(String name, String extension) {
+    String filename = '$name.$extension';
+    SourceFileSink sink = new SourceFileSink(filename);
+    Uri uri = Uri.parse(filename);
+    outputMap[uri] = sink;
+    return sink;
+  }
 
   @override
   EventSink<String> createEventSink(String name, String extension) {
-    if (extension == 'js.map') {
-      return jsMapOutput = new BufferedEventSink();
-    }
-    return new NullSink('$name.$extension');
+    return createSourceFileSink(name, extension);
   }
 }
 
@@ -42,11 +80,8 @@ class CloningOutputProvider extends OutputProvider {
   @override
   EventSink<String> createEventSink(String name, String extension) {
     EventSink<String> output = outputProvider(name, extension);
-    if (extension == 'js.map') {
-      output = new CloningEventSink(
-          [output, jsMapOutput = new BufferedEventSink()]);
-    }
-    return output;
+    return new CloningEventSink(
+        [output, createSourceFileSink(name, extension)]);
   }
 }
 
@@ -56,17 +91,23 @@ abstract class SourceFileManager {
 
 class ProviderSourceFileManager implements SourceFileManager {
   final SourceFileProvider sourceFileProvider;
+  final OutputProvider outputProvider;
 
-  ProviderSourceFileManager(this.sourceFileProvider);
+  ProviderSourceFileManager(this.sourceFileProvider, this.outputProvider);
 
   @override
   SourceFile getSourceFile(uri) {
-    return sourceFileProvider.getSourceFile(uri);
+    SourceFile sourceFile = sourceFileProvider.getSourceFile(uri);
+    if (sourceFile == null) {
+      sourceFile = outputProvider.getSourceFile(uri);
+    }
+    return sourceFile;
   }
 }
 
 class RecordingPrintingContext extends LenientPrintingContext {
   CodePositionListener listener;
+  Map<js.Node, CodePosition> codePositions = <js.Node, CodePosition>{};
 
   RecordingPrintingContext(this.listener);
 
@@ -75,11 +116,152 @@ class RecordingPrintingContext extends LenientPrintingContext {
                 int startPosition,
                 int endPosition,
                 int closingPosition) {
+    codePositions[node] =
+        new CodePosition(startPosition, endPosition, closingPosition);
     listener.onPositions(
         node, startPosition, endPosition, closingPosition);
   }
 }
 
+class SourceMapperWrapper implements SourceMapper {

[Siggi Cherem (dart-lang), 2016/01/21 18:18:34]

+ dartdoc, maybe rename to something more descriptive? For instasnce:

/// A [SourceMapper] that records the source locations on each node.
class RecordingSourceMapper implements SourceMapper {
  ...
}

[Johnni Winther, 2016/01/22 15:12:39]

Done.

+  final SourceMapper sourceMapper;
+  final NodeToSourceLocationRecorder nodeToSourceLocationsMap;
+
+  SourceMapperWrapper(this.sourceMapper, this.nodeToSourceLocationsMap);
+
+  @override
+  void register(js.Node node, int codeOffset, SourceLocation sourceLocation) {
+    nodeToSourceLocationsMap.register(node, codeOffset, sourceLocation);
+    sourceMapper.register(node, codeOffset, sourceLocation);
+  }
+}
+
+class SourceInformationProcessorWrapper implements SourceInformationProcessor {
+  final SourceInformationStrategyWrapper wrapper;
+  final SourceInformationProcessor processor;
+  final CodePositionRecorder codePositions;
+  final NodeToSourceLocationsMap nodeToSourceLocationsMap;
+
+  SourceInformationProcessorWrapper(
+      this.wrapper,
+      this.processor,
+      this.codePositions,
+      this.nodeToSourceLocationsMap);
+
+  @override
+  void onPositions(js.Node node,
+                   int startPosition,
+                   int endPosition,
+                   int closingPosition) {
+    codePositions.registerPositions(
+        node, startPosition, endPosition, closingPosition);
+    processor.onPositions(node, startPosition, endPosition, closingPosition);
+  }
+
+  @override
+  void process(js.Node node, BufferedCodeOutput code) {
+    processor.process(node, code);
+    wrapper.registerProcess(
+        node, code, codePositions, nodeToSourceLocationsMap);
+  }
+}
+
+class SourceInformationSubProcess {

[Siggi Cherem (dart-lang), 2016/01/21 18:18:35]

rename? SubProcess really makes me think that you are about to do a
Process.run/fork somewhere.

[Johnni Winther, 2016/01/22 15:12:39]

Renamed to 'RecordedSourceInformationProcess'.

+  final js.Node root;
+  final String code;
+  final CodePositionRecorder codePositions;
+  final NodeToSourceLocationsMap nodeToSourceLocationsMap;
+
+  SourceInformationSubProcess(
+      this.root,
+      this.code,
+      this.codePositions,
+      this.nodeToSourceLocationsMap);
+}
+
+
+class SourceInformationStrategyWrapper

[Siggi Cherem (dart-lang), 2016/01/21 18:18:35]

it took me some time to figure out what these wrappers are meant to be used for.
It appears that you are trying to record information about each element
separately? So that you can easily report results per element?

If so - maybe a possible naming scheme could be to prefix with `PerElement`?

SourceInformationProcessorWrapper => PerElementSourceInformationProcessor
SourceInformationStrategyWrapper => PerElementSourceInformationStrategy
SourceInformationSubProcess => PerElementRecorder

Since this entire file is about source-information, consider dropping the
SourceInformation piece, or abbreviating it:


PerElementProcessor
PerElementStrategy

[Johnni Winther, 2016/01/22 15:12:38]

It's (only) per element (also) output units. Updated the names to use Record and
Recorded.

+    extends JavaScriptSourceInformationStrategy {
+  final JavaScriptSourceInformationStrategy strategy;
+  final Map<SourceInformationSubProcess, js.Node> processMap =
+      <SourceInformationSubProcess, js.Node>{};
+  final Map<js.Node, SourceInformationSubProcess> nodeMap =
+      <js.Node, SourceInformationSubProcess>{};
+
+  SourceInformationStrategyWrapper(this.strategy);
+
+  @override
+  SourceInformationBuilder createBuilderForContext(AstElement element) {
+    return strategy.createBuilderForContext(element);
+  }
+
+  @override
+  SourceInformationProcessor createProcessor(SourceMapper sourceMapper) {
+    NodeToSourceLocationsMap nodeToSourceLocationsMap =
+        new NodeToSourceLocationRecorder();
+    CodePositionRecorder codePositions = new CodePositionRecorder();
+    return new SourceInformationProcessorWrapper(
+        this,
+        strategy.createProcessor(new SourceMapperWrapper(
+            sourceMapper, nodeToSourceLocationsMap)),
+            codePositions, nodeToSourceLocationsMap);
+  }
+
+  void registerProcess(js.Node root,
+                       BufferedCodeOutput code,
+                       CodePositionRecorder codePositions,
+                       NodeToSourceLocationsMap nodeToSourceLocationsMap) {
+    SourceInformationSubProcess subProcess = new SourceInformationSubProcess(
+        root, code.getText(), codePositions, nodeToSourceLocationsMap);
+    processMap[subProcess] = root;
+  }
+
+  SourceInformationSubProcess subProcessForNode(js.Node node) {
+    return nodeMap.putIfAbsent(node, () {
+      for (SourceInformationSubProcess subProcess in processMap.keys) {
+        js.Node root = processMap[subProcess];
+        FindVisitor visitor = new FindVisitor(node);
+        root.accept(visitor);
+        if (visitor.found) {
+          return new SourceInformationSubProcess(
+              node,
+              subProcess.code,
+              subProcess.codePositions,
+              new NodeToSourceLocationsMapWrapper(
+                  visitor.nodes, subProcess.nodeToSourceLocationsMap));
+        }
+        return null;
+      }
+    });
+  }
+}
+
+class FindVisitor extends js.BaseVisitor {

[Siggi Cherem (dart-lang), 2016/01/21 18:18:34]

+dartdoc:

/// Visitor that collects all nodes that are within a function. Used by the
[PerElementStrategy] to filter what is recorded on a [PerElementRecorder].

+  final js.Node soughtNode;
+  bool found = false;
+  bool add = false;
+  final Set<js.Node> nodes = new Set<js.Node>();
+
+  FindVisitor(this.soughtNode);
+
+  visitNode(js.Node node) {
+    if (node == soughtNode) {
+      found = true;
+      add = true;
+    }
+    if (add) {
+      nodes.add(node);
+    }
+    node.visitChildren(this);
+    if (node == soughtNode) {
+      add = false;
+    }
+  }
+}
+
+const String USE_NEW_SOURCE_INFO =  '--use-new-source-info';
+const String DISABLE_INLINING = '--disable-inlining';
+
 /// Processor that computes [SourceMapInfo] for the JavaScript compiled for a
 /// given Dart file.
 class SourceMapProcessor {
@@ -112,11 +294,12 @@ class SourceMapProcessor {
   /// Computes the [SourceMapInfo] for the compiled elements.
   Future<List<SourceMapInfo>> process(
       List<String> options,
-      {bool verbose: true}) async {
+      {bool verbose: true,
+       bool perElement: true}) async {
     OutputProvider outputProvider = outputToFile
-        ? new OutputProvider()
-        : new CloningOutputProvider(targetUri, sourceMapFileUri);
-    if (options.contains('--use-new-source-info')) {
+        ? new CloningOutputProvider(targetUri, sourceMapFileUri)
+        : new OutputProvider();
+    if (options.contains(USE_NEW_SOURCE_INFO)) {
       if (verbose) print('Using the new source information system.');
       useNewSourceInfo = true;
     }
@@ -125,7 +308,7 @@ class SourceMapProcessor {
         // TODO(johnniwinther): Use [verbose] to avoid showing diagnostics.
         options: ['--out=$targetUri', '--source-map=$sourceMapFileUri']
             ..addAll(options));
-    if (options.contains('--disable-inlining')) {
+    if (options.contains(DISABLE_INLINING)) {
       if (verbose) print('Inlining disabled');
       compiler.disableInlining = true;
     }
@@ -133,30 +316,58 @@ class SourceMapProcessor {
     JavaScriptBackend backend = compiler.backend;
     var handler = compiler.handler;
     SourceFileProvider sourceFileProvider = handler.provider;
-    sourceFileManager = new ProviderSourceFileManager(sourceFileProvider);
+    sourceFileManager = new ProviderSourceFileManager(
+        sourceFileProvider,
+        outputProvider);
+    SourceInformationStrategyWrapper strategy =
+        new SourceInformationStrategyWrapper(backend.sourceInformationStrategy);
+    backend.sourceInformationStrategy = strategy;
     await compiler.run(inputUri);
 
     List<SourceMapInfo> infoList = <SourceMapInfo>[];
-    backend.generatedCode.forEach((Element element, js.Expression node) {
-      js.JavaScriptPrintingOptions options =
-          new js.JavaScriptPrintingOptions();
-      JavaScriptSourceInformationStrategy sourceInformationStrategy =
-          compiler.backend.sourceInformationStrategy;
-      NodeToSourceLocationsMap nodeMap = new NodeToSourceLocationsMap();
-      SourceInformationProcessor sourceInformationProcessor =
-          sourceInformationStrategy.createProcessor(nodeMap);
-      RecordingPrintingContext printingContext =
-          new RecordingPrintingContext(sourceInformationProcessor);
-      new js.Printer(options, printingContext).visit(node);
-      sourceInformationProcessor.process(node);
-
-      String code = printingContext.getText();
+    if (perElement) {
+      backend.generatedCode.forEach((Element element, js.Expression node) {
+        SourceInformationSubProcess subProcess =
+            strategy.subProcessForNode(node);
+        if (subProcess == null) {
+          // TODO(johnniwinther): Find out when this is happening and if it
+          // is benign. (Known to happen for `bool#fromString`)
+          print('No subProcess found for $element');
+          return;
+        }
+        NodeToSourceLocationsMap nodeMap = subProcess.nodeToSourceLocationsMap;
+        String code = subProcess.code;
+        CodePositionRecorder codePositions = subProcess.codePositions;
+        CodePointComputer visitor =
+            new CodePointComputer(sourceFileManager, code, nodeMap);
+        visitor.apply(node);
+        List<CodePoint> codePoints = visitor.codePoints;
+        infoList.add(new SourceMapInfo(
+            element, code, node,
+            codePoints,
+            codePositions/*strategy.codePositions*/,
+            nodeMap));
+      });
+    } else {
+      // TODO(johnniwinther): Supported multiple output units.
+      SourceInformationSubProcess process = strategy.processMap.keys.first;
+      js.Node node = strategy.processMap[process];
+      String code;
+      NodeToSourceLocationsMap nodeMap;
+      CodePositionRecorder codePositions;
+      nodeMap = process.nodeToSourceLocationsMap;
+      code = process.code;
+      codePositions = process.codePositions;
       CodePointComputer visitor =
           new CodePointComputer(sourceFileManager, code, nodeMap);
       visitor.apply(node);
       List<CodePoint> codePoints = visitor.codePoints;
-      infoList.add(new SourceMapInfo(element, code, node, codePoints, nodeMap));
-    });
+      infoList.add(new SourceMapInfo(
+          null, code, node,
+          codePoints,
+          codePositions,
+          nodeMap));
+    }
 
     return infoList;
   }
@@ -167,19 +378,37 @@ class SourceMapInfo {
   final String name;
   final Element element;
   final String code;
-  final js.Expression node;
+  final js.Node node;
   final List<CodePoint> codePoints;
+  final CodePositionMap jsCodePositions;
   final NodeToSourceLocationsMap nodeMap;
 
   SourceMapInfo(
-      Element element, this.code, this.node, this.codePoints, this.nodeMap)
-      : this.name = computeElementNameForSourceMaps(element),
+      Element element,
+      this.code,
+      this.node,
+      this.codePoints,
+      this.jsCodePositions,
+      this.nodeMap)
+      : this.name =
+          element != null ? computeElementNameForSourceMaps(element) : '',
         this.element = element;
+
+  String toString() {
+    return '$name:$element';
+  }
 }
 
 /// Collection of JavaScript nodes with their source mapped target offsets
 /// and source locations.
-class NodeToSourceLocationsMap implements SourceMapper {
+abstract class NodeToSourceLocationsMap {
+  Iterable<js.Node> get nodes;
+
+  Map<int, List<SourceLocation>> operator[] (js.Node node);
+}
+
+class NodeToSourceLocationRecorder
+    implements SourceMapper, NodeToSourceLocationsMap {
   final Map<js.Node, Map<int, List<SourceLocation>>> _nodeMap = {};
 
   @override
@@ -196,6 +425,20 @@ class NodeToSourceLocationsMap implements SourceMapper {
   }
 }
 
+class NodeToSourceLocationsMapWrapper implements NodeToSourceLocationsMap {

[Siggi Cherem (dart-lang), 2016/01/21 18:18:35]

nits/ideas:

- rename to PerElementNodeToSourceLocationsMap or
FilteredNodeToSourceLocationsMap
- consider making it a private class (same with any of the other ones above).
- it might be nice to put factory ctors in ModeToSourceLocationsMap for this and
the recorder, so you can write:

  new NodeToSourceLocationsMap.filter(nodes, map);

- are there words in these names we could get rid of to make it shorter, but
still be precise enough to convey what we need to say?

  It seems to me that "Source" and "NodeTo..." are not really that necessary.
Something like `LocationMap` could be descriptive enough. For example:

   NodeToSourceLocationMap => LocationMap
   NodeToSourceLocationRecorder => LocationRecorder
   NodeToSourceLocationMapWrapper => FilteredLocationMap

+  final Set<js.Node> _nodes;
+  final NodeToSourceLocationsMap map;
+
+  NodeToSourceLocationsMapWrapper(this._nodes, this.map);
+
+  Iterable<js.Node> get nodes => map.nodes.where((n) => _nodes.contains(n));
+
+  Map<int, List<SourceLocation>> operator[] (js.Node node) {
+    return map[node];
+  }
+}
+
+
 /// Visitor that computes the [CodePoint]s for source mapping locations.
 class CodePointComputer extends js.BaseVisitor {
   final SourceFileManager sourceFileManager;