Eliminate multi-callback structure for LibraryLoader.

pkg/compiler/lib/src/compiler.dart

 // Copyright (c) 2012, 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.
 
 library dart2js.compiler_base;
 
 import 'dart:async' show EventSink, Future;
 
 import '../compiler_new.dart' as api;
+import 'io/source_file.dart' show SourceFile;

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

nit: move this further down to keep the alphabetical order

[Emily Fortuna, 2017/03/24 18:30:19]

removed

 import 'closure.dart' as closureMapping show ClosureTask;
 import 'common/names.dart' show Selectors;
 import 'common/names.dart' show Identifiers, Uris;
 import 'common/resolution.dart'
     show
         ParsingContext,
         Resolution,
         ResolutionWorkItem,
         ResolutionImpact,
         Target;
@@ skipping 24 matching lines @@
 import 'environment.dart';
 import 'id_generator.dart';
 import 'io/source_information.dart' show SourceInformation;
 import 'js_backend/backend.dart' show JavaScriptBackend;
 import 'library_loader.dart'
     show
         ElementScanner,
         LibraryLoader,
         LibraryLoaderTask,
         LoadedLibraries,
-        LibraryLoaderListener,
         LibraryProvider,
         ScriptLoader;
 import 'mirrors_used.dart' show MirrorUsageAnalyzerTask;
 import 'null_compiler_output.dart' show NullCompilerOutput, NullSink;
 import 'options.dart' show CompilerOptions, DiagnosticOptions;
 import 'parser/diet_parser_task.dart' show DietParserTask;
 import 'parser/parser_task.dart' show ParserTask;
 import 'patch_parser.dart' show PatchParserTask;
 import 'resolution/resolution.dart' show ResolverTask;
 import 'resolved_uri_translator.dart';
@@ skipping 12 matching lines @@
     show ResolutionWorldBuilder, CodegenWorldBuilder;
 import 'universe/use.dart' show StaticUse, TypeUse;
 import 'universe/world_impact.dart'
     show ImpactStrategy, WorldImpact, WorldImpactBuilderImpl;
 import 'util/util.dart' show Link;
 import 'world.dart' show ClosedWorld, ClosedWorldRefiner, ClosedWorldImpl;
 
 typedef CompilerDiagnosticReporter MakeReporterFunction(
     Compiler compiler, CompilerOptions options);
 
-abstract class Compiler implements LibraryLoaderListener {
+abstract class Compiler {
   Measurer get measurer;
 
   final IdGenerator idGenerator = new IdGenerator();
   Types types;
   _CompilerCommonElements _commonElements;
   _CompilerElementEnvironment _elementEnvironment;
   CompilerDiagnosticReporter _reporter;
   CompilerResolution _resolution;
   ParsingContext _parsingContext;
 
@@ skipping 108 matching lines @@
       progress = new Stopwatch()..start();
     }
 
     backend = createBackend();
     enqueuer = backend.makeEnqueuer();
 
     tasks = [
       dietParser = new DietParserTask(idGenerator, backend, reporter, measurer),
       scanner = createScannerTask(),
       serialization = new SerializationTask(this),
+      patchParser = new PatchParserTask(this),
       libraryLoader = new LibraryLoaderTask(
           resolvedUriTranslator,
           options.compileOnly
               ? new _NoScriptLoader(this)
               : new _ScriptLoader(this),
           new _ElementScanner(scanner),
           serialization,
-          this,
+          resolvePatchUri,
+          patchParser,
           environment,
           reporter,
           measurer),
       parser = new ParserTask(this),
-      patchParser = new PatchParserTask(this),
       resolver = createResolverTask(),
       closureToClassMapper = new closureMapping.ClosureTask(this),
       checker = new TypeCheckerTask(this),
       globalInference = new GlobalTypeInferenceTask(this),
       constants = backend.constantCompilerTask,
       deferredLoadTask = new DeferredLoadTask(this),
       mirrorUsageAnalyzerTask = new MirrorUsageAnalyzerTask(this),
       // [enqueuer] is created earlier because it contains the resolution world
       // objects needed by other tasks.
       enqueuer,
@@ skipping 71 matching lines @@
 
         return new Future.sync(() => runInternal(uri))
             .catchError((error) => _reporter.onError(uri, error))
             .whenComplete(() {
           measurer.stopWallClock();
         }).then((_) {
           return !compilationFailed;
         });
       });
 
