Revert "[cfe] Encode field references as @getters and @setters"

This reverts commit 3892e95547.

Reason for revert: Breaks Flutter web

Original change's description:
> [cfe] Encode field references as @getters and @setters
>
> This removes the @fields and @=fields canonical name
> encodings that allowed for encoding of conflicting members
> and didn't support field<->getter/setter conversion
> between dills or between outline and full dill.
>
> TEST=existing tests
>
> Change-Id: Id15e58ad4d1847d2c98a688705e5945196146c6d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184783
> Commit-Queue: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Jens Johansen <jensj@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>

Change-Id: I744e284b16e097fa0833c5bdf1bc7653f13bdf63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186147
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Johnni Winther 2021-02-22 12:56:43 +00:00 committed by commit-bot@chromium.org
parent ae81a3ca9b
commit 79de274398
20 changed files with 102 additions and 122 deletions

View file

@ -6,8 +6,8 @@ library from "package:example/a.dart" as a {
}
library from "package:foo/foo.dart" as foo {
additionalExports = (a::example,
a::a,
a::example)
a::example,
a::a)
export "package:example/a.dart";

View file

@ -6,8 +6,8 @@ library from "package:example/a.dart" as a {
}
library from "package:foo/foo.dart" as foo {
additionalExports = (a::example,
a::a,
a::example)
a::example,
a::a)
export "package:example/a.dart";

View file

@ -10,8 +10,8 @@ library from "org-dartlang-test:///lib.dart" as lib {
}
library from "org-dartlang-test:///libExporter.dart" as lib2 {
additionalExports = (lib::libField,
lib::requireStuffFromLibExporter,
lib::libField)
lib::libField,
lib::requireStuffFromLibExporter)
export "org-dartlang-test:///lib.dart";

View file

@ -10,8 +10,8 @@ library from "org-dartlang-test:///lib.dart" as lib {
}
library from "org-dartlang-test:///libExporter.dart" as lib2 {
additionalExports = (lib::libField,
lib::requireStuffFromLibExporter,
lib::libField)
lib::libField,
lib::requireStuffFromLibExporter)
export "org-dartlang-test:///lib.dart";

View file

@ -10,8 +10,8 @@ library from "org-dartlang-test:///lib.dart" as lib {
}
library from "org-dartlang-test:///libExporter.dart" as lib2 {
additionalExports = (lib::libField,
lib::requireStuffFromLibExporter,
lib::libField)
lib::libField,
lib::requireStuffFromLibExporter)
export "org-dartlang-test:///lib.dart";

View file

@ -10,8 +10,8 @@ library from "org-dartlang-test:///lib.dart" as lib {
}
library from "org-dartlang-test:///libExporter.dart" as lib2 {
additionalExports = (lib::libField,
lib::requireStuffFromLibExporter,
lib::libField)
lib::libField,
lib::requireStuffFromLibExporter)
export "org-dartlang-test:///lib.dart";

View file

