A step towards handling `new Object()` in compile_from_dill_test

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

 // Copyright (c) 2014, 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.js_emitter.program_builder;
 
 import 'dart:io';
 import 'dart:convert' show JSON;
 
 import '../../closure.dart' show ClosureTask, ClosureFieldElement;
 import '../../common.dart';
 import '../../common/names.dart' show Names, Selectors;
 import '../../constants/values.dart'
     show ConstantValue, InterceptorConstantValue;
 import '../../common_elements.dart' show CommonElements, ElementEnvironment;
 import '../../deferred_load.dart' show DeferredLoadTask, OutputUnit;
 import '../../elements/elements.dart'
     show
         ClassElement,
+        ConstructorBodyElement,
         Element,
         Elements,
         FieldElement,
         FunctionElement,
         FunctionSignature,
         GetterElement,
         LibraryElement,
         MemberElement,
         MethodElement,
         ParameterElement,
@@ skipping 199 matching lines @@
       if (superclass != null) {
         c.setSuperclass(_classes[superclass]);
         assert(
             c.superclass != null,
             failedAt(
                 cls,
                 "No Class for has been created for superclass "
                 "${superclass} of $c."));
       }
       if (c is MixinApplication) {
         c.setMixinClass(_classes[computeMixinClass(cls)]);

[Siggi Cherem (dart-lang), 2017/06/08 18:01:46]

should we do here the same fix you did in the other file? Or change
computeMixinClass to take the _elementEnvironment and continue to use it in all
3 places?

[Johnni Winther, 2017/06/09 09:28:49]

ElementEnvironment.forEachMixin returns all mixins, here we need just the one...

[Siggi Cherem (dart-lang), 2017/06/14 18:13:24]

are you saying that the code in program_builder/collector.dart was actually
broken?

[Johnni Winther, 2017/06/15 07:44:11]

It wasn't - just weird: computeMixinClass didn't make sense when it was written
(a mixed in class could not itself be a mixin application). Turned out that the
refactoring was inconsistent. Fixed in
https://codereview.chromium.org/2934783002/

         assert(c.mixinClass != null);
       }
     });
 
     List<Class> nativeClasses = collector.nativeClassesAndSubclasses
         .map((ClassEntity classElement) => _classes[classElement])
         .toList();
 
     Set<ClassEntity> interceptorClassesNeededByConstants =
         collector.computeInterceptorsReferencedFromConstants();
