Skip to content

Commit ce6f40b

Browse files
vsmenoncommit-bot@chromium.org
authored andcommitted
[dartdevc] fix for const / overridden fields
Fixes #37839 Change-Id: Ibe7b151e8e5fdd4d83cf5b69d49c7b3d17a1cefe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113136 Commit-Queue: Vijay Menon <[email protected]> Reviewed-by: Mark Zhou <[email protected]>
1 parent 38c6152 commit ce6f40b

File tree

4 files changed

+53
-6
lines changed

4 files changed

+53
-6
lines changed

pkg/dev_compiler/lib/src/compiler/shared_compiler.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ abstract class SharedCompiler<Library, Class, InterfaceType, FunctionNode> {
252252
/// runtime call.
253253
js_ast.TemporaryId initPrivateNameSymbol() {
254254
var idName = name.endsWith('=') ? name.replaceAll('=', '_') : name;
255+
idName = idName.replaceAll('.', '_');
255256
var id = js_ast.TemporaryId(idName);
256257
moduleItems.add(js.statement('const # = #.privateName(#, #)',
257258
[id, runtimeModule, emitLibraryName(library), js.string(name)]));
@@ -262,6 +263,16 @@ abstract class SharedCompiler<Library, Class, InterfaceType, FunctionNode> {
262263
return privateNames.putIfAbsent(name, initPrivateNameSymbol);
263264
}
264265

266+
/// Emits a private name JS Symbol for [name] unique to a Dart class [cls].
267+
///
268+
/// This is now required for fields of constant objects that may be
269+
/// overridden within the same library.
270+
@protected
271+
js_ast.TemporaryId emitClassPrivateNameSymbol(
272+
Library library, String className, String memberName) {
273+
return emitPrivateNameSymbol(library, '$className.$memberName');
274+
}
275+
265276
/// Emits an expression to set the property [nameExpr] on the class [className],
266277
/// with [value].
267278
///

pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2795,10 +2795,9 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
27952795

27962796
void _emitVirtualFieldSymbols(Class c, List<js_ast.Statement> body) {
27972797
_classProperties.virtualFields.forEach((field, virtualField) {
2798-
body.add(js.statement('const # = Symbol(#);', [
2799-
virtualField,
2800-
js.string('${getLocalClassName(c)}.${field.name.name}')
2801-
]));
2798+
var symbol = emitClassPrivateNameSymbol(
2799+
c.enclosingLibrary, getLocalClassName(c), field.name.name);
2800+
body.add(js.statement('const # = #;', [virtualField, symbol]));
28022801
});
28032802
}
28042803

@@ -5405,8 +5404,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
54055404
entryToProperty(MapEntry<Reference, Constant> entry) {
54065405
var constant = visitConstant(entry.value);
54075406
var member = entry.key.asField;
5408-
return js_ast.Property(
5409-
_emitMemberName(member.name.name, member: member), constant);
5407+
var cls = member.enclosingClass;
5408+
// Enums cannot be overridden, so we can safely use the field name
5409+
// directly. Otherwise, use a private symbol in case the field
5410+
// was overridden.
5411+
var symbol = cls.isEnum
5412+
? _emitMemberName(member.name.name, member: member)
5413+
: emitClassPrivateNameSymbol(
5414+
cls.enclosingLibrary, getLocalClassName(cls), member.name.name);
5415+
return js_ast.Property(symbol, constant);
54105416
}
54115417

54125418
var type = visitInterfaceType(node.getType(_types) as InterfaceType);

pkg/dev_compiler/lib/src/kernel/property_model.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,13 @@ class _LibraryVirtualFieldModel {
161161
if (_extensiblePrivateClasses.contains(class_)) return true;
162162
}
163163

164+
if (class_.constructors.any((c) => c.isConst)) {
165+
// Always virtualize fields of a (might be) non-enum (see above) const
166+
// class. The way these are lowered by the CFE, they need to be
167+
// writable from different modules even if overridden.
168+
return true;
169+
}
170+
164171
// Otherwise, the field is effectively private and we only need to make it
165172
// virtual if it's overridden.
166173
return _overriddenPrivateFields.contains(field);
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
// Dart test checking that static/instance field shadowing do not conflict.
5+
6+
import 'package:expect/expect.dart';
7+
8+
class A {
9+
final field;
10+
const A(this.field);
11+
}
12+
13+
class B extends A {
14+
final field;
15+
const B(this.field, fieldA) : super(fieldA);
16+
get fieldA => super.field;
17+
}
18+
19+
main() {
20+
const b = B(1, 2);
21+
Expect.equals(1, b.field);
22+
Expect.equals(2, b.fieldA);
23+
}

0 commit comments

Comments
 (0)