Store ResolvedAst on AstElement

tests/compiler/dart2js/serialization/helper.dart

Index: tests/compiler/dart2js/serialization/helper.dart
diff --git a/tests/compiler/dart2js/serialization/helper.dart b/tests/compiler/dart2js/serialization/helper.dart
index a2f38ad8f67106af785813e87a5c269c503e83e5..3ceea68e097ab4119a694e900a88c3bfd6ac033d 100644
--- a/tests/compiler/dart2js/serialization/helper.dart
+++ b/tests/compiler/dart2js/serialization/helper.dart
@@ -232,11 +232,14 @@ class _DeserializerSystem extends DeserializerSystem {
 
   @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);
   }
 
@@ -275,6 +278,9 @@ class _DeserializerSystem extends DeserializerSystem {
     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);
   }
 
@@ -314,8 +320,6 @@ class ResolvedAstDeserializerPlugin extends DeserializerPlugin {
   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>{};
@@ -324,24 +328,23 @@ class ResolvedAstDeserializerPlugin extends DeserializerPlugin {
       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;
+    }
+
+    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 resolvedAst;
+    return null;
   }
 
   Token findToken(Uri uri, int offset) {