@ -147,7 +147,7 @@ type CanonicalName {
type ComponentFile {
UInt32 magic = 0x90ABCDEF;
UInt32 formatVersion = 56;
UInt32 formatVersion = 55;
Byte[10] shortSdkHash;
List<String> problemsAsJson; // Described in problems.md.
Library[] libraries;

View file

@ -678,6 +678,8 @@ class BinaryBuilder {
for (CanonicalName child in parentChildren) {
if (child.name != '@methods' &&
child.name != '@typedefs' &&
child.name != '@fields' &&
child.name != '@=fields' &&
child.name != '@getters' &&
child.name != '@setters' &&
child.name != '@factories' &&

View file

@ -173,7 +173,7 @@ class Tag {
/// Internal version of kernel binary format.
/// Bump it when making incompatible changes in kernel binaries.
/// Keep in sync with runtime/vm/kernel_binary.h, pkg/kernel/binary.md.
static const int BinaryFormatVersion = 56;
static const int BinaryFormatVersion = 55;
}
abstract class ConstantTag {

View file

@ -30,14 +30,9 @@ import 'ast.dart';
/// "@constructors"
/// Qualified name
///
/// Field or the implicit getter of a field:
/// Field:
/// Canonical name of enclosing class or library
/// "@getters"
/// Qualified name
///
/// Implicit setter of a field:
/// Canonical name of enclosing class or library
/// "@setters"
/// "@fields"
/// Qualified name
///
/// Typedef:
@ -137,11 +132,11 @@ class CanonicalName {
}
CanonicalName getChildFromField(Field field) {
return getChild('@getters').getChildFromQualifiedName(field.name!);
return getChild('@fields').getChildFromQualifiedName(field.name!);
}
CanonicalName getChildFromFieldSetter(Field field) {
return getChild('@setters').getChildFromQualifiedName(field.name!);
return getChild('@=fields').getChildFromQualifiedName(field.name!);
}
CanonicalName getChildFromConstructor(Constructor constructor) {
@ -156,11 +151,11 @@ class CanonicalName {
}
CanonicalName getChildFromFieldWithName(Name name) {
return getChild('@getters').getChildFromQualifiedName(name);
return getChild('@fields').getChildFromQualifiedName(name);
}
CanonicalName getChildFromFieldSetterWithName(Name name) {
return getChild('@setters').getChildFromQualifiedName(name);
return getChild('@=fields').getChildFromQualifiedName(name);
}
CanonicalName getChildFromTypedef(Typedef typedef_) {

View file

@ -37,14 +37,12 @@ main() {
List<int> writtenBytesFieldOriginal = serialize(lib1, lib2);
// Canonical names are now set: Verify that the field is marked as such,
// canonical-name-wise.
String getterCanonicalName = '${field.getterCanonicalName}';
if (field.getterCanonicalName.parent.name != "@getters") {
throw "Expected @getters parent, but had "
if (field.getterCanonicalName.parent.name != "@fields") {
throw "Expected @fields parent, but had "
"${field.getterCanonicalName.parent.name}";
}
String setterCanonicalName = '${field.setterCanonicalName}';
if (field.setterCanonicalName.parent.name != "@setters") {
throw "Expected @setters parent, but had "
if (field.setterCanonicalName.parent.name != "@=fields") {
throw "Expected @=fields parent, but had "
"${field.setterCanonicalName.parent.name}";
}
@ -82,18 +80,10 @@ main() {
throw "Expected @getters parent, but had "
"${getter.canonicalName.parent.name}";
}
if ('${getter.canonicalName}' != getterCanonicalName) {
throw "Unexpected getter canonical name. "
"Expected $getterCanonicalName, actual ${getter.canonicalName}.";
}
if (setter.canonicalName.parent.name != "@setters") {
throw "Expected @setters parent, but had "
"${setter.canonicalName.parent.name}";
}
if ('${setter.canonicalName}' != setterCanonicalName) {
throw "Unexpected setter canonical name. "
"Expected $setterCanonicalName, actual ${setter.canonicalName}.";
}
// Replace getter/setter with field.
lib1.procedures.remove(getter);
@ -111,12 +101,12 @@ main() {
List<int> writtenBytesFieldNew = serialize(lib1, lib2);
// Canonical names are now set: Verify that the field is marked as such,
// canonical-name-wise.
if (fieldReplacement.getterCanonicalName.parent.name != "@getters") {
throw "Expected @getters parent, but had "
if (fieldReplacement.getterCanonicalName.parent.name != "@fields") {
throw "Expected @fields parent, but had "
"${fieldReplacement.getterCanonicalName.parent.name}";
}
if (fieldReplacement.setterCanonicalName.parent.name != "@setters") {
throw "Expected @setters parent, but had "
if (fieldReplacement.setterCanonicalName.parent.name != "@=fields") {
throw "Expected @=fields parent, but had "
"${fieldReplacement.setterCanonicalName.parent.name}";
}

View file

@ -133,7 +133,7 @@ void test() {
name: '/* suppose top-level: dynamic field; */ field;',
node: new ExpressionStatement(new StaticGet(field)),
expectation: ''
'(expr (get-static "package:foo/bar.dart::@getters::field"))',
'(expr (get-static "package:foo/bar.dart::@fields::field"))',
makeSerializationState: () => new SerializationState(null),
makeDeserializationState: () =>
new DeserializationState(null, component.root),
@ -151,7 +151,7 @@ void test() {
name: '/* suppose top-level: dynamic field; */ field;',
node: new ExpressionStatement(new StaticGet(field)),
expectation: ''
'(expr (get-static "package:foo/bar.dart::@getters::field"))',
'(expr (get-static "package:foo/bar.dart::@fields::field"))',
makeSerializationState: () => new SerializationState(null),
makeDeserializationState: () =>
new DeserializationState(null, component.root),
@ -171,7 +171,7 @@ void test() {
new ExpressionStatement(new StaticSet(field, new IntLiteral(1))),
expectation: ''
'(expr'
' (set-static "package:foo/bar.dart::@setters::field" (int 1)))',
' (set-static "package:foo/bar.dart::@=fields::field" (int 1)))',
makeSerializationState: () => new SerializationState(null),
makeDeserializationState: () =>
new DeserializationState(null, component.root),

View file

@ -510,29 +510,17 @@ class _FfiDefinitionTransformer extends FfiTransformer {
List<Procedure> _generateMethodsForField(Field field, NativeTypeCfe type,
Map<Abi, int> offsets, IndexedClass indexedClass) {
// TODO(johnniwinther): Avoid passing [indexedClass]. When compiling
// incrementally, [field] should already carry the references from
// [indexedClass].
final getterStatement = type.generateGetterStatement(
field.type, field.fileOffset, offsets, this);
Reference getterReference =
indexedClass?.lookupGetterReference(field.name) ??
field.getterReference;
assert(getterReference == field.getterReference,
"Unexpected getter reference for ${field}, found $getterReference.");
final Procedure getter = Procedure(field.name, ProcedureKind.Getter,
FunctionNode(getterStatement, returnType: field.type),
fileUri: field.fileUri, reference: getterReference)
fileUri: field.fileUri,
reference: indexedClass?.lookupGetterReference(field.name))
..fileOffset = field.fileOffset
..isNonNullableByDefault = field.isNonNullableByDefault;
Procedure setter = null;
if (!field.isFinal) {
Reference setterReference =
indexedClass?.lookupSetterReference(field.name) ??
field.setterReference;
assert(setterReference == field.setterReference,
"Unexpected setter reference for ${field}, found $setterReference.");
final VariableDeclaration argument =
VariableDeclaration('#v', type: field.type)
..fileOffset = field.fileOffset;
@ -544,7 +532,7 @@ class _FfiDefinitionTransformer extends FfiTransformer {
FunctionNode(setterStatement,
returnType: VoidType(), positionalParameters: [argument]),
fileUri: field.fileUri,
reference: setterReference)
reference: indexedClass?.lookupSetterReference(field.name))
..fileOffset = field.fileOffset
..isNonNullableByDefault = field.isNonNullableByDefault;
}

View file

@ -1324,12 +1324,7 @@ class _TreeShakerPass2 extends RemovingTransformer {
if (!shaker.isMemberUsed(node) && !_preserveSpecialMember(node)) {
// Ensure that kernel file writer will not be able to
// write a dangling reference to the deleted member.
if (node is Field) {
node.getterCanonicalName?.reference = null;
node.setterCanonicalName?.reference = null;
} else {
node.canonicalName?.reference = null;
}
node.reference.canonicalName = null;
Statistics.membersDropped++;
return removalSentinel;
}

View file

@ -264,8 +264,7 @@ InstancePtr ConstantReader::ReadConstantInternal(intptr_t constant_offset) {
Field& field = Field::Handle(Z);
Instance& constant = Instance::Handle(Z);
for (intptr_t j = 0; j < number_of_fields; ++j) {
field = H.LookupFieldByKernelGetterOrSetter(
reader.ReadCanonicalNameReference());
field = H.LookupFieldByKernelField(reader.ReadCanonicalNameReference());
// Recurse into lazily evaluating all "sub" constants
// needed to evaluate the current constant.
const intptr_t entry_offset = reader.ReadUInt();

View file

@ -237,7 +237,7 @@ Fragment StreamingFlowGraphBuilder::BuildInitializers(
ReadBool();
const NameIndex field_name = ReadCanonicalNameReference();
const Field& field =
Field::Handle(Z, H.LookupFieldByKernelGetterOrSetter(field_name));
Field::Handle(Z, H.LookupFieldByKernelField(field_name));
initializer_fields[i] = &field;
SkipExpression();
continue;
@ -2180,7 +2180,8 @@ Fragment StreamingFlowGraphBuilder::BuildPropertyGet(TokenPosition* p) {
const Function* tearoff_interface_target = &Function::null_function();
const NameIndex itarget_name =
ReadInterfaceMemberNameReference(); // read interface_target_reference.
if (!H.IsRoot(itarget_name) && H.IsGetter(itarget_name)) {
if (!H.IsRoot(itarget_name) &&
(H.IsGetter(itarget_name) || H.IsField(itarget_name))) {
interface_target = &Function::ZoneHandle(
Z,
H.LookupMethodByMember(itarget_name, H.DartGetterName(itarget_name)));
@ -2548,11 +2549,10 @@ Fragment StreamingFlowGraphBuilder::BuildStaticGet(TokenPosition* p) {
inferred_type_metadata_helper_.GetInferredType(offset);
NameIndex target = ReadCanonicalNameReference(); // read target_reference.
ASSERT(H.IsGetter(target));
const Field& field = Field::ZoneHandle(
Z, H.LookupFieldByKernelGetterOrSetter(target, /*required=*/false));
if (!field.IsNull()) {
if (H.IsField(target)) {
const Field& field =
Field::ZoneHandle(Z, H.LookupFieldByKernelField(target));
if (field.is_const()) {
// Since the CFE inlines all references to const variables and fields,
// it never emits a StaticGet of a const field.
@ -2605,39 +2605,44 @@ Fragment StreamingFlowGraphBuilder::BuildStaticSet(TokenPosition* p) {
if (p != NULL) *p = position;
NameIndex target = ReadCanonicalNameReference(); // read target_reference.
ASSERT(H.IsSetter(target));
// Evaluate the expression on the right hand side.
Fragment instructions = BuildExpression(); // read expression.
if (H.IsField(target)) {
const Field& field =
Field::ZoneHandle(Z, H.LookupFieldByKernelField(target));
const Class& owner = Class::Handle(Z, field.Owner());
const String& setter_name = H.DartSetterName(target);
const Function& setter =
Function::ZoneHandle(Z, owner.LookupStaticFunction(setter_name));
Fragment instructions = BuildExpression(); // read expression.
if (NeedsDebugStepCheck(stack(), position)) {
instructions = DebugStepCheck(position) + instructions;
}
LocalVariable* variable = MakeTemporary();
instructions += LoadLocal(variable);
if (!setter.IsNull() && field.NeedsSetter()) {
instructions += StaticCall(position, setter, 1, ICData::kStatic);
instructions += Drop();
} else {
instructions += StoreStaticField(position, field);
}
return instructions;
} else {
ASSERT(H.IsProcedure(target));
// Look up the target as a setter first and, if not present, as a field
// second. This order is needed to avoid looking up a final field as the
// target.
const Function& function = Function::ZoneHandle(
Z, H.LookupStaticMethodByKernelProcedure(target, /*required=*/false));
if (!function.IsNull()) {
// Evaluate the expression on the right hand side.
Fragment instructions = BuildExpression(); // read expression.
LocalVariable* variable = MakeTemporary();
// Prepare argument.
instructions += LoadLocal(variable);
// Invoke the setter function.
const Function& function =
Function::ZoneHandle(Z, H.LookupStaticMethodByKernelProcedure(target));
instructions += StaticCall(position, function, 1, ICData::kStatic);
// Drop the unused result & leave the stored value on the stack.
return instructions + Drop();
} else {
const Field& field =
Field::ZoneHandle(Z, H.LookupFieldByKernelGetterOrSetter(target));
ASSERT(!field.NeedsSetter());
if (NeedsDebugStepCheck(stack(), position)) {
instructions = DebugStepCheck(position) + instructions;
}
LocalVariable* variable = MakeTemporary();
instructions += LoadLocal(variable);
instructions += StoreStaticField(position, field);
return instructions;
}
}
@ -2765,7 +2770,8 @@ Fragment StreamingFlowGraphBuilder::BuildMethodInvocation(TokenPosition* p) {
// TODO(dartbug.com/34497): Once front-end desugars calls via
// fields/getters, filtering of field and getter interface targets here
// can be turned into assertions.
if (!H.IsRoot(itarget_name) && !H.IsGetter(itarget_name)) {
if (!H.IsRoot(itarget_name) && !H.IsField(itarget_name) &&
!H.IsGetter(itarget_name)) {
interface_target = &Function::ZoneHandle(
Z, H.LookupMethodByMember(itarget_name,
H.DartProcedureName(itarget_name)));

View file

@ -323,7 +323,7 @@ void KernelFingerprintHelper::CalculateFunctionTypeFingerprint(bool simple) {
void KernelFingerprintHelper::CalculateGetterNameFingerprint() {
const NameIndex name = ReadCanonicalNameReference();
if (!H.IsRoot(name) && H.IsGetter(name)) {
if (!H.IsRoot(name) && (H.IsGetter(name) || H.IsField(name))) {
BuildHash(H.DartGetterName(name).Hash());
}
ReadCanonicalNameReference(); // read interface_target_origin_reference
@ -340,7 +340,7 @@ void KernelFingerprintHelper::CalculateSetterNameFingerprint() {
void KernelFingerprintHelper::CalculateMethodNameFingerprint() {
const NameIndex name =
ReadCanonicalNameReference(); // read interface_target_reference.
if (!H.IsRoot(name)) {
if (!H.IsRoot(name) && !H.IsField(name)) {
BuildHash(H.DartProcedureName(name).Hash());
}
ReadCanonicalNameReference(); // read interface_target_origin_reference

View file

@ -235,7 +235,22 @@ bool TranslationHelper::IsClass(NameIndex name) {
}
bool TranslationHelper::IsMember(NameIndex name) {
return IsConstructor(name) || IsProcedure(name);
return IsConstructor(name) || IsField(name) || IsProcedure(name);
}
bool TranslationHelper::IsField(NameIndex name) {
// Fields with private names have the import URI of the library where they are
// visible as the parent and the string "@fields" as the parent's parent.
// Fields with non-private names have the string "@fields' as the parent.
if (IsRoot(name)) {
return false;
}
NameIndex kind = CanonicalNameParent(name);
if (IsPrivate(name)) {
kind = CanonicalNameParent(kind);
}
return StringEquals(CanonicalNameString(kind), "@fields") ||
StringEquals(CanonicalNameString(kind), "@=fields");
}
bool TranslationHelper::IsConstructor(NameIndex name) {
@ -315,7 +330,7 @@ bool TranslationHelper::IsFactory(NameIndex name) {
}
NameIndex TranslationHelper::EnclosingName(NameIndex name) {
ASSERT(IsConstructor(name) || IsProcedure(name));
ASSERT(IsField(name) || IsConstructor(name) || IsProcedure(name));
NameIndex enclosing = CanonicalNameParent(CanonicalNameParent(name));
if (IsPrivate(name)) {
enclosing = CanonicalNameParent(enclosing);
@ -569,10 +584,8 @@ ClassPtr TranslationHelper::LookupClassByKernelClass(NameIndex kernel_class) {
return info_.InsertClass(thread_, name_index_handle_, klass);
}
FieldPtr TranslationHelper::LookupFieldByKernelGetterOrSetter(
NameIndex kernel_field,
bool required) {
ASSERT(IsGetter(kernel_field) || IsSetter(kernel_field));
FieldPtr TranslationHelper::LookupFieldByKernelField(NameIndex kernel_field) {
ASSERT(IsField(kernel_field));
NameIndex enclosing = EnclosingName(kernel_field);
Class& klass = Class::Handle(Z);
@ -588,15 +601,12 @@ FieldPtr TranslationHelper::LookupFieldByKernelGetterOrSetter(
Field& field = Field::Handle(
Z, klass.LookupFieldAllowPrivate(
DartSymbolObfuscate(CanonicalNameString(kernel_field))));
if (required) {
CheckStaticLookup(field);
}
CheckStaticLookup(field);
return field.ptr();
}
FunctionPtr TranslationHelper::LookupStaticMethodByKernelProcedure(
NameIndex procedure,
bool required) {
NameIndex procedure) {
const String& procedure_name = DartProcedureName(procedure);
// The parent is either a library or a class (in which case the procedure is a
@ -607,9 +617,7 @@ FunctionPtr TranslationHelper::LookupStaticMethodByKernelProcedure(
Library::Handle(Z, LookupLibraryByKernelLibrary(enclosing));
Function& function =
Function::Handle(Z, library.LookupFunctionAllowPrivate(procedure_name));
if (required) {
CheckStaticLookup(function);
}
CheckStaticLookup(function);
return function.ptr();
} else {
ASSERT(IsClass(enclosing));
@ -618,9 +626,7 @@ FunctionPtr TranslationHelper::LookupStaticMethodByKernelProcedure(
ASSERT(error == Error::null());
Function& function = Function::ZoneHandle(
Z, klass.LookupFunctionAllowPrivate(procedure_name));
if (required) {
CheckStaticLookup(function);
}
CheckStaticLookup(function);
return function.ptr();
}
}

View file

@ -101,6 +101,7 @@ class TranslationHelper {
bool IsLibrary(NameIndex name);
bool IsClass(NameIndex name);
bool IsMember(NameIndex name);
bool IsField(NameIndex name);
bool IsConstructor(NameIndex name);
bool IsProcedure(NameIndex name);
bool IsMethod(NameIndex name);
@ -163,10 +164,8 @@ class TranslationHelper {
virtual LibraryPtr LookupLibraryByKernelLibrary(NameIndex library);
virtual ClassPtr LookupClassByKernelClass(NameIndex klass);
FieldPtr LookupFieldByKernelGetterOrSetter(NameIndex field,
bool required = true);
FunctionPtr LookupStaticMethodByKernelProcedure(NameIndex procedure,
bool required = true);
FieldPtr LookupFieldByKernelField(NameIndex field);
FunctionPtr LookupStaticMethodByKernelProcedure(NameIndex procedure);
FunctionPtr LookupConstructorByKernelConstructor(NameIndex constructor);
FunctionPtr LookupConstructorByKernelConstructor(const Class& owner,
NameIndex constructor);

View file

@ -20,8 +20,8 @@ namespace kernel {
static const uint32_t kMagicProgramFile = 0x90ABCDEFu;
// Both version numbers are inclusive.
static const uint32_t kMinSupportedKernelFormatVersion = 56;
static const uint32_t kMaxSupportedKernelFormatVersion = 56;
static const uint32_t kMinSupportedKernelFormatVersion = 55;
static const uint32_t kMaxSupportedKernelFormatVersion = 55;
// Keep in sync with package:kernel/lib/binary/tag.dart
#define KERNEL_TAG_LIST(V) \