Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(813)

Unified Diff: pkg/dev_compiler/lib/src/compiler/class_property_model.dart

Issue 2691433004: Support virtual fields in DDC without requiring the @virtual annotation.
Patch Set: Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | pkg/dev_compiler/lib/src/compiler/code_generator.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: pkg/dev_compiler/lib/src/compiler/class_property_model.dart
diff --git a/pkg/dev_compiler/lib/src/compiler/class_property_model.dart b/pkg/dev_compiler/lib/src/compiler/class_property_model.dart
index 8319c828ffb49350736ae532ed78e5db0e0f86db..55b7677fc5ebad57b7090ebd31b80a576542273e 100644
--- a/pkg/dev_compiler/lib/src/compiler/class_property_model.dart
+++ b/pkg/dev_compiler/lib/src/compiler/class_property_model.dart
@@ -2,8 +2,9 @@
// 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.
-import 'dart:collection' show HashSet;
+import 'dart:collection' show HashMap, HashSet;
+import 'package:analyzer/analyzer.dart' as analyzer;
import 'package:analyzer/dart/ast/ast.dart' show Identifier;
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/element/element.dart' show FieldElementImpl;
@@ -27,6 +28,15 @@ class ClassPropertyModel {
/// The value property stores the symbol used for the field's storage slot.
final virtualFields = <FieldElement, JS.TemporaryId>{};
+ /// Fields that have been overridden by the current class and must be
+ /// virtualized.
+ ///
+ /// These fields are the elements of the super class, and not the
+ /// overriding elements.
+ ///
+ /// These elements must point to real fields (not getters).
+ final virtualizedFields = <FieldElement, JS.TemporaryId>{};
+
/// Static fields that are overridden, this does not matter for Dart but in
/// JS we need to take care initializing these because JS classes inherit
/// statics.
@@ -44,6 +54,10 @@ class ClassPropertyModel {
ClassPropertyModel.build(
ClassElement classElem, Iterable<ExecutableElement> extensionMembers) {
+ LibraryElement classLibrary = classElem.library;
+ HashMap<String, FieldElementImpl> overriddenFields = new HashMap();
+ HashSet<String> alreadyVirtual = new HashSet();
+
// Visit superclasses to collect information about their fields/accessors.
// This is expensive so we try to collect everything in one pass.
for (var base in getSuperclasses(classElem)) {
@@ -53,14 +67,43 @@ class ClassPropertyModel {
var field = accessor.variable;
var name = field.name;
+
// Ignore private names from other libraries.
if (Identifier.isPrivateName(name) &&
- accessor.library != classElem.library) {
+ accessor.library != classLibrary) {
continue;
}
- if (field.getter?.isAbstract == false) inheritedGetters.add(name);
- if (field.setter?.isAbstract == false) inheritedSetters.add(name);
+ // Ignore abstract getters and setters.
+ if (!(field.getter?.isAbstract == false ||
+ field.setter?.isAbstract == false)) {
+ continue;
+ }
+
+ if (!field.isStatic && !alreadyVirtual.contains(name)) {
+ // Synthetic field means there is a getter or setter and the member
+ // is therefore already virtual.
+ if (field.isSynthetic) {
+ alreadyVirtual.add(name);
+ } else if (field is FieldElementImpl) {
+ if (overriddenFields.containsKey(name)) {
+ // The field is already overridden by a super class.
+ // class A { int x; } // Second time the field is added.
+ // class B extends A { int x; } // First time the field is added.
+ // class C extends B { ... };
+ alreadyVirtual.add(name);
+ } else {
+ overriddenFields[name] = field;
+ }
+ }
+ }
+
+ if (field.getter?.isAbstract == false) {
+ inheritedGetters.add(name);
+ }
+ if (field.setter?.isAbstract == false) {
+ inheritedSetters.add(name);
+ }
}
}
@@ -77,12 +120,21 @@ class ClassPropertyModel {
var field = accessor.variable;
var name = field.name;
+
+ if (!field.isStatic &&
+ !alreadyVirtual.contains(name) &&
+ overriddenFields.containsKey(name)) {
+ JS.TemporaryId id = new JS.TemporaryId(name);
+ var superField = overriddenFields[name];
+ virtualizedFields[superField] = id;
+ alreadyVirtual.add(name);
+ }
+
// Is it a field?
if (!field.isSynthetic && field is FieldElementImpl) {
if (inheritedGetters.contains(name) ||
inheritedSetters.contains(name) ||
- extensionNames.contains(name) ||
- field.isVirtual) {
+ extensionNames.contains(name)) {
if (field.isStatic) {
staticFieldOverrides.add(field);
} else {
@@ -91,5 +143,38 @@ class ClassPropertyModel {
}
}
}
+
+ // Run through all bodies and find all `super.x` accesses. Then take their
+ // names (`x`) and see if they need to be virtualized.
+ for (var name in SuperFinder.accessedSuperProperties(classElem)) {
+ if (!alreadyVirtual.contains(name) &&
+ overriddenFields.containsKey(name)) {
+ JS.TemporaryId id = new JS.TemporaryId(name);
+ var superField = overriddenFields[name];
+ virtualizedFields[superField] = id;
+ }
+ }
+ }
+}
+
+class SuperFinder extends analyzer.GeneralizingAstVisitor<Object> {
+ final accessed = new HashSet<String>();
+
+ static Iterable<String> accessedSuperProperties(ClassElement element) {
+ // When compiling the SDK, only one out of ~50 classes has any references to
+ // super at all, so we avoid searching the method bodies in the common case.
+ if (!element.hasReferenceToSuper) return const <String>[];
+ SuperFinder finder = new SuperFinder();
+ element.computeNode().accept(finder);
+ return finder.accessed;
+ }
+
+ @override
+ analyzer.PropertyAccess visitPropertyAccess(analyzer.PropertyAccess node) {
+ if (node.realTarget is analyzer.SuperExpression) {
+ var name = node.propertyName.name;
+ accessed.add(name);
+ }
+ return node;
}
}
« no previous file with comments | « no previous file | pkg/dev_compiler/lib/src/compiler/code_generator.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698