-  /// This method is called immediately after the [LibraryElement] [library] has
-  /// been created.
-  ///
-  /// Use this callback method to store references to specific libraries.
-  /// Note that [library] has not been scanned yet, nor has its imports/exports
-  /// been resolved.
-  void onLibraryCreated(LibraryElement library) {
-    _commonElements.onLibraryCreated(library);
-  }
-
-  /// This method is called immediately after the [library] and its parts have
-  /// been scanned.
-  ///
-  /// Use this callback method to store references to specific member declared
-  /// in certain libraries. Note that [library] has not been patched yet, nor
-  /// has its imports/exports been resolved.
-  ///
-  /// Use [loader] to register the creation and scanning of a patch library
-  /// for [library].
-  Future onLibraryScanned(LibraryElement library, LibraryLoader loader) {
-    return backend.onLibraryScanned(library, loader);
-  }
-
   /// Compute the set of distinct import chains to the library at [uri] within
   /// [loadedLibraries].
   ///
   /// The chains are strings of the form
   ///
   ///       <main-uri> => <intermediate-uri1> => <intermediate-uri2> => <uri>
   ///
   Set<String> computeImportChainsFor(LoadedLibraries loadedLibraries, Uri uri) {
     // TODO(johnniwinther): Move computation of dependencies to the library
     // loader.
-    Uri rootUri = loadedLibraries.rootUri;
     Set<String> importChains = new Set<String>();
     // The maximum number of full imports chains to process.
     final int chainLimit = 10000;
     // The maximum number of imports chains to show.
     final int compactChainLimit = options.verbose ? 20 : 10;
     int chainCount = 0;
     loadedLibraries.forEachImportChain(uri,
         callback: (Link<Uri> importChainReversed) {
       Link<CodeLocation> compactImportChain = const Link<CodeLocation>();
       CodeLocation currentCodeLocation =
           new UriLocation(importChainReversed.head);
       compactImportChain = compactImportChain.prepend(currentCodeLocation);
       for (Link<Uri> link = importChainReversed.tail;
           !link.isEmpty;
           link = link.tail) {
         Uri uri = link.head;
         if (!currentCodeLocation.inSameLocation(uri)) {
           currentCodeLocation =
               options.verbose ? new UriLocation(uri) : new CodeLocation(uri);
           compactImportChain = compactImportChain.prepend(currentCodeLocation);
         }
       }
       String importChain = compactImportChain.map((CodeLocation codeLocation) {
-        return codeLocation.relativize(rootUri);
+        return codeLocation
+            .relativize(loadedLibraries.rootLibrary.canonicalUri);
       }).join(' => ');
 
       if (!importChains.contains(importChain)) {
         if (importChains.length > compactChainLimit) {
           importChains.add('...');
           return false;
         } else {
           importChains.add(importChain);
         }
       }
@@ skipping 10 matching lines @@
   }
 
   /// This method is called when all new libraries loaded through
   /// [LibraryLoader.loadLibrary] has been loaded and their imports/exports
   /// have been computed.
   ///
   /// [loadedLibraries] contains the newly loaded libraries.
   ///
   /// The method returns a [Future] allowing for the loading of additional
   /// libraries.
-  Future onLibrariesLoaded(LoadedLibraries loadedLibraries) {
-    return new Future.sync(() {
-      for (Uri uri in resolvedUriTranslator.disallowedLibraryUris) {
-        if (loadedLibraries.containsLibrary(uri)) {
-          Set<String> importChains =
-              computeImportChainsFor(loadedLibraries, uri);
-          reporter.reportInfo(
-              NO_LOCATION_SPANNABLE, MessageKind.DISALLOWED_LIBRARY_IMPORT, {
-            'uri': uri,
-            'importChain': importChains
-                .join(MessageTemplate.DISALLOWED_LIBRARY_IMPORT_PADDING)
-          });
-        }
+  LoadedLibraries processLoadedLibraries(LoadedLibraries loadedLibraries) {
+    loadedLibraries.forEachLibrary((LibraryElement library) {
+      _commonElements.registerCommonLibraries(library);
+      backend.setAnnotations(library);
+    });
+
+    for (Uri uri in resolvedUriTranslator.disallowedLibraryUris) {
+      if (loadedLibraries.containsLibrary(uri)) {
+        Set<String> importChains = computeImportChainsFor(loadedLibraries, uri);
+        reporter.reportInfo(
+            NO_LOCATION_SPANNABLE, MessageKind.DISALLOWED_LIBRARY_IMPORT, {
+          'uri': uri,
+          'importChain': importChains
+              .join(MessageTemplate.DISALLOWED_LIBRARY_IMPORT_PADDING)
+        });
       }
+    }
 
-      if (!loadedLibraries.containsLibrary(Uris.dart_core)) {
-        return null;
-      }
-
+    if (loadedLibraries.containsLibrary(Uris.dart_core)) {
       bool importsMirrorsLibrary =
           loadedLibraries.containsLibrary(Uris.dart_mirrors);
       if (importsMirrorsLibrary && !backend.supportsReflection) {
         Set<String> importChains =
             computeImportChainsFor(loadedLibraries, Uris.dart_mirrors);
         reporter.reportErrorMessage(NO_LOCATION_SPANNABLE,
             MessageKind.MIRRORS_LIBRARY_NOT_SUPPORT_BY_BACKEND, {
           'importChain': importChains
               .join(MessageTemplate.MIRRORS_NOT_SUPPORTED_BY_BACKEND_PADDING)
         });
       } else if (importsMirrorsLibrary && !options.enableExperimentalMirrors) {
         Set<String> importChains =
             computeImportChainsFor(loadedLibraries, Uris.dart_mirrors);
         reporter.reportWarningMessage(
             NO_LOCATION_SPANNABLE, MessageKind.IMPORT_EXPERIMENTAL_MIRRORS, {
           'importChain': importChains
               .join(MessageTemplate.IMPORT_EXPERIMENTAL_MIRRORS_PADDING)
         });
       }
-    }).then((_) => backend.onLibrariesLoaded(loadedLibraries));
+    }
+    backend.onLibrariesLoaded(loadedLibraries);
+    return loadedLibraries;
   }
 
   // TODO(johnniwinther): Move this to [PatchParser] when it is moved to the
   // [JavaScriptBackend]. Currently needed for testing.
   String get patchVersion => backend.patchVersion;
 
   /**
    * Get an [Uri] pointing to a patch for the dart: library with
    * the given path. Returns null if there is no patch.
    */
   Uri resolvePatchUri(String dartLibraryPath);
 
-  Future runInternal(Uri uri) {
+  Future runInternal(Uri uri) async {
     // TODO(ahe): This prevents memory leaks when invoking the compiler
     // multiple times. Implement a better mechanism where we can store
     // such caches in the compiler and get access to them through a
     // suitably maintained static reference to the current compiler.
     StringToken.canonicalizer.clear();
     Selector.canonicalizedValues.clear();
 
     // The selector objects held in static fields must remain canonical.
     for (Selector selector in Selectors.ALL) {
       Selector.canonicalizedValues
           .putIfAbsent(selector.hashCode, () => <Selector>[])
           .add(selector);
     }
 
     assert(uri != null || options.analyzeOnly);
-    return new Future.sync(() {
-      if (librariesToAnalyzeWhenRun != null) {
-        return Future.forEach(librariesToAnalyzeWhenRun, (libraryUri) {
-          reporter.log('Analyzing $libraryUri (${options.buildId})');
-          return libraryLoader.loadLibrary(libraryUri);
-        });
+    // As far as I can tell, this branch is only used by test code.
+    if (librariesToAnalyzeWhenRun != null) {
+      await Future.forEach(librariesToAnalyzeWhenRun, (libraryUri) async {
+        reporter.log('Analyzing $libraryUri (${options.buildId})');
+        LoadedLibraries loadedLibraries =
+            await libraryLoader.loadLibrary(libraryUri);
+        processLoadedLibraries(loadedLibraries);
+      });
+    }
+    if (uri != null) {
+      if (options.analyzeOnly) {
+        reporter.log('Analyzing $uri (${options.buildId})');
+      } else {
+        reporter.log('Compiling $uri (${options.buildId})');
       }
-    }).then((_) {
-      if (uri != null) {
-        if (options.analyzeOnly) {
-          reporter.log('Analyzing $uri (${options.buildId})');
-        } else {
-          reporter.log('Compiling $uri (${options.buildId})');
-        }
-        return libraryLoader.loadLibrary(uri).then((LibraryElement library) {
-          mainApp = library;
-        });
-      }
-    }).then((_) {
-      compileLoadedLibraries();
-    });
+      LoadedLibraries libraries = await libraryLoader.loadLibrary(uri);
+      processLoadedLibraries(libraries);
+      mainApp = libraries.rootLibrary;
+    }
+    compileLoadedLibraries();
   }
 
   WorldImpact computeMain() {
     if (mainApp == null) return const WorldImpact();
 
     WorldImpactBuilderImpl impactBuilder = new WorldImpactBuilderImpl();
     Element main = mainApp.findExported(Identifiers.main);
     ErroneousElement errorElement = null;
     if (main == null) {
       if (options.analyzeOnly) {
@@ skipping 56 matching lines @@
   }
 
   /// Analyze all members of the library in [libraryUri].
   ///
   /// If [skipLibraryWithPartOfTag] is `true`, member analysis is skipped if the
   /// library has a `part of` tag, assuming it is a part and not a library.
   ///
   /// This operation assumes an unclosed resolution queue and is only supported
   /// when the '--analyze-main' option is used.
   Future<LibraryElement> analyzeUri(Uri libraryUri,
-      {bool skipLibraryWithPartOfTag: true}) {
+      {bool skipLibraryWithPartOfTag: true}) async {
     assert(options.analyzeMain);
     reporter.log('Analyzing $libraryUri (${options.buildId})');
-    return libraryLoader
-        .loadLibrary(libraryUri, skipFileWithPartOfTag: true)
-        .then((LibraryElement library) {
-      if (library == null) return null;
+    LoadedLibraries loadedLibraries = await libraryLoader
+        .loadLibrary(libraryUri, skipFileWithPartOfTag: true);
+    if (loadedLibraries != null) {

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

minor nit - we tend to prefer the fast-exit pattern that was here before:

if (loadedLibraries == null) return null;

processLoadedLIbraries(loadedLibraries);
...
return library;

[Emily Fortuna, 2017/03/24 18:30:19]

Done.

+      processLoadedLibraries(loadedLibraries);
+      LibraryElement library = loadedLibraries.rootLibrary;

[Siggi Cherem (dart-lang), 2017/03/24 17:40:27]

is it safe to say that the root library is not null at this point, or do we need
to check for that too?

[Emily Fortuna, 2017/03/24 18:30:19]

As the code is currently written if you receive a LoadedLibraries object, its
rootLibrary cannot be null. I'll add an assert in the constructor though.

       ResolutionEnqueuer resolutionEnqueuer = startResolution();
       resolutionEnqueuer.applyImpact(computeImpactForLibrary(library));
       emptyQueue(resolutionEnqueuer, onProgress: showResolutionProgress);
       resolutionEnqueuer.logSummary(reporter.log);
       return library;
-    });
+    }
+    return null;
   }
 
   /// Starts the resolution phase, creating the [ResolutionEnqueuer] if not
   /// already created.
   ///
   /// During normal compilation resolution only started once, but through
   /// [analyzeUri] resolution is started repeatedly.
   ResolutionEnqueuer startResolution() {
     ResolutionEnqueuer resolutionEnqueuer;
     if (enqueuer.hasResolution) {
@@ skipping 524 matching lines @@
   LibraryElement coreLibrary;
   LibraryElement asyncLibrary;
   LibraryElement mirrorsLibrary;
   LibraryElement typedDataLibrary;
 
   _CompilerCommonElements(this.environment, this.resolution, this.reporter);
 
   @override
   ResolutionDynamicType get dynamicType => const ResolutionDynamicType();
 
-  void onLibraryCreated(LibraryElement library) {
+  void registerCommonLibraries(LibraryElement library) {
     Uri uri = library.canonicalUri;
     if (uri == Uris.dart_core) {
       coreLibrary = library;
     } else if (uri == Uris.dart_async) {
       asyncLibrary = library;
     } else if (uri == Uris.dart__native_typed_data) {
       typedDataLibrary = library;
     } else if (uri == Uris.dart_mirrors) {
       mirrorsLibrary = library;
     }
@@ skipping 785 matching lines @@
     compiler.reporter
         .internalError(spannable, "Script loading of '$uri' is not enabled.");
   }
 }
 
 class _ElementScanner implements ElementScanner {
   ScannerTask scanner;
   _ElementScanner(this.scanner);
   void scanLibrary(LibraryElement library) => scanner.scanLibrary(library);
   void scanUnit(CompilationUnitElement unit) => scanner.scan(unit);
+  Token scanFile(SourceFile file) => scanner.scanFile(file);
 }
 
 class _EmptyEnvironment implements Environment {
   const _EmptyEnvironment();
 
   String valueOf(String key) => null;
 }
 
 /// An element environment base on a [Compiler].
 class _CompilerElementEnvironment implements ElementEnvironment {
@@ skipping 117 matching lines @@
     if (library != null && library.isSynthesized) {
       return null;
     }
     if (library == null && required) {
       throw new SpannableAssertionFailure(
           library, "The library '${uri}' was not found.");
     }
     return library;
   }
 }