Store ResolvedAst on AstElement

tests/compiler/dart2js/serialization/helper.dart

 // Copyright (c) 2016, 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.serialization_helper;
 
 import 'dart:async';
 import 'dart:io';
 
 import 'package:compiler/src/commandline_options.dart';
@@ skipping 213 matching lines @@
           script.file = newScript.file;
           _resolvedAstDeserializer.sourceFiles[script.resourceUri] =
               newScript.file;
         });
       }).then((_) => library);
     }
     return new Future<LibraryElement>.value(library);
   }
 
   @override
   bool hasResolvedAst(ExecutableElement element) {

[Siggi Cherem (dart-lang), 2016/05/13 19:43:51]

I wonder if we can get rid of `hasResovledAst` now and simply test whether
resolvedAst != null (here, and in AstElement and AstElementZ)

In particular, the `hasResolvedAst` API seems to only be used in this file and
in assertions elsewhere.

[Johnni Winther, 2016/05/17 11:59:52]

Soon. Adding a TODO.

-    return _resolvedAstDeserializer.hasResolvedAst(element);
+    return getResolvedAst(element) != null;
   }
 
   @override
   ResolvedAst getResolvedAst(ExecutableElement element) {
+    if (element.hasResolvedAst) {
+      return element.resolvedAst;
+    }
     return _resolvedAstDeserializer.getResolvedAst(element);
   }
 
   @override
   bool hasResolutionImpact(Element element) {
     if (element.isConstructor &&
             element.enclosingClass.isUnnamedMixinApplication) {
       return true;
     }
     return _resolutionImpactDeserializer.impactMap.containsKey(element);
@@ skipping 18 matching lines @@
       });
     }
     return _resolutionImpactDeserializer.impactMap[element];
   }
 
   @override
   WorldImpact computeWorldImpact(Element element) {
     ResolutionImpact resolutionImpact = getResolutionImpact(element);
     assert(invariant(element, resolutionImpact != null,
         message: 'No impact found for $element (${element.library})'));
+    if (element is ExecutableElement) {
+      getResolvedAst(element);
+    }
     return _impactTransformer.transformResolutionImpact(resolutionImpact);
   }
 
   @override
   bool isDeserialized(Element element) {
     return deserializedLibraries.contains(element.library);
   }
 }
 
 const String RESOLVED_AST_TAG = 'resolvedAst';
@@ skipping 19 matching lines @@
           nativeDataSerializer).serialize();
     }
   }
 }
 
 class ResolvedAstDeserializerPlugin extends DeserializerPlugin {
   final ParsingContext parsingContext;
   final DeserializerPlugin nativeDataDeserializer;
   final Map<Uri, SourceFile> sourceFiles = <Uri, SourceFile>{};
 
-  Map<ExecutableElement, ResolvedAst> _resolvedAstMap =
-      <ExecutableElement, ResolvedAst>{};
   Map<MemberElement, ObjectDecoder> _decoderMap =
       <MemberElement, ObjectDecoder>{};
   Map<Uri, Token> beginTokenMap = <Uri, Token>{};
 
   ResolvedAstDeserializerPlugin(
       this.parsingContext, this.nativeDataDeserializer);
 
   bool hasResolvedAst(ExecutableElement element) {
-    return _resolvedAstMap.containsKey(element) ||
-        _decoderMap.containsKey(element.memberContext);
+    return getResolvedAst(element) != null;
   }
 
   ResolvedAst getResolvedAst(ExecutableElement element) {
-    ResolvedAst resolvedAst = _resolvedAstMap[element];
-    if (resolvedAst == null) {
-      ObjectDecoder decoder = _decoderMap[element.memberContext];
-      if (decoder != null) {
-         ResolvedAstDeserializer.deserialize(
-             element.memberContext, decoder, parsingContext, findToken,
-             nativeDataDeserializer,
-             _resolvedAstMap);
-        _decoderMap.remove(element);
-        resolvedAst = _resolvedAstMap[element];
-      }
+    if (element.hasResolvedAst) {

[Siggi Cherem (dart-lang), 2016/05/13 19:43:51]

correct me if I'm wrong, Is this always called from
_DeserializerSystem.getResolvedAst?

If so, it seems we could check `hasResolvedAst` only in one of them.

[Johnni Winther, 2016/05/17 11:59:51]

Done.

+      return element.resolvedAst;
     }
-    return resolvedAst;
+
+    ObjectDecoder decoder = _decoderMap[element.memberContext];
+    if (decoder != null) {
+       ResolvedAstDeserializer.deserialize(
+           element.memberContext, decoder, parsingContext, findToken,
+           nativeDataDeserializer);
+      _decoderMap.remove(element);
+      return element.resolvedAst;

[Siggi Cherem (dart-lang), 2016/05/13 19:43:51]

could we assert here that element.resolvedAst != null?

[Johnni Winther, 2016/05/17 11:59:52]

Done.

+    }
+    return null;
   }
 
   Token findToken(Uri uri, int offset) {
     Token beginToken = beginTokenMap.putIfAbsent(uri, () {
       SourceFile sourceFile = sourceFiles[uri];
       if (sourceFile == null) {
         throw 'No source file found for $uri in:\n '
               '${sourceFiles.keys.join('\n ')}';
       }
       return new Scanner(sourceFile).tokenize();
     });
     return ResolvedAstDeserializer.findTokenInStream(beginToken, offset);
   }
 
   @override
   void onElement(Element element, ObjectDecoder getDecoder(String tag)) {
     ObjectDecoder decoder = getDecoder(RESOLVED_AST_TAG);
     if (decoder != null) {
       _decoderMap[element] = decoder;
     }
   }
 }