mirror of
https://github.com/dart-lang/sdk
synced 2024-09-16 02:27:39 +00:00
Reland "[dartdevc] fix for const / overridden fields"
This reverts commit 43cacafb51
.
Patchset 1 is the original CL. Compare PS 1 to 4 to see additional fix.
It undoes an optimization that assumes private fields
are not overridden in the SDK. This patterns happens in dart:ui
and would be difficult to enforce now that flutter web also adds to
the SDK. As a result, all private SDK fields are virtualized, adding 0.7% to the size of
dart_sdk.js.
Fixes https://github.com/dart-lang/sdk/issues/38455
Change-Id: If969dddcb7143316ac8c771df1ed83def21412b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/118362
Commit-Queue: Vijay Menon <vsm@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
This commit is contained in:
parent
4167867bb2
commit
a6cc7b94c7
|
@ -252,6 +252,7 @@ abstract class SharedCompiler<Library, Class, InterfaceType, FunctionNode> {
|
|||
/// runtime call.
|
||||
js_ast.TemporaryId initPrivateNameSymbol() {
|
||||
var idName = name.endsWith('=') ? name.replaceAll('=', '_') : name;
|
||||
idName = idName.replaceAll('.', '_');
|
||||
var id = js_ast.TemporaryId(idName);
|
||||
moduleItems.add(js.statement('const # = #.privateName(#, #)',
|
||||
[id, runtimeModule, emitLibraryName(library), js.string(name)]));
|
||||
|
@ -262,6 +263,17 @@ abstract class SharedCompiler<Library, Class, InterfaceType, FunctionNode> {
|
|||
return privateNames.putIfAbsent(name, initPrivateNameSymbol);
|
||||
}
|
||||
|
||||
/// Emits a private name JS Symbol for [memberName] unique to a Dart
|
||||
/// class [className].
|
||||
///
|
||||
/// This is now required for fields of constant objects that may be
|
||||
/// overridden within the same library.
|
||||
@protected
|
||||
js_ast.TemporaryId emitClassPrivateNameSymbol(
|
||||
Library library, String className, String memberName) {
|
||||
return emitPrivateNameSymbol(library, '$className.$memberName');
|
||||
}
|
||||
|
||||
/// Emits an expression to set the property [nameExpr] on the class [className],
|
||||
/// with [value].
|
||||
///
|
||||
|
|
|
@ -2796,10 +2796,9 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
|
|||
|
||||
void _emitVirtualFieldSymbols(Class c, List<js_ast.Statement> body) {
|
||||
_classProperties.virtualFields.forEach((field, virtualField) {
|
||||
body.add(js.statement('const # = Symbol(#);', [
|
||||
virtualField,
|
||||
js.string('${getLocalClassName(c)}.${field.name.name}')
|
||||
]));
|
||||
var symbol = emitClassPrivateNameSymbol(
|
||||
c.enclosingLibrary, getLocalClassName(c), field.name.name);
|
||||
body.add(js.statement('const # = #;', [virtualField, symbol]));
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -5413,8 +5412,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
|
|||
entryToProperty(MapEntry<Reference, Constant> entry) {
|
||||
var constant = visitConstant(entry.value);
|
||||
var member = entry.key.asField;
|
||||
return js_ast.Property(
|
||||
_emitMemberName(member.name.name, member: member), constant);
|
||||
var cls = member.enclosingClass;
|
||||
// Enums cannot be overridden, so we can safely use the field name
|
||||
// directly. Otherwise, use a private symbol in case the field
|
||||
// was overridden.
|
||||
var symbol = cls.isEnum
|
||||
? _emitMemberName(member.name.name, member: member)
|
||||
: emitClassPrivateNameSymbol(
|
||||
cls.enclosingLibrary, getLocalClassName(cls), member.name.name);
|
||||
return js_ast.Property(symbol, constant);
|
||||
}
|
||||
|
||||
var type = visitInterfaceType(node.getType(_types) as InterfaceType);
|
||||
|
|
|
@ -147,11 +147,6 @@ class _LibraryVirtualFieldModel {
|
|||
// Enums are not extensible.
|
||||
return false;
|
||||
}
|
||||
var libraryUri = class_.enclosingLibrary.importUri;
|
||||
if (libraryUri.scheme == 'dart' && libraryUri.path.startsWith('_')) {
|
||||
// There should be no extensible fields in private SDK libraries.
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!field.name.isPrivate) {
|
||||
// Public fields in public classes (or extensible private classes)
|
||||
|
@ -161,6 +156,13 @@ class _LibraryVirtualFieldModel {
|
|||
if (_extensiblePrivateClasses.contains(class_)) return true;
|
||||
}
|
||||
|
||||
if (class_.constructors.any((c) => c.isConst)) {
|
||||
// Always virtualize fields of a (might be) non-enum (see above) const
|
||||
// class. The way these are lowered by the CFE, they need to be
|
||||
// writable from different modules even if overridden.
|
||||
return true;
|
||||
}
|
||||
|
||||
// Otherwise, the field is effectively private and we only need to make it
|
||||
// virtual if it's overridden.
|
||||
return _overriddenPrivateFields.contains(field);
|
||||
|
|
23
tests/language_2/override_const_field_test.dart
Normal file
23
tests/language_2/override_const_field_test.dart
Normal file
|
@ -0,0 +1,23 @@
|
|||
// Copyright (c) 2019, 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.
|
||||
// Dart test checking that static/instance field shadowing do not conflict.
|
||||
|
||||
import 'package:expect/expect.dart';
|
||||
|
||||
class A {
|
||||
final field;
|
||||
const A(this.field);
|
||||
}
|
||||
|
||||
class B extends A {
|
||||
final field;
|
||||
const B(this.field, fieldA) : super(fieldA);
|
||||
get fieldA => super.field;
|
||||
}
|
||||
|
||||
main() {
|
||||
const b = B(1, 2);
|
||||
Expect.equals(1, b.field);
|
||||
Expect.equals(2, b.fieldA);
|
||||
}
|
Loading…
Reference in a new issue