@@ skipping 260 matching lines @@
     // Interceptor class with implementations that directly call the
     // corresponding JavaScript member. We do not attempt to bind this when
     // tearing off JavaScript methods as we cannot distinguish between calling
     // a regular getter that returns a JavaScript function and tearing off
     // a method in the case where there exist multiple JavaScript classes
     // that conflict on whether the member is a getter or a method.
     var interceptorClass = _classes[_commonElements.jsJavaScriptObjectClass];
     var stubNames = new Set<String>();
     librariesMap
         .forEach((LibraryEntity library, List<ClassEntity> classElements, _) {
-      for (ClassElement e in classElements) {
-        if (_nativeData.isJsInteropClass(e)) {
+      for (ClassEntity cls in classElements) {
+        if (_nativeData.isJsInteropClass(cls)) {
+          // TODO(johnniwinther): Handle class entities.
+          ClassElement e = cls;
           e.declaration.forEachMember((_, Element member) {
             var jsName = _nativeData.computeUnescapedJSInteropName(member.name);
             if (!member.isInstanceMember) return;
             if (member.isGetter || member.isField || member.isFunction) {
               var selectors =
                   _worldBuilder.getterInvocationsByName(member.name);
               if (selectors != null && !selectors.isEmpty) {
                 for (var selector in selectors.keys) {
                   var stubName = _namer.invocationName(selector);
                   if (stubNames.add(stubName.key)) {
@@ skipping 124 matching lines @@
         .toList(growable: false);
 
     bool visitStatics = true;
     List<Field> staticFieldsForReflection =
         _buildFields(library: library, visitStatics: visitStatics);
 
     return new Library(
         library, uri, statics, classes, staticFieldsForReflection);
   }
 
-  bool _isSoftDeferred(ClassElement element) {
+  bool _isSoftDeferred(ClassEntity element) {
     return _notSoftDeferred != null && !_notSoftDeferred.contains(element);
   }
 
-  Class _buildClass(ClassElement element) {
-    bool onlyForRti = collector.classesOnlyNeededForRti.contains(element);
-    bool hasRtiField = _rtiNeed.classNeedsRtiField(element);
-    if (_nativeData.isJsInteropClass(element)) {
+  Class _buildClass(ClassEntity cls) {
+    bool onlyForRti = collector.classesOnlyNeededForRti.contains(cls);
+    bool hasRtiField = _rtiNeed.classNeedsRtiField(cls);
+    if (_nativeData.isJsInteropClass(cls)) {
       // TODO(jacobr): check whether the class has any active static fields
       // if it does not we can suppress it completely.
       onlyForRti = true;
     }
-    bool isClosureBaseClass = element == _commonElements.closureClass;
+    bool isClosureBaseClass = cls == _commonElements.closureClass;
 
     List<Method> methods = [];
     List<StubMethod> callStubs = <StubMethod>[];
 
     ClassStubGenerator classStubGenerator = new ClassStubGenerator(
         _task.emitter, _commonElements, _namer, _worldBuilder, _closedWorld,
         enableMinification: _options.enableMinification);
     RuntimeTypeGenerator runtimeTypeGenerator = new RuntimeTypeGenerator(
         _commonElements,
         _closureToClassMapper,
         _task,
         _namer,
         _nativeData,
         _rtiChecks,
         _rtiEncoder,
         _rtiNeed,
         _rtiSubstitutions,
         _jsInteropAnalysis);
 
-    void visitMember(ClassElement enclosing, MemberElement member) {
-      assert(member.isDeclaration, failedAt(element));
-      assert(element == enclosing, failedAt(element));
+    void visitMember(ClassEntity declarer, MemberEntity member) {
+      if (cls != declarer) return;
 
-      if (Elements.isNonAbstractInstanceMember(member)) {
+      if (member.isInstanceMember && !member.isAbstract && !member.isField) {
         // TODO(herhut): Remove once _buildMethod can no longer return null.
         Method method = _buildMethod(member);
         if (method != null) methods.add(method);
       }
       if (member.isGetter || member.isField) {
         Map<Selector, SelectorConstraints> selectors =
             _worldBuilder.invocationsByName(member.name);
         if (selectors != null && !selectors.isEmpty) {
           Map<js.Name, js.Expression> callStubsForMember =
               classStubGenerator.generateCallStubsForGetter(member, selectors);
           callStubsForMember.forEach((js.Name name, js.Expression code) {
             callStubs.add(_buildStubMethod(name, code, element: member));
           });
         }
       }
     }
 
     List<StubMethod> noSuchMethodStubs = <StubMethod>[];
 
-    if (_backendUsage.isNoSuchMethodUsed && element.isObject) {
+    if (_backendUsage.isNoSuchMethodUsed &&
+        cls == _commonElements.objectClass) {
       Map<js.Name, Selector> selectors =
           classStubGenerator.computeSelectorsForNsmHandlers();
       selectors.forEach((js.Name name, Selector selector) {
         // If the program contains `const Symbol` names we have to retain them.
         String selectorName = selector.name;
         if (selector.isSetter) selectorName = "$selectorName=";
         if (_mirrorsData.symbolsUsed.contains(selectorName)) {
           _symbolsMap[name] = selectorName;
         }
         noSuchMethodStubs.add(
             classStubGenerator.generateStubForNoSuchMethod(name, selector));
       });
     }
 
     if (isClosureBaseClass) {
       // We add a special getter to allow for tearing off a closure from itself.
       js.Name name = _namer.getterForMember(Names.call);
       js.Fun function = js.js('function() { return this; }');
       callStubs.add(_buildStubMethod(name, function));
     }
 
-    ClassElement implementation = element.implementation;
-
     // MixinApplications run through the members of their mixin. Here, we are
     // only interested in direct members.
-    if (!onlyForRti && !element.isMixinApplication) {
-      implementation.forEachMember(visitMember, includeBackendMembers: true);
+    if (!onlyForRti && !_elementEnvironment.isUnnamedMixinApplication(cls)) {
+      _elementEnvironment.forEachClassMember(cls, visitMember);
+      if (cls is ClassElement) {
+        // TODO(johnniwinther): Support constructor bodies for entities.
+        cls.forEachBackendMember((ConstructorBodyElement constructorBody) =>
+            visitMember(cls, constructorBody));
+      }
     }
-    bool isInterceptedClass = _interceptorData.isInterceptedClass(element);
+    bool isInterceptedClass = _interceptorData.isInterceptedClass(cls);
     List<Field> instanceFields = onlyForRti
         ? const <Field>[]
         : _buildFields(
-            cls: element,
+            cls: cls,
             visitStatics: false,
             isHolderInterceptedClass: isInterceptedClass);
     List<Field> staticFieldsForReflection = onlyForRti
         ? const <Field>[]
         : _buildFields(
-            cls: element,
+            cls: cls,
             visitStatics: true,
             isHolderInterceptedClass: isInterceptedClass);
 
-    TypeTestProperties typeTests = runtimeTypeGenerator.generateIsTests(element,
+    TypeTestProperties typeTests = runtimeTypeGenerator.generateIsTests(cls,
         storeFunctionTypeInMetadata: _storeFunctionTypesInMetadata);
 
     List<StubMethod> checkedSetters = <StubMethod>[];
     List<StubMethod> isChecks = <StubMethod>[];
-    if (_nativeData.isJsInteropClass(element)) {
+    if (_nativeData.isJsInteropClass(cls)) {
       typeTests.properties.forEach((js.Name name, js.Node code) {
         _classes[_commonElements.jsInterceptorClass]
             .isChecks
             .add(_buildStubMethod(name, code));
       });
     } else {
       for (Field field in instanceFields) {
         if (field.needsCheckedSetter) {
           assert(!field.needsUncheckedSetter);
           FieldElement element = field.element;
           js.Expression code = _generatedCode[element];
           assert(code != null);
           js.Name name = _namer.deriveSetterName(field.accessorName);
           checkedSetters.add(_buildStubMethod(name, code, element: element));
         }
       }
 
       typeTests.properties.forEach((js.Name name, js.Node code) {
         isChecks.add(_buildStubMethod(name, code));
       });
     }
 
-    js.Name name = _namer.className(element);
-    String holderName = _namer.globalObjectForClass(element);
+    js.Name name = _namer.className(cls);
+    String holderName = _namer.globalObjectForClass(cls);
     // TODO(floitsch): we shouldn't update the registry in the middle of
     // building a class.
     Holder holder = _registry.registerHolder(holderName);
-    bool isInstantiated = !_nativeData.isJsInteropClass(element) &&
-        _worldBuilder.directlyInstantiatedClasses.contains(element);
+    bool isInstantiated = !_nativeData.isJsInteropClass(cls) &&
+        _worldBuilder.directlyInstantiatedClasses.contains(cls);
 
     Class result;
-    if (element.isMixinApplication && !onlyForRti) {
-      assert(!_nativeData.isNativeClass(element));
+    if (_elementEnvironment.isUnnamedMixinApplication(cls) && !onlyForRti) {
+      assert(!_nativeData.isNativeClass(cls));
       assert(methods.isEmpty);
       assert(!isClosureBaseClass);
 
       result = new MixinApplication(
-          element,
+          cls,
           name,
           holder,
           instanceFields,
           staticFieldsForReflection,
           callStubs,
           checkedSetters,
           isChecks,
           typeTests.functionTypeIndex,
           isDirectlyInstantiated: isInstantiated,
           hasRtiField: hasRtiField,
           onlyForRti: onlyForRti);
     } else {
       result = new Class(
-          element,
+          cls,
           name,
           holder,
           methods,
           instanceFields,
           staticFieldsForReflection,
           callStubs,
           noSuchMethodStubs,
           checkedSetters,
           isChecks,
           typeTests.functionTypeIndex,
           isDirectlyInstantiated: isInstantiated,
           hasRtiField: hasRtiField,
           onlyForRti: onlyForRti,
-          isNative: _nativeData.isNativeClass(element),
+          isNative: _nativeData.isNativeClass(cls),
           isClosureBaseClass: isClosureBaseClass,
-          isSoftDeferred: _isSoftDeferred(element));
+          isSoftDeferred: _isSoftDeferred(cls));
     }
-    _classes[element] = result;
+    _classes[cls] = result;
     return result;
   }
 
   bool _methodNeedsStubs(FunctionEntity method) {
     return method.parameterStructure.optionalParameters != 0;
   }
 
   bool _methodCanBeReflected(FunctionEntity method) {
     return _mirrorsData.isMemberAccessibleByReflection(method);
   }
@@ skipping 16 matching lines @@
       optionalParameterDefaultValues = <ConstantValue>[];
       signature.forEachOptionalParameter((ParameterElement parameter) {
         ConstantValue def =
             _constantHandler.getConstantValue(parameter.constant);
         optionalParameterDefaultValues.add(def);
       });
     }
     return optionalParameterDefaultValues;
   }
 
-  DartMethod _buildMethod(MethodElement element) {
-    assert(element.isDeclaration);
+  DartMethod _buildMethod(FunctionEntity element) {
+    assert(!(element is MethodElement && !element.isDeclaration));
     js.Name name = _namer.methodPropertyName(element);
     js.Expression code = _generatedCode[element];
 
     // TODO(kasperl): Figure out under which conditions code is null.
     if (code == null) return null;
 
     bool canTearOff = false;
     js.Name tearOffName;
     bool isClosureCallMethod = false;
-    bool isNotApplyTarget = !element.isFunction || element.isAccessor;
+    bool isNotApplyTarget =
+        !element.isFunction || element.isGetter || element.isSetter;
 
     bool canBeReflected = _methodCanBeReflected(element);
     bool canBeApplied = _methodCanBeApplied(element);
 
     js.Name aliasName = _superMemberData.isAliasedSuperMember(element)
         ? _namer.aliasedSuperMemberPropertyName(element)
         : null;
 
     if (isNotApplyTarget) {
       canTearOff = false;
     } else {
       if (element.enclosingClass.isClosure) {
         canTearOff = false;
         isClosureCallMethod = true;
       } else {
         // Careful with operators.
         canTearOff = _worldBuilder.hasInvokedGetter(element, _closedWorld) ||
-            (canBeReflected && !element.isOperator);
+            (canBeReflected && !Selector.isOperatorName(element.name));
         assert(canTearOff ||
             !_worldBuilder.methodsNeedingSuperGetter.contains(element));
         tearOffName = _namer.getterForElement(element);
       }
     }
 
     if (canTearOff) {
-      assert(!element.isGenerativeConstructor, failedAt(element));
-      assert(!element.isGenerativeConstructorBody, failedAt(element));
-      assert(!element.isConstructor, failedAt(element));
+      assert(element is! ConstructorEntity, failedAt(element));
+      assert(element is! ConstructorBodyElement, failedAt(element));
     }
 
     js.Name callName = null;
     if (canTearOff) {
       Selector callSelector =
           new Selector.fromElement(element).toCallSelector();
       callName = _namer.invocationName(callSelector);
     }
 
-    ResolutionDartType memberType;
-    if (element.isGenerativeConstructorBody) {
-      // TODO(herhut): Why does this need to be normalized away? We never need
-      //               this information anyway as they cannot be torn off or
-      //               reflected.
-      var body = element;
-      memberType = body.constructor.type;
-    } else {
-      memberType = element.type;
-    }
-
+    DartType memberType = _elementEnvironment.getFunctionType(element);
     js.Expression functionType;
     if (canTearOff || canBeReflected) {
-      OutputUnit outputUnit = _deferredLoadTask.outputUnitForElement(element);
+      OutputUnit outputUnit = _deferredLoadTask.outputUnitForMember(element);
       functionType = _generateFunctionType(memberType, outputUnit);
     }
 
     int requiredParameterCount;
     var /* List | Map */ optionalParameterDefaultValues;
     if (canBeApplied || canBeReflected) {
-      FunctionSignature signature = element.functionSignature;
+      // TODO(johnniwinther): Handle function entities.
+      MethodElement method = element;
+      FunctionSignature signature = method.functionSignature;
       requiredParameterCount = signature.requiredParameterCount;
       optionalParameterDefaultValues =
           _computeParameterDefaultValues(signature);
     }
 
     return new InstanceMethod(element, name, code,
         _generateParameterStubs(element, canTearOff), callName,
         needsTearOff: canTearOff,
         tearOffName: tearOffName,
         isClosureCallMethod: isClosureCallMethod,
@@ skipping 228 matching lines @@
       Constant constant = new Constant(name, holder, constantValue);
       _constants[constantValue] = constant;
     }
   }
 
   Holder _registerStaticStateHolder() {
     return _registry.registerHolder(_namer.staticStateHolder,
         isStaticStateHolder: true);
   }
 }