From b06e4d575450c47c48cecc4c9bdc096ff3698bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20von=20der=20Ahe=CC=81?= Date: Wed, 5 Apr 2017 16:07:31 +0200 Subject: [PATCH] Create separate scopes for constructors, setters, and other members. R=karlklose@google.com Review-Url: https://codereview.chromium.org/2788153002 . --- .../lib/src/fasta/builder/builder.dart | 4 +- .../lib/src/fasta/builder/class_builder.dart | 84 +++------ .../src/fasta/builder/library_builder.dart | 43 +++-- .../lib/src/fasta/builder/mixed_accessor.dart | 24 --- .../named_mixin_application_builder.dart | 19 +- .../lib/src/fasta/builder/prefix_builder.dart | 8 +- .../src/fasta/dill/dill_class_builder.dart | 37 ++-- .../src/fasta/dill/dill_library_builder.dart | 35 ++-- pkg/front_end/lib/src/fasta/import.dart | 9 +- .../lib/src/fasta/kernel/body_builder.dart | 30 ++-- .../lib/src/fasta/kernel/fasta_accessors.dart | 12 +- .../fasta/kernel/kernel_class_builder.dart | 14 +- .../src/fasta/kernel/kernel_enum_builder.dart | 33 ++-- .../fasta/kernel/kernel_library_builder.dart | 106 ++++++----- .../kernel_mixin_application_builder.dart | 19 +- ...ernel_named_mixin_application_builder.dart | 20 ++- .../lib/src/fasta/kernel/kernel_target.dart | 2 +- pkg/front_end/lib/src/fasta/scope.dart | 167 ++++++++++++------ .../lib/src/fasta/source/diet_listener.dart | 44 ++--- .../fasta/source/source_class_builder.dart | 95 +++++----- .../fasta/source/source_library_builder.dart | 120 +++++++------ samples/samples.status | 3 - tests/co19/co19-kernel.status | 8 - tests/language/language_kernel.status | 5 - tests/standalone/standalone.status | 35 ++-- 25 files changed, 514 insertions(+), 462 deletions(-) delete mode 100644 pkg/front_end/lib/src/fasta/builder/mixed_accessor.dart diff --git a/pkg/front_end/lib/src/fasta/builder/builder.dart b/pkg/front_end/lib/src/fasta/builder/builder.dart index 5cee11161b6..aab930a7e3a 100644 --- a/pkg/front_end/lib/src/fasta/builder/builder.dart +++ b/pkg/front_end/lib/src/fasta/builder/builder.dart @@ -46,9 +46,7 @@ export 'prefix_builder.dart' show PrefixBuilder; export 'invalid_type_builder.dart' show InvalidTypeBuilder; -export 'mixed_accessor.dart' show MixedAccessor; - -export '../scope.dart' show AccessErrorBuilder, Scope; +export '../scope.dart' show AccessErrorBuilder, Scope, ScopeBuilder; export 'builtin_type_builder.dart' show BuiltinTypeBuilder; diff --git a/pkg/front_end/lib/src/fasta/builder/class_builder.dart b/pkg/front_end/lib/src/fasta/builder/class_builder.dart index a6e892aeda2..6cbecb53381 100644 --- a/pkg/front_end/lib/src/fasta/builder/class_builder.dart +++ b/pkg/front_end/lib/src/fasta/builder/class_builder.dart @@ -15,12 +15,12 @@ import 'builder.dart' MetadataBuilder, MixinApplicationBuilder, NamedTypeBuilder, + Scope, + ScopeBuilder, TypeBuilder, TypeDeclarationBuilder, TypeVariableBuilder; -import '../scope.dart' show AccessErrorBuilder, AmbiguousBuilder, Scope; - abstract class ClassBuilder extends TypeDeclarationBuilder { final List typeVariables; @@ -29,7 +29,13 @@ abstract class ClassBuilder List interfaces; - final Map members; + final Scope scope; + + final Scope constructors; + + final ScopeBuilder scopeBuilder; + + final ScopeBuilder constructorScopeBuilder; ClassBuilder( List metadata, @@ -38,10 +44,13 @@ abstract class ClassBuilder this.typeVariables, this.supertype, this.interfaces, - this.members, + this.scope, + this.constructors, LibraryBuilder parent, int charOffset) - : super(metadata, modifiers, name, parent, charOffset); + : scopeBuilder = new ScopeBuilder(scope), + constructorScopeBuilder = new ScopeBuilder(constructors), + super(metadata, modifiers, name, parent, charOffset); /// Returns true if this class is the result of applying a mixin to its /// superclass. @@ -51,10 +60,6 @@ abstract class ClassBuilder List get constructorReferences => null; - Map get constructors; - - Map get membersInScope => members; - LibraryBuilder get library { LibraryBuilder library = parent; return library.partOfLibrary ?? library; @@ -63,70 +68,23 @@ abstract class ClassBuilder @override int resolveConstructors(LibraryBuilder library) { if (constructorReferences == null) return 0; - Scope scope = computeInstanceScope(library.scope); for (ConstructorReferenceBuilder ref in constructorReferences) { ref.resolveIn(scope); } return constructorReferences.length; } - Scope computeInstanceScope(Scope parent) { - if (typeVariables != null) { - Map local = {}; - for (TypeVariableBuilder t in typeVariables) { - local[t.name] = t; - } - parent = new Scope(local, null, parent, isModifiable: false); - } - return new Scope(membersInScope, null, parent, isModifiable: false); - } - /// Used to lookup a static member of this class. Builder findStaticBuilder(String name, int charOffset, Uri fileUri, {bool isSetter: false}) { - Builder builder = members[name]; - if (builder?.next != null) { - Builder getterBuilder; - Builder setterBuilder; - Builder current = builder; - while (current != null) { - if (current.isGetter && getterBuilder == null) { - getterBuilder = current; - } else if (current.isSetter && setterBuilder == null) { - setterBuilder = current; - } else { - return new AmbiguousBuilder(name, builder, charOffset, fileUri); - } - current = current.next; - } - if (getterBuilder?.isInstanceMember ?? false) { - getterBuilder = null; - } - if (setterBuilder?.isInstanceMember ?? false) { - setterBuilder = null; - } - builder = isSetter ? setterBuilder : getterBuilder; - if (builder == null) { - if (isSetter && getterBuilder != null) { - return new AccessErrorBuilder( - name, getterBuilder, charOffset, fileUri); - } else if (!isSetter && setterBuilder != null) { - return new AccessErrorBuilder( - name, setterBuilder, charOffset, fileUri); - } - } - } - if (builder == null) { - return null; - } else if (isSetter && builder.isGetter) { - return null; - } else { - return builder.isInstanceMember ? null : builder; - } + Builder builder = isSetter + ? scope.lookupSetter(name, charOffset, fileUri, isInstanceScope: false) + : scope.lookup(name, charOffset, fileUri, isInstanceScope: false); + return builder; } Builder findConstructorOrFactory(String name, int charOffset, Uri uri) { - return constructors[name]; + return constructors.lookup(name, charOffset, uri); } /// Returns a map which maps the type variables of [superclass] to their @@ -193,14 +151,14 @@ abstract class ClassBuilder } void forEach(void f(String name, MemberBuilder builder)) { - members.forEach(f); + scope.forEach(f); } /// Don't use for scope lookup. Only use when an element is known to exist /// (and isn't a setter). MemberBuilder operator [](String name) { // TODO(ahe): Rename this to getLocalMember. - return members[name] ?? internalError("Not found: '$name'."); + return scope.local[name] ?? internalError("Not found: '$name'."); } void addCompileTimeError(int charOffset, String message) { diff --git a/pkg/front_end/lib/src/fasta/builder/library_builder.dart b/pkg/front_end/lib/src/fasta/builder/library_builder.dart index 2010800716e..efd77e5ed09 100644 --- a/pkg/front_end/lib/src/fasta/builder/library_builder.dart +++ b/pkg/front_end/lib/src/fasta/builder/library_builder.dart @@ -22,35 +22,40 @@ import 'builder.dart' DynamicTypeBuilder, ClassBuilder, Scope, + ScopeBuilder, TypeBuilder, VoidTypeBuilder; abstract class LibraryBuilder extends Builder { + final Scope scope; + + final Scope exports; + + final ScopeBuilder scopeBuilder; + + final ScopeBuilder exportScopeBuilder; + final List exporters = []; final List compileTimeErrors = []; + final Uri fileUri; + + final String relativeFileUri; + LibraryBuilder partOfLibrary; + LibraryBuilder(Uri fileUri, this.scope, this.exports) + : fileUri = fileUri, + relativeFileUri = relativizeUri(fileUri), + scopeBuilder = new ScopeBuilder(scope), + exportScopeBuilder = new ScopeBuilder(exports), + super(null, -1, fileUri); + Loader get loader; Uri get uri; - final Uri fileUri; - final String relativeFileUri; - - Map get members; - - // TODO(ahe): Move this to SourceLibraryBuilder. - Scope get scope; - - Map get exports; - - LibraryBuilder(Uri fileUri) - : fileUri = fileUri, - relativeFileUri = relativizeUri(fileUri), - super(null, -1, fileUri); - Builder addBuilder(String name, Builder builder, int charOffset); void addExporter( @@ -104,7 +109,7 @@ abstract class LibraryBuilder extends Builder { Builder getConstructor(String className, {String constructorName, bool isPrivate: false}) { constructorName ??= ""; - Builder cls = (isPrivate ? members : exports)[className]; + Builder cls = (isPrivate ? scope : exports).lookup(className, -1, null); if (cls is ClassBuilder) { // TODO(ahe): This code is similar to code in `endNewExpression` in // `body_builder.dart`, try to share it. @@ -133,16 +138,16 @@ abstract class LibraryBuilder extends Builder { } void forEach(void f(String name, Builder builder)) { - members.forEach(f); + scope.forEach(f); } /// Don't use for scope lookup. Only use when an element is known to exist /// (and not a setter). Builder operator [](String name) { - return members[name] ?? internalError("Not found: '$name'."); + return scope.local[name] ?? internalError("Not found: '$name'."); } Builder lookup(String name, int charOffset, Uri fileUri) { - return members[name]; + return scope.lookup(name, charOffset, fileUri); } } diff --git a/pkg/front_end/lib/src/fasta/builder/mixed_accessor.dart b/pkg/front_end/lib/src/fasta/builder/mixed_accessor.dart deleted file mode 100644 index 59b65527f14..00000000000 --- a/pkg/front_end/lib/src/fasta/builder/mixed_accessor.dart +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2016, 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 fasta.mixed_accessor; - -import 'builder.dart' show Builder, LibraryBuilder; - -/// Represents the import of a getter and setter from two different libraries. -class MixedAccessor extends Builder { - final Builder getter; - final Builder setter; - - MixedAccessor(this.getter, this.setter, LibraryBuilder parent) - : super( - parent, - -1, // Synthetic element has no charOffset. - parent.fileUri) { - next = getter; - } - - @override - String get fullNameForErrors => getter.fullNameForErrors; -} diff --git a/pkg/front_end/lib/src/fasta/builder/named_mixin_application_builder.dart b/pkg/front_end/lib/src/fasta/builder/named_mixin_application_builder.dart index 1947b139848..3fff6d28595 100644 --- a/pkg/front_end/lib/src/fasta/builder/named_mixin_application_builder.dart +++ b/pkg/front_end/lib/src/fasta/builder/named_mixin_application_builder.dart @@ -6,10 +6,11 @@ library fasta.named_mixin_application_builder; import 'builder.dart' show - Builder, ClassBuilder, LibraryBuilder, + MemberBuilder, MetadataBuilder, + Scope, TypeBuilder, TypeVariableBuilder; @@ -24,8 +25,20 @@ abstract class NamedMixinApplicationBuilder List interfaces, LibraryBuilder parent, int charOffset) - : super(metadata, modifiers, name, typeVariables, supertype, interfaces, - {}, parent, charOffset); + : super( + metadata, + modifiers, + name, + typeVariables, + supertype, + interfaces, + new Scope({}, {}, + parent.scope.withTypeVariables(typeVariables), + isModifiable: false), + new Scope({}, null, null, + isModifiable: false), + parent, + charOffset); T get mixinApplication => supertype; } diff --git a/pkg/front_end/lib/src/fasta/builder/prefix_builder.dart b/pkg/front_end/lib/src/fasta/builder/prefix_builder.dart index caef2eea810..861075fffec 100644 --- a/pkg/front_end/lib/src/fasta/builder/prefix_builder.dart +++ b/pkg/front_end/lib/src/fasta/builder/prefix_builder.dart @@ -4,21 +4,21 @@ library fasta.prefix_builder; -import 'builder.dart' show Builder, LibraryBuilder; +import 'builder.dart' show Builder, LibraryBuilder, Scope; class PrefixBuilder extends Builder { final String name; - final Map exports; + final Scope exports = new Scope.top(); final LibraryBuilder parent; - PrefixBuilder(this.name, this.exports, LibraryBuilder parent, int charOffset) + PrefixBuilder(this.name, LibraryBuilder parent, int charOffset) : parent = parent, super(parent, charOffset, parent.fileUri); Builder lookup(String name, int charOffset, Uri fileUri) { - return exports[name]; + return exports.lookup(name, charOffset, fileUri); } @override diff --git a/pkg/front_end/lib/src/fasta/dill/dill_class_builder.dart b/pkg/front_end/lib/src/fasta/dill/dill_class_builder.dart index e47700af89a..52eb599699b 100644 --- a/pkg/front_end/lib/src/fasta/dill/dill_class_builder.dart +++ b/pkg/front_end/lib/src/fasta/dill/dill_class_builder.dart @@ -4,13 +4,12 @@ library fasta.dill_class_builder; -import 'package:kernel/ast.dart' - show Class, Constructor, Member, Procedure, ProcedureKind; +import 'package:kernel/ast.dart' show Class, Member; import '../errors.dart' show internalError; import '../kernel/kernel_builder.dart' - show Builder, KernelClassBuilder, KernelTypeBuilder; + show MemberBuilder, KernelClassBuilder, KernelTypeBuilder, Scope; import '../modifier.dart' show abstractMask; @@ -21,27 +20,31 @@ import 'dill_library_builder.dart' show DillLibraryBuilder; class DillClassBuilder extends KernelClassBuilder { final Class cls; - @override - final Map constructors = {}; - DillClassBuilder(Class cls, DillLibraryBuilder parent) : cls = cls, - super(null, computeModifiers(cls), cls.name, null, null, null, - {}, parent, cls.fileOffset); + super( + null, + computeModifiers(cls), + cls.name, + null, + null, + null, + new Scope({}, {}, + parent.scope, isModifiable: false), + new Scope({}, null, null, + isModifiable: false), + parent, + cls.fileOffset); void addMember(Member member) { DillMemberBuilder builder = new DillMemberBuilder(member, this); String name = member.name.name; - if (member is Constructor || - (member is Procedure && member.kind == ProcedureKind.Factory)) { - constructors[name] = builder; + if (builder.isConstructor || builder.isFactory) { + constructorScopeBuilder.addMember(name, builder); + } else if (builder.isSetter) { + scopeBuilder.addSetter(name, builder); } else { - DillMemberBuilder existing = members[name]; - if (existing == null) { - members[name] = builder; - } else { - existing.next = builder; - } + scopeBuilder.addMember(name, builder); } } diff --git a/pkg/front_end/lib/src/fasta/dill/dill_library_builder.dart b/pkg/front_end/lib/src/fasta/dill/dill_library_builder.dart index 56b8c79519b..5854e6f9326 100644 --- a/pkg/front_end/lib/src/fasta/dill/dill_library_builder.dart +++ b/pkg/front_end/lib/src/fasta/dill/dill_library_builder.dart @@ -20,7 +20,12 @@ import 'package:kernel/ast.dart' import '../errors.dart' show internalError; import '../kernel/kernel_builder.dart' - show Builder, KernelInvalidTypeBuilder, KernelTypeBuilder, LibraryBuilder; + show + Builder, + KernelInvalidTypeBuilder, + KernelTypeBuilder, + LibraryBuilder, + Scope; import '../kernel/redirecting_factory_body.dart' show RedirectingFactoryBody; @@ -33,20 +38,12 @@ import 'dill_loader.dart' show DillLoader; class DillLibraryBuilder extends LibraryBuilder { final Uri uri; - final Map members = {}; - - // TODO(ahe): Some export information needs to be serialized. - final Map exports = {}; - final DillLoader loader; Library library; - DillLibraryBuilder(Uri uri, this.loader) - : uri = uri, - super(uri); - - get scope => internalError("Scope not supported"); + DillLibraryBuilder(this.uri, this.loader) + : super(uri, new Scope.top(), new Scope.top()); Uri get fileUri => uri; @@ -81,8 +78,9 @@ class DillLibraryBuilder extends LibraryBuilder { void addMember(Member member) { String name = member.name.name; if (name == "_exports#") { + // TODO(ahe): Add this to exportScope. // This is a hack / work around for storing exports in dill files. See - // [compile_platform.dart](../compile_platform.dart). + // [compile_platform_dartk.dart](../analyzer/compile_platform_dartk.dart). } else { addBuilder(name, new DillMemberBuilder(member, this), member.fileOffset); } @@ -90,9 +88,18 @@ class DillLibraryBuilder extends LibraryBuilder { Builder addBuilder(String name, Builder builder, int charOffset) { if (name == null || name.isEmpty) return null; - members[name] = builder; + bool isSetter = builder.isSetter; + if (isSetter) { + scopeBuilder.addSetter(name, builder); + } else { + scopeBuilder.addMember(name, builder); + } if (!name.startsWith("_")) { - exports[name] = builder; + if (isSetter) { + exportScopeBuilder.addSetter(name, builder); + } else { + exportScopeBuilder.addMember(name, builder); + } } return builder; } diff --git a/pkg/front_end/lib/src/fasta/import.dart b/pkg/front_end/lib/src/fasta/import.dart index c8a943bb506..9c5b205dc0f 100644 --- a/pkg/front_end/lib/src/fasta/import.dart +++ b/pkg/front_end/lib/src/fasta/import.dart @@ -38,10 +38,13 @@ class Import { importer.addToScope(name, member, charOffset, true); }; } else { - prefix = new PrefixBuilder( - this.prefix, {}, importer, prefixCharOffset); + prefix = new PrefixBuilder(this.prefix, importer, prefixCharOffset); add = (String name, Builder member) { - prefix.exports[name] = member; + if (member.isSetter) { + prefix.exports.setters[name] = member; + } else { + prefix.exports.local[name] = member; + } }; } imported.exports.forEach((String name, Builder member) { diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart index 19cbda29b07..26554d07036 100644 --- a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart @@ -719,8 +719,7 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { this.scope.parent == enclosingScope); // This deals with this kind of initializer: `C(a) : a = a;` Scope scope = inInitializer ? enclosingScope : this.scope; - Builder builder = scope.lookup(name, token.charOffset, uri); - push(builderToFirstExpression(builder, name, token.charOffset)); + push(scopeLookup(scope, name, token.charOffset)); return; } else if (context.inDeclaration) { if (context == IdentifierContext.topLevelVariableDeclaration || @@ -735,12 +734,17 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { push(new Identifier(name)..fileOffset = token.charOffset); } + /// Look up [name] in [scope] using [charOffset] to report any + /// problems. [isQualified] should be true if [name] is a qualified access + /// (which implies that it shouldn't be turned into a [ThisPropertyAccessor] + /// if the name doesn't resolve in the scope). @override - builderToFirstExpression(Builder builder, String name, int charOffset, - {bool isPrefix: false}) { + scopeLookup(Scope scope, String name, int charOffset, + {bool isQualified: false}) { + Builder builder = scope.lookup(name, charOffset, uri); if (builder == null || (!isInstanceContext && builder.isInstanceMember)) { Name n = new Name(name, library.library); - if (!isPrefix && isInstanceContext) { + if (!isQualified && isInstanceContext) { assert(builder == null); if (constantExpressionRequired) { addCompileTimeError(charOffset, "Not a constant expression."); @@ -776,12 +780,6 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { return new StaticAccessor(this, charOffset, builder.target, null); } else if (builder is PrefixBuilder) { return builder; - } else if (builder is MixedAccessor) { - if (constantExpressionRequired && !builder.getter.target.isConst) { - addCompileTimeError(charOffset, "Not a constant expression."); - } - return new StaticAccessor( - this, charOffset, builder.getter.target, builder.setter.target); } else { if (builder.hasProblem && builder is! AccessErrorBuilder) return builder; Builder setter; @@ -1278,9 +1276,8 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { builder = scope.lookup(prefix, beginToken.charOffset, uri); } if (builder is PrefixBuilder) { - name = builderToFirstExpression( - builder.exports[suffix], suffix, beginToken.charOffset, - isPrefix: true); + name = scopeLookup(builder.exports, suffix, beginToken.charOffset, + isQualified: true); } else { push(const DynamicType()); addCompileTimeError(beginToken.charOffset, @@ -1658,9 +1655,8 @@ class BodyBuilder extends ScopeListener implements BuilderHelper { var prefix = type[0]; identifier = type[1]; if (prefix is PrefixBuilder) { - // TODO(ahe): Handle privacy in prefix.exports. - type = builderToFirstExpression( - prefix.exports[identifier.name], identifier.name, start.charOffset); + type = scopeLookup(prefix.exports, identifier.name, start.charOffset, + isQualified: true); identifier = null; } else if (prefix is ClassBuilder) { type = prefix; diff --git a/pkg/front_end/lib/src/fasta/kernel/fasta_accessors.dart b/pkg/front_end/lib/src/fasta/kernel/fasta_accessors.dart index 25336a6ff9b..b4659448d0f 100644 --- a/pkg/front_end/lib/src/fasta/kernel/fasta_accessors.dart +++ b/pkg/front_end/lib/src/fasta/kernel/fasta_accessors.dart @@ -12,7 +12,7 @@ import 'package:kernel/ast.dart'; import '../errors.dart' show internalError; -import '../scope.dart' show AccessErrorBuilder, ProblemBuilder; +import '../scope.dart' show AccessErrorBuilder, ProblemBuilder, Scope; import 'frontend_accessors.dart' as kernel show @@ -43,7 +43,7 @@ abstract class BuilderHelper { Member lookupSuperMember(Name name, {bool isSetter: false}); - builderToFirstExpression(Builder builder, String name, int offset); + scopeLookup(Scope scope, String name, int offset, {bool isQualified: false}); finishSend(Object receiver, Arguments arguments, int offset); @@ -431,8 +431,8 @@ class SendAccessor extends IncompleteSend { } if (receiver is PrefixBuilder) { PrefixBuilder prefix = receiver; - receiver = helper.builderToFirstExpression( - prefix.exports[name.name], "${prefix.name}.${name.name}", offset); + receiver = helper.scopeLookup(prefix.exports, name.name, offset, + isQualified: true); return helper.finishSend(receiver, arguments, offset); } Expression result; @@ -517,8 +517,8 @@ class IncompletePropertyAccessor extends IncompleteSend { } if (receiver is PrefixBuilder) { PrefixBuilder prefix = receiver; - return helper.builderToFirstExpression( - prefix.exports[name.name], name.name, offset); + return helper.scopeLookup(prefix.exports, name.name, offset, + isQualified: true); } if (receiver is KernelClassBuilder) { Builder builder = receiver.findStaticBuilder(name.name, offset, uri); diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart index 209699dac7c..b81da23bd02 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart @@ -42,8 +42,10 @@ import 'kernel_builder.dart' KernelProcedureBuilder, KernelTypeBuilder, LibraryBuilder, + MemberBuilder, MetadataBuilder, ProcedureBuilder, + Scope, TypeVariableBuilder, computeDefaultTypeArguments; @@ -58,11 +60,12 @@ abstract class KernelClassBuilder List typeVariables, KernelTypeBuilder supertype, List interfaces, - Map members, + Scope scope, + Scope constructors, LibraryBuilder parent, int charOffset) : super(metadata, modifiers, name, typeVariables, supertype, interfaces, - members, parent, charOffset); + scope, constructors, parent, charOffset); Class get cls; @@ -111,10 +114,11 @@ abstract class KernelClassBuilder int resolveConstructors(LibraryBuilder library) { int count = super.resolveConstructors(library); if (count != 0) { + Map constructors = this.constructors.local; // Copy keys to avoid concurrent modification error. - List names = members.keys.toList(); + List names = constructors.keys.toList(); for (String name in names) { - Builder builder = members[name]; + Builder builder = constructors[name]; if (builder is KernelProcedureBuilder && builder.isFactory) { // Compute the immediate redirection target, not the effective. ConstructorReferenceBuilder redirectionTarget = @@ -159,7 +163,7 @@ abstract class KernelClassBuilder // // TODO(ahe): Generate the correct factory body instead. DillMemberBuilder constructorsField = - members.putIfAbsent("_redirecting#", () { + scope.local.putIfAbsent("_redirecting#", () { ListLiteral literal = new ListLiteral([]); Name name = new Name("_redirecting#", library.library); Field field = new Field(name, diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart index 8a897098855..6264d7e4ea5 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart @@ -32,7 +32,9 @@ import '../errors.dart' show inputError; import '../modifier.dart' show constMask, finalMask, staticMask; -import "../source/source_class_builder.dart" show SourceClassBuilder; +import '../names.dart' show indexGetName; + +import '../source/source_class_builder.dart' show SourceClassBuilder; import 'kernel_builder.dart' show @@ -48,9 +50,8 @@ import 'kernel_builder.dart' KernelTypeBuilder, LibraryBuilder, MemberBuilder, - MetadataBuilder; - -import '../names.dart' show indexGetName; + MetadataBuilder, + Scope; class KernelEnumBuilder extends SourceClassBuilder implements EnumBuilder { @@ -65,7 +66,8 @@ class KernelEnumBuilder extends SourceClassBuilder KernelEnumBuilder.internal( List metadata, String name, - Map members, + Scope scope, + Scope constructors, Class cls, this.constantNamesAndOffsets, this.toStringMap, @@ -73,8 +75,8 @@ class KernelEnumBuilder extends SourceClassBuilder this.stringType, LibraryBuilder parent, int charOffset) - : super(metadata, 0, name, null, null, null, members, parent, null, - charOffset, cls); + : super(metadata, 0, name, null, null, null, scope, constructors, parent, + null, charOffset, cls); factory KernelEnumBuilder( List metadata, @@ -91,7 +93,8 @@ class KernelEnumBuilder extends SourceClassBuilder KernelTypeBuilder stringType = parent.addType( new KernelNamedTypeBuilder("String", null, charOffset, parent.fileUri)); Class cls = new Class(name: name); - Map members = {}; + Map members = {}; + Map constructors = {}; KernelNamedTypeBuilder selfType = new KernelNamedTypeBuilder(name, null, charOffset, parent.fileUri); KernelTypeBuilder listType = parent.addType(new KernelNamedTypeBuilder( @@ -123,7 +126,7 @@ class KernelEnumBuilder extends SourceClassBuilder charOffset, charOffset, charEndOffset); - members[""] = constructorBuilder; + constructors[""] = constructorBuilder; int index = 0; List toStringEntries = []; KernelFieldBuilder valuesBuilder = new KernelFieldBuilder( @@ -162,7 +165,8 @@ class KernelEnumBuilder extends SourceClassBuilder KernelEnumBuilder enumBuilder = new KernelEnumBuilder.internal( metadata, name, - members, + new Scope(members, null, parent.scope, isModifiable: false), + new Scope(constructors, null, null, isModifiable: false), cls, constantNamesAndOffsets, toStringMap, @@ -171,10 +175,13 @@ class KernelEnumBuilder extends SourceClassBuilder parent, charOffset); // TODO(sigmund): dynamic should be `covariant MemberBuilder`. - members.forEach((String name, dynamic b) { + void setParent(String name, dynamic b) { MemberBuilder builder = b; builder.parent = enumBuilder; - }); + } + + members.forEach(setParent); + constructors.forEach(setParent); selfType.builder = enumBuilder; return enumBuilder; } @@ -212,7 +219,7 @@ class KernelEnumBuilder extends SourceClassBuilder valuesBuilder.build(libraryBuilder); valuesBuilder.initializer = new ListLiteral(values, typeArgument: cls.rawType, isConst: true); - KernelConstructorBuilder constructorBuilder = this[""]; + KernelConstructorBuilder constructorBuilder = constructorScopeBuilder[""]; Constructor constructor = constructorBuilder.build(libraryBuilder); constructor.initializers.insert( 0, diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart index d7598fd0569..45b17e597b9 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart @@ -46,10 +46,10 @@ import 'kernel_builder.dart' KernelTypeVariableBuilder, MemberBuilder, MetadataBuilder, - MixedAccessor, NamedMixinApplicationBuilder, PrefixBuilder, ProcedureBuilder, + Scope, TypeBuilder, TypeVariableBuilder, compareProcedures; @@ -103,6 +103,17 @@ class KernelLibraryBuilder int charOffset) { assert(currentDeclaration.parent == libraryDeclaration); Map members = currentDeclaration.members; + Map constructors = currentDeclaration.constructors; + Map setters = currentDeclaration.setters; + + Scope classScope = new Scope( + members, setters, scope.withTypeVariables(typeVariables), + isModifiable: false); + + // When looking up a constructor, we don't consider type variables or the + // library scope. + Scope constructorScope = + new Scope(constructors, null, null, isModifiable: false); ClassBuilder cls = new SourceClassBuilder( metadata, modifiers, @@ -110,17 +121,22 @@ class KernelLibraryBuilder typeVariables, supertype, interfaces, - members, + classScope, + constructorScope, this, new List.from(constructorReferences), charOffset); constructorReferences.clear(); - members.forEach((String name, MemberBuilder builder) { - while (builder != null) { - builder.parent = cls; - builder = builder.next; + void setParent(String name, MemberBuilder member) { + while (member != null) { + member.parent = cls; + member = member.next; } - }); + } + + members.forEach(setParent); + constructors.forEach(setParent); + setters.forEach(setParent); // Nested declaration began in `OutlineBuilder.beginClassDeclaration`. endNestedDeclaration().resolveTypes(typeVariables, this); addBuilder(className, cls, charOffset); @@ -151,10 +167,28 @@ class KernelLibraryBuilder charOffset); } - String computeConstructorName(String name) { - assert(isConstructorName(name, currentDeclaration.name)); + String computeAndValidateConstructorName(String name, int charOffset) { + String className = currentDeclaration.name; + bool startsWithClassName = name.startsWith(className); + if (startsWithClassName && name.length == className.length) { + // Unnamed constructor or factory. + return ""; + } int index = name.indexOf("."); - return index == -1 ? "" : name.substring(index + 1); + if (startsWithClassName && index == className.length) { + // Named constructor or factory. + return name.substring(index + 1); + } + if (index == -1) { + // A legal name for a regular method, but not for a constructor. + return null; + } + String suffix = name.substring(index + 1); + addCompileTimeError( + charOffset, + "'$name' isn't a legal method name.\n" + "Did you mean '$className.$suffix'?"); + return suffix; } void addProcedure( @@ -175,8 +209,10 @@ class KernelLibraryBuilder // `OutlineBuilder.beginTopLevelMethod`. endNestedDeclaration().resolveTypes(typeVariables, this); ProcedureBuilder procedure; - if (!isTopLevel && isConstructorName(name, currentDeclaration.name)) { - name = computeConstructorName(name); + String constructorName = + isTopLevel ? null : computeAndValidateConstructorName(name, charOffset); + if (constructorName != null) { + name = constructorName; procedure = new KernelConstructorBuilder( metadata, modifiers & ~abstractMask, @@ -214,7 +250,7 @@ class KernelLibraryBuilder void addFactoryMethod( List metadata, int modifiers, - ConstructorReferenceBuilder constructorName, + ConstructorReferenceBuilder constructorNameReference, List formals, AsyncMarker asyncModifier, ConstructorReferenceBuilder redirectionTarget, @@ -225,11 +261,13 @@ class KernelLibraryBuilder // Nested declaration began in `OutlineBuilder.beginFactoryMethod`. DeclarationBuilder factoryDeclaration = endNestedDeclaration(); - String name = constructorName.name; - if (isConstructorName(name, currentDeclaration.name)) { - name = computeConstructorName(name); + String name = constructorNameReference.name; + String constructorName = + computeAndValidateConstructorName(name, charOffset); + if (constructorName != null) { + name = constructorName; } - assert(constructorName.suffix == null); + assert(constructorNameReference.suffix == null); KernelProcedureBuilder procedure = new KernelProcedureBuilder( metadata, staticMask | modifiers, @@ -335,6 +373,7 @@ class KernelLibraryBuilder Builder buildAmbiguousBuilder( String name, Builder builder, Builder other, int charOffset, {bool isExport: false, bool isImport: false}) { + // TODO(ahe): Can I move this to Scope or Prefix? if (builder == other) return builder; if (builder is InvalidTypeBuilder) return builder; if (other is InvalidTypeBuilder) return other; @@ -352,7 +391,7 @@ class KernelLibraryBuilder Uri otherUri; Uri preferredUri; Uri hiddenUri; - if (members[name] == builder) { + if (scope.local[name] == builder) { isLocal = true; preferred = builder; hiddenUri = other.computeLibraryUri(); @@ -394,26 +433,15 @@ class KernelLibraryBuilder return preferred; } if (builder.next == null && other.next == null) { - if (builder.isGetter && other.isSetter) { - return new MixedAccessor(builder, other, this); - } else if (builder.isSetter && other.isGetter) { - return new MixedAccessor(other, builder, this); - } if (isImport && builder is PrefixBuilder && other is PrefixBuilder) { // Handles the case where the same prefix is used for different // imports. - PrefixBuilder prefix = builder; - other.exports.forEach((String name, Builder member) { - Builder existing = exports[name]; - if (existing != null) { - if (existing != member) { - member = buildAmbiguousBuilder(name, existing, member, charOffset, - isExport: isExport, isImport: isImport); - } - } - prefix.exports[name] = member; - }); - return builder; + return builder + ..exports.merge(other.exports, + (String name, Builder existing, Builder member) { + return buildAmbiguousBuilder(name, existing, member, charOffset, + isExport: isExport, isImport: isImport); + }); } } if (isExport) { @@ -515,11 +543,3 @@ class KernelLibraryBuilder assert(mixinApplicationClasses.isEmpty); } } - -bool isConstructorName(String name, String className) { - if (name.startsWith(className)) { - if (name.length == className.length) return true; - if (name.startsWith(".", className.length)) return true; - } - return false; -} diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_mixin_application_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_mixin_application_builder.dart index 8b9d903425d..9c46ba8cda3 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_mixin_application_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_mixin_application_builder.dart @@ -8,23 +8,24 @@ import 'package:kernel/ast.dart' show InterfaceType, Supertype, setParents; import '../modifier.dart' show abstractMask; +import '../source/source_class_builder.dart' show SourceClassBuilder; + +import '../util/relativize.dart' show relativizeUri; + import 'kernel_builder.dart' show - Builder, ConstructorReferenceBuilder, KernelLibraryBuilder, KernelNamedTypeBuilder, KernelTypeBuilder, KernelTypeVariableBuilder, LibraryBuilder, + MemberBuilder, MixinApplicationBuilder, + Scope, TypeBuilder, TypeVariableBuilder; -import '../util/relativize.dart' show relativizeUri; - -import '../source/source_class_builder.dart' show SourceClassBuilder; - class KernelMixinApplicationBuilder extends MixinApplicationBuilder implements KernelTypeBuilder { @@ -93,7 +94,10 @@ class KernelMixinApplicationBuilder newTypeVariables, supertype, null, - {}, + new Scope({}, {}, + library.scope.withTypeVariables(newTypeVariables), + isModifiable: false), + new Scope({}, null, null, isModifiable: false), library, [], charOffset, @@ -109,6 +113,7 @@ class KernelMixinApplicationBuilder return cls; }); return new KernelNamedTypeBuilder( - name, typeArguments, charOffset, library.fileUri)..builder = cls; + name, typeArguments, charOffset, library.fileUri) + ..builder = cls; } } diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_named_mixin_application_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_named_mixin_application_builder.dart index 03786bd6144..5ad745e157c 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_named_mixin_application_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_named_mixin_application_builder.dart @@ -10,11 +10,12 @@ import '../source/source_class_builder.dart' show SourceClassBuilder; import 'kernel_builder.dart' show - Builder, KernelTypeBuilder, LibraryBuilder, + MemberBuilder, MetadataBuilder, NamedMixinApplicationBuilder, + Scope, TypeVariableBuilder; class KernelNamedMixinApplicationBuilder extends SourceClassBuilder @@ -28,8 +29,21 @@ class KernelNamedMixinApplicationBuilder extends SourceClassBuilder List interfaces, LibraryBuilder parent, int charOffset) - : super(metadata, modifiers, name, typeVariables, mixinApplication, - interfaces, {}, parent, null, charOffset); + : super( + metadata, + modifiers, + name, + typeVariables, + mixinApplication, + interfaces, + new Scope({}, {}, + parent.scope.withTypeVariables(typeVariables), + isModifiable: false), + new Scope({}, null, null, + isModifiable: false), + parent, + null, + charOffset); KernelTypeBuilder get mixinApplication => supertype; diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart index 831dbf62cc2..ab66f3da581 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart @@ -428,7 +428,7 @@ class KernelTarget extends TargetImplementation { // to add a default constructor to complete error recovery. return; } - if (builder.constructors.isNotEmpty) return; + if (builder.constructors.local.isNotEmpty) return; /// Quotes below are from [Dart Programming Language Specification, 4th /// Edition]( diff --git a/pkg/front_end/lib/src/fasta/scope.dart b/pkg/front_end/lib/src/fasta/scope.dart index c477ba95b30..2dd1656ef0b 100644 --- a/pkg/front_end/lib/src/fasta/scope.dart +++ b/pkg/front_end/lib/src/fasta/scope.dart @@ -4,21 +4,25 @@ library fasta.scope; -import 'builder/builder.dart' show Builder, MixedAccessor; +import 'builder/builder.dart' show Builder, TypeVariableBuilder; import 'errors.dart' show internalError; -class Scope { +class MutableScope { /// Names declared in this scope. - final Map local; + Map local; /// Setters declared in this scope. - final Map setters; + Map setters; /// The scope that this scope is nested within, or `null` if this is the top /// level scope. - final Scope parent; + Scope parent; + MutableScope(this.local, this.setters, this.parent); +} + +class Scope extends MutableScope { /// Indicates whether an attempt to declare new names in this scope should /// succeed. final bool isModifiable; @@ -27,9 +31,9 @@ class Scope { Map forwardDeclaredLabels; - Scope(this.local, Map setters, this.parent, + Scope(Map local, Map setters, Scope parent, {this.isModifiable: true}) - : setters = setters ?? const {}; + : super(local, setters = setters ?? const {}, parent); Scope.top({bool isModifiable: false}) : this({}, {}, null, @@ -42,10 +46,38 @@ class Scope { Scope.nested(Scope parent, {bool isModifiable: true}) : this({}, null, parent, isModifiable: isModifiable); + /// Don't use this. Use [becomePartOf] instead. + void set local(_) => internalError("Unsupported operation."); + + /// Don't use this. Use [becomePartOf] instead. + void set setters(_) => internalError("Unsupported operation."); + + /// Don't use this. Use [becomePartOf] instead. + void set parent(_) => internalError("Unsupported operation."); + + /// This scope becomes equivalent to [scope]. This is used for parts to + /// become part of their library's scope. + void becomePartOf(Scope scope) { + assert(parent.parent == null); + assert(scope.parent.parent == null); + super.local = scope.local; + super.setters = scope.setters; + super.parent = scope.parent; + } + Scope createNestedScope({bool isModifiable: true}) { return new Scope.nested(this, isModifiable: isModifiable); } + Scope withTypeVariables(List typeVariables) { + if (typeVariables == null) return this; + Scope newScope = new Scope.nested(this, isModifiable: false); + for (TypeVariableBuilder t in typeVariables) { + newScope.local[t.name] = t; + } + return newScope; + } + /// Create a special scope for use by labeled staments. This scope doesn't /// introduce a new scope for local variables, only for labels. This deals /// with corner cases like this: @@ -57,66 +89,49 @@ class Scope { return new Scope(local, setters, parent, isModifiable: true); } + Builder lookupIn(String name, int charOffset, Uri fileUri, + Map map, bool isInstanceScope) { + Builder builder = map[name]; + if (builder == null) return null; + if (builder.next != null) { + return new AmbiguousBuilder(name, builder, charOffset, fileUri); + } else if (!isInstanceScope && builder.isInstanceMember) { + return null; + } else { + return builder; + } + } + Builder lookup(String name, int charOffset, Uri fileUri, {bool isInstanceScope: true}) { - Builder builder = local[name]; - if (builder != null) { - if (builder.next != null) { - return lookupAmbiguous(name, builder, false, charOffset, fileUri); - } - return builder.isSetter - ? new AccessErrorBuilder(name, builder, charOffset, fileUri) - : builder; - } else { - return parent?.lookup(name, charOffset, fileUri); + Builder builder = + lookupIn(name, charOffset, fileUri, local, isInstanceScope); + if (builder != null) return builder; + builder = lookupIn(name, charOffset, fileUri, setters, isInstanceScope); + if (builder != null && !builder.hasProblem) { + return new AccessErrorBuilder(name, builder, charOffset, fileUri); } + if (!isInstanceScope) { + // For static lookup, do not seach the parent scope. + return builder; + } + return builder ?? parent?.lookup(name, charOffset, fileUri); } Builder lookupSetter(String name, int charOffset, Uri fileUri, {bool isInstanceScope: true}) { - Builder builder = local[name]; - if (builder != null) { - if (builder.next != null) { - return lookupAmbiguous(name, builder, true, charOffset, fileUri); - } - if (builder.isField) { - if (builder.isFinal) { - return new AccessErrorBuilder(name, builder, charOffset, fileUri); - } else { - return builder; - } - } else if (builder.isSetter) { - return builder; - } else { - return new AccessErrorBuilder(name, builder, charOffset, fileUri); - } - } else { - return parent?.lookupSetter(name, charOffset, fileUri); + Builder builder = + lookupIn(name, charOffset, fileUri, setters, isInstanceScope); + if (builder != null) return builder; + builder = lookupIn(name, charOffset, fileUri, local, isInstanceScope); + if (builder != null && !builder.hasProblem) { + return new AccessErrorBuilder(name, builder, charOffset, fileUri); } - } - - Builder lookupAmbiguous( - String name, Builder builder, bool setter, int charOffset, Uri fileUri) { - assert(builder.next != null); - if (builder is MixedAccessor) { - return setter ? builder.setter : builder.getter; + if (!isInstanceScope) { + // For static lookup, do not seach the parent scope. + return builder; } - Builder setterBuilder; - Builder getterBuilder; - Builder current = builder; - while (current != null) { - if (current.isGetter && getterBuilder == null) { - getterBuilder = current; - } else if (current.isSetter && setterBuilder == null) { - setterBuilder = current; - } else { - return new AmbiguousBuilder(name, builder, charOffset, fileUri); - } - current = current.next; - } - assert(getterBuilder != null); - assert(setterBuilder != null); - return setter ? setterBuilder : getterBuilder; + return builder ?? parent?.lookupSetter(name, charOffset, fileUri); } bool hasLocalLabel(String name) => labels != null && labels.containsKey(name); @@ -161,8 +176,28 @@ class Scope { } } + void merge(Scope scope, + buildAmbiguousBuilder(String name, Builder existing, Builder member)) { + Map map = local; + + void mergeMember(String name, Builder member) { + Builder existing = map[name]; + if (existing != null) { + if (existing != member) { + member = buildAmbiguousBuilder(name, existing, member); + } + } + map[name] = member; + } + + scope.local.forEach(mergeMember); + map = setters; + scope.setters.forEach(mergeMember); + } + void forEach(f(String name, Builder member)) { local.forEach(f); + setters.forEach(f); } String get debugString { @@ -188,6 +223,22 @@ class Scope { } } +class ScopeBuilder { + final Scope scope; + + ScopeBuilder(this.scope); + + void addMember(String name, Builder builder) { + scope.local[name] = builder; + } + + void addSetter(String name, Builder builder) { + scope.setters[name] = builder; + } + + Builder operator [](String name) => scope.local[name]; +} + abstract class ProblemBuilder extends Builder { final String name; diff --git a/pkg/front_end/lib/src/fasta/source/diet_listener.dart b/pkg/front_end/lib/src/fasta/source/diet_listener.dart index 0b7b385aece..42c137d7dd4 100644 --- a/pkg/front_end/lib/src/fasta/source/diet_listener.dart +++ b/pkg/front_end/lib/src/fasta/source/diet_listener.dart @@ -31,8 +31,6 @@ import '../builder/builder.dart'; import 'source_library_builder.dart' show SourceLibraryBuilder; -import '../kernel/kernel_library_builder.dart' show isConstructorName; - class DietListener extends StackListener { final SourceLibraryBuilder library; @@ -420,7 +418,7 @@ class DietListener extends StackListener { assert(currentClass == null); currentClass = lookupBuilder(token, null, name); assert(memberScope == library.scope); - memberScope = currentClass.computeInstanceScope(memberScope); + memberScope = currentClass.scope; } @override @@ -516,37 +514,33 @@ class DietListener extends StackListener { } Builder lookupBuilder(Token token, Token getOrSet, String name) { + // TODO(ahe): Can I move this to Scope or ScopeBuilder? Builder builder; if (currentClass != null) { - builder = currentClass.members[name]; - if (builder == null && isConstructorName(name, currentClass.name)) { - int index = name.indexOf("."); - name = index == -1 ? "" : name.substring(index + 1); - builder = currentClass.members[name]; + if (getOrSet != null && optional("set", getOrSet)) { + builder = currentClass.scope.setters[name]; + } else { + builder = currentClass.scope.local[name]; } + if (builder == null) { + if (name == currentClass.name) { + name = ""; + } else { + int index = name.indexOf("."); + name = name.substring(index + 1); + } + builder = currentClass.constructors.local[name]; + } + } else if (getOrSet != null && optional("set", getOrSet)) { + builder = library.scope.setters[name]; } else { - builder = library.members[name]; + builder = library.scopeBuilder[name]; } if (builder == null) { return internalError("Builder not found: $name", uri, token.charOffset); } if (builder.next != null) { - Builder getterBuilder; - Builder setterBuilder; - Builder current = builder; - while (current != null) { - if (current.isGetter && getterBuilder == null) { - getterBuilder = current; - } else if (current.isSetter && setterBuilder == null) { - setterBuilder = current; - } else { - return inputError(uri, token.charOffset, "Duplicated name: $name"); - } - current = current.next; - } - assert(getOrSet != null); - if (optional("get", getOrSet)) return getterBuilder; - if (optional("set", getOrSet)) return setterBuilder; + return inputError(uri, token.charOffset, "Duplicated name: $name"); } return builder; } diff --git a/pkg/front_end/lib/src/fasta/source/source_class_builder.dart b/pkg/front_end/lib/src/fasta/source/source_class_builder.dart index de5cb0fced6..81f8a0b55d3 100644 --- a/pkg/front_end/lib/src/fasta/source/source_class_builder.dart +++ b/pkg/front_end/lib/src/fasta/source/source_class_builder.dart @@ -21,7 +21,7 @@ import '../kernel/kernel_builder.dart' KernelTypeVariableBuilder, LibraryBuilder, MetadataBuilder, - ProcedureBuilder, + Scope, TypeVariableBuilder, compareProcedures; @@ -42,11 +42,6 @@ Class initializeClass( class SourceClassBuilder extends KernelClassBuilder { final Class cls; - @override - final Map constructors; - - final Map membersInScope; - final List constructorReferences; final KernelTypeBuilder mixedInType; @@ -58,17 +53,16 @@ class SourceClassBuilder extends KernelClassBuilder { List typeVariables, KernelTypeBuilder supertype, List interfaces, - Map members, + Scope scope, + Scope constructors, LibraryBuilder parent, this.constructorReferences, int charOffset, [Class cls, this.mixedInType]) : cls = initializeClass(cls, name, parent, charOffset), - membersInScope = computeMembersInScope(members), - constructors = computeConstructors(members), super(metadata, modifiers, name, typeVariables, supertype, interfaces, - members, parent, charOffset); + scope, constructors, parent, charOffset); int resolveTypes(LibraryBuilder library) { int count = 0; @@ -83,24 +77,23 @@ class SourceClassBuilder extends KernelClassBuilder { } Class build(KernelLibraryBuilder library) { - void buildBuilder(Builder builder) { - if (builder is KernelFieldBuilder) { - // TODO(ahe): It would be nice to have a common interface for the build - // method to avoid duplicating these two cases. - cls.addMember(builder.build(library)); - } else if (builder is KernelFunctionBuilder) { - cls.addMember(builder.build(library)); - } else { - internalError("Unhandled builder: ${builder.runtimeType}"); - } - } - - members.forEach((String name, Builder builder) { + void buildBuilders(String name, Builder builder) { do { - buildBuilder(builder); + if (builder is KernelFieldBuilder) { + // TODO(ahe): It would be nice to have a common interface for the + // build method to avoid duplicating these two cases. + cls.addMember(builder.build(library)); + } else if (builder is KernelFunctionBuilder) { + cls.addMember(builder.build(library)); + } else { + internalError("Unhandled builder: ${builder.runtimeType}"); + } builder = builder.next; } while (builder != null); - }); + } + + scope.forEach(buildBuilders); + constructors.forEach(buildBuilders); cls.supertype = supertype?.buildSupertype(library); cls.mixedInType = mixedInType?.buildSupertype(library); // TODO(ahe): If `cls.supertype` is null, and this isn't Object, report a @@ -116,6 +109,32 @@ class SourceClassBuilder extends KernelClassBuilder { } } + constructors.forEach((String name, Builder constructor) { + Builder member = scopeBuilder[name]; + if (member == null) return; + // TODO(ahe): charOffset is missing. + addCompileTimeError( + constructor.charOffset, "Conflicts with member '${name}'."); + if (constructor.isFactory) { + addCompileTimeError(member.charOffset, + "Conflicts with factory '${this.name}.${name}'."); + } else { + addCompileTimeError(member.charOffset, + "Conflicts with constructor '${this.name}.${name}'."); + } + }); + + scope.setters.forEach((String name, Builder setter) { + Builder member = scopeBuilder[name]; + if (member == null || !member.isField || member.isFinal) return; + // TODO(ahe): charOffset is missing. + var report = member.isInstanceMember != setter.isInstanceMember + ? addWarning + : addCompileTimeError; + report(setter.charOffset, "Conflicts with member '${name}'."); + report(member.charOffset, "Conflicts with setter '${name}'."); + }); + cls.procedures.sort(compareProcedures); return cls; } @@ -125,29 +144,7 @@ class SourceClassBuilder extends KernelClassBuilder { cls.constructors.add(constructor); constructor.parent = cls; DillMemberBuilder memberBuilder = new DillMemberBuilder(constructor, this); - memberBuilder.next = constructors[name]; - constructors[name] = memberBuilder; + memberBuilder.next = constructorScopeBuilder[name]; + constructorScopeBuilder.addMember(name, memberBuilder); } } - -Map computeMembersInScope(Map members) { - Map membersInScope = {}; - members.forEach((String name, Builder builder) { - if (builder is ProcedureBuilder) { - if (builder.isConstructor || builder.isFactory) return; - } - membersInScope[name] = builder; - }); - return membersInScope; -} - -Map computeConstructors(Map members) { - Map constructors = {}; - members.forEach((String name, Builder builder) { - if (builder is ProcedureBuilder && - (builder.isConstructor || builder.isFactory)) { - constructors[name] = builder; - } - }); - return constructors; -} diff --git a/pkg/front_end/lib/src/fasta/source/source_library_builder.dart b/pkg/front_end/lib/src/fasta/source/source_library_builder.dart index 8ee14d6c82a..c1f800081db 100644 --- a/pkg/front_end/lib/src/fasta/source/source_library_builder.dart +++ b/pkg/front_end/lib/src/fasta/source/source_library_builder.dart @@ -38,8 +38,7 @@ abstract class SourceLibraryBuilder extends LibraryBuilder { final SourceLoader loader; - final DeclarationBuilder libraryDeclaration = - new DeclarationBuilder({}, null); + final DeclarationBuilder libraryDeclaration; final List constructorReferences = []; @@ -48,9 +47,7 @@ abstract class SourceLibraryBuilder final List imports = []; - final Map exports = {}; - - final Scope scope = new Scope.top(); + final Scope importScope; final Uri fileUri; @@ -72,18 +69,20 @@ abstract class SourceLibraryBuilder bool canAddImplementationBuilders = false; - SourceLibraryBuilder(this.loader, Uri fileUri) - : fileUri = fileUri, - super(fileUri) { - currentDeclaration = libraryDeclaration; - } + SourceLibraryBuilder(SourceLoader loader, Uri fileUri) + : this.fromScopes(loader, fileUri, new DeclarationBuilder.library(), + new Scope.top()); + + SourceLibraryBuilder.fromScopes( + this.loader, this.fileUri, this.libraryDeclaration, this.importScope) + : currentDeclaration = libraryDeclaration, + super( + fileUri, libraryDeclaration.toScope(importScope), new Scope.top()); Uri get uri; bool get isPart => partOfName != null || partOfUri != null; - Map get members => libraryDeclaration.members; - List get types => libraryDeclaration.types; T addNamedType(String name, List arguments, int charOffset); @@ -105,9 +104,8 @@ abstract class SourceLibraryBuilder return ref; } - void beginNestedDeclaration(String name, {bool hasMembers}) { - currentDeclaration = new DeclarationBuilder( - {}, name, currentDeclaration); + void beginNestedDeclaration(String name, {bool hasMembers: true}) { + currentDeclaration = currentDeclaration.createNested(name, hasMembers); } DeclarationBuilder endNestedDeclaration() { @@ -234,12 +232,6 @@ abstract class SourceLibraryBuilder TypeVariableBuilder addTypeVariable(String name, T bound, int charOffset); Builder addBuilder(String name, Builder builder, int charOffset) { - if (name.indexOf(".") != -1 && name.indexOf("&") == -1) { - addCompileTimeError( - charOffset, - "Only constructors and factories can have" - " names containing a period ('.'): $name"); - } // TODO(ahe): Set the parent correctly here. Could then change the // implementation of MemberBuilder.isTopLevel to test explicitly for a // LibraryBuilder. @@ -256,19 +248,22 @@ abstract class SourceLibraryBuilder } else { assert(currentDeclaration.parent == libraryDeclaration); } - Map members = currentDeclaration.members; + bool isConstructor = builder is ProcedureBuilder && + (builder.isConstructor || builder.isFactory); + Map members = isConstructor + ? currentDeclaration.constructors + : (builder.isSetter + ? currentDeclaration.setters + : currentDeclaration.members); Builder existing = members[name]; builder.next = existing; if (builder is PrefixBuilder && existing is PrefixBuilder) { assert(existing.next == null); - builder.exports.forEach((String name, Builder builder) { - Builder other = existing.exports.putIfAbsent(name, () => builder); - if (other != builder) { - existing.exports[name] = - buildAmbiguousBuilder(name, other, builder, charOffset); - } - }); - return existing; + return existing + ..exports.merge(builder.exports, + (String name, Builder existing, Builder member) { + return buildAmbiguousBuilder(name, existing, member, charOffset); + }); } else if (isDuplicatedDefinition(existing, builder)) { addCompileTimeError(charOffset, "Duplicated definition of '$name'."); } @@ -313,6 +308,17 @@ abstract class SourceLibraryBuilder buildBuilder(builder); } canAddImplementationBuilders = false; + + scope.setters.forEach((String name, Builder setter) { + Builder member = scopeBuilder[name]; + if (member == null || !member.isField || member.isFinal) return; + // TODO(ahe): charOffset is missing. + addCompileTimeError( + setter.charOffset, "Conflicts with member '${name}'."); + addCompileTimeError( + member.charOffset, "Conflicts with setter '${name}'."); + }); + return null; } @@ -381,14 +387,12 @@ abstract class SourceLibraryBuilder types.addAll(part.types); constructorReferences.addAll(part.constructorReferences); part.partOfLibrary = this; + part.scope.becomePartOf(scope); // TODO(ahe): Include metadata from part? } void buildInitialScopes() { forEach(addToExportScope); - forEach((String name, Builder member) { - addToScope(name, member, member.charOffset, false); - }); } void addImportsToScope() { @@ -408,15 +412,16 @@ abstract class SourceLibraryBuilder @override void addToScope(String name, Builder member, int charOffset, bool isImport) { - Builder existing = scope.lookup(name, member.charOffset, fileUri); + Map map = + member.isSetter ? importScope.setters : importScope.local; + Builder existing = map[name]; if (existing != null) { if (existing != member) { - scope.local[name] = buildAmbiguousBuilder( - name, existing, member, charOffset, + map[name] = buildAmbiguousBuilder(name, existing, member, charOffset, isImport: isImport); } } else { - scope.local[name] = member; + map[name] = member; } } @@ -424,15 +429,17 @@ abstract class SourceLibraryBuilder bool addToExportScope(String name, Builder member) { if (name.startsWith("_")) return false; if (member is PrefixBuilder) return false; - Builder existing = exports[name]; + Map map = + member.isSetter ? exports.setters : exports.local; + Builder existing = map[name]; if (existing == member) return false; if (existing != null) { Builder result = buildAmbiguousBuilder(name, existing, member, -1, isExport: true); - exports[name] = result; + map[name] = result; return result != existing; } else { - exports[name] = member; + map[name] = member; } return true; } @@ -471,25 +478,30 @@ class DeclarationBuilder { final Map members; + final Map constructors; + + final Map setters; + final List types = []; final String name; - final Map> factoryDeclarations = - >{}; + final Map> factoryDeclarations; - DeclarationBuilder(this.members, this.name, [this.parent]); + DeclarationBuilder(this.members, this.setters, this.constructors, + this.factoryDeclarations, this.name, this.parent); - void addMember(String name, MemberBuilder builder) { - if (members == null) { - parent.addMember(name, builder); - } else { - members[name] = builder; - } - } + DeclarationBuilder.library() + : this({}, {}, null, null, null, null); - MemberBuilder lookupMember(String name) { - return members == null ? parent.lookupMember(name) : members[name]; + DeclarationBuilder createNested(String name, bool hasMembers) { + return new DeclarationBuilder( + hasMembers ? {} : null, + hasMembers ? {} : null, + hasMembers ? {} : null, + >{}, + name, + this); } void addType(T type) { @@ -546,4 +558,8 @@ class DeclarationBuilder { ProcedureBuilder procedure, DeclarationBuilder factoryDeclaration) { factoryDeclarations[procedure] = factoryDeclaration; } + + Scope toScope(Scope parent) { + return new Scope(members, setters, parent, isModifiable: false); + } } diff --git a/samples/samples.status b/samples/samples.status index ddcc2015e56..4a9105ebddd 100644 --- a/samples/samples.status +++ b/samples/samples.status @@ -31,6 +31,3 @@ sample_extension/test/*: Skip # These tests attempt to spawn another script usin [ $compiler == none && $runtime == vm && $system == windows && $mode == debug ] sample_extension/test/sample_extension_app_snapshot_test: Pass, RuntimeError # Issue 28842 - -[ ($compiler == dartk || $compiler == dartkp) && ($runtime == vm || $runtime == dart_precompiled) ] -build_dart/test/build_dart_test: DartkCrash diff --git a/tests/co19/co19-kernel.status b/tests/co19/co19-kernel.status index 1eb6d701779..bfd29d7c4cc 100644 --- a/tests/co19/co19-kernel.status +++ b/tests/co19/co19-kernel.status @@ -38,18 +38,13 @@ Language/Classes/Constructors/Generative_Constructors/superclass_constructor_t06 Language/Classes/Constructors/Generative_Constructors/syntax_t02: MissingCompileTimeError Language/Classes/Constructors/Generative_Constructors/syntax_t04: MissingCompileTimeError Language/Classes/Constructors/Generative_Constructors/syntax_t05: MissingCompileTimeError -Language/Classes/Getters/static_getter_t02: CompileTimeError Language/Classes/Getters/syntax_t02: MissingCompileTimeError Language/Classes/Getters/syntax_t03: MissingCompileTimeError Language/Classes/Getters/syntax_t04: MissingCompileTimeError Language/Classes/Getters/syntax_t05: MissingCompileTimeError Language/Classes/Getters/syntax_t07: MissingCompileTimeError Language/Classes/Instance_Methods/Operators/syntax_t04: MissingCompileTimeError -Language/Classes/Instance_Methods/same_name_setter_t01: CompileTimeError Language/Classes/Instance_Variables/constant_t01: MissingCompileTimeError -Language/Classes/Setters/name_t01: CompileTimeError -Language/Classes/Setters/name_t02: CompileTimeError -Language/Classes/Setters/name_t07: CompileTimeError Language/Classes/Setters/parameter_t01: MissingCompileTimeError Language/Classes/Setters/parameter_t02: MissingCompileTimeError Language/Classes/Setters/parameter_t03: MissingCompileTimeError @@ -58,7 +53,6 @@ Language/Classes/Setters/parameter_t06: MissingCompileTimeError Language/Classes/Setters/parameter_t08: MissingCompileTimeError Language/Classes/Setters/syntax_t03: MissingCompileTimeError Language/Classes/Static_Methods/declaration_t01: MissingCompileTimeError -Language/Classes/Static_Methods/same_name_method_and_setter_t01: CompileTimeError Language/Classes/Superclasses/wrong_superclass_t08: MissingCompileTimeError Language/Classes/Superinterfaces/wrong_type_t04: MissingCompileTimeError Language/Classes/Superinterfaces/wrong_type_t05: MissingCompileTimeError @@ -214,7 +208,6 @@ Language/Libraries_and_Scripts/Imports/deferred_import_t02: RuntimeError # Kerne Language/Libraries_and_Scripts/Imports/invalid_uri_deferred_t02: CompileTimeError Language/Libraries_and_Scripts/Imports/invalid_uri_t01: MissingCompileTimeError Language/Libraries_and_Scripts/Imports/invalid_uri_t02: Pass # OK -Language/Libraries_and_Scripts/Imports/namespace_changes_t27: CompileTimeError Language/Libraries_and_Scripts/Imports/same_name_t02/01: MissingRuntimeError Language/Libraries_and_Scripts/Imports/same_name_t05: RuntimeError Language/Libraries_and_Scripts/Imports/same_name_t10: RuntimeError @@ -233,7 +226,6 @@ Language/Libraries_and_Scripts/Scripts/syntax_t08: MissingCompileTimeError Language/Libraries_and_Scripts/Scripts/syntax_t10: MissingCompileTimeError Language/Libraries_and_Scripts/Scripts/syntax_t14: MissingCompileTimeError Language/Libraries_and_Scripts/Scripts/top_level_main_t01: Crash -Language/Libraries_and_Scripts/Scripts/top_level_main_t02: Crash Language/Libraries_and_Scripts/definition_syntax_t01: MissingCompileTimeError Language/Libraries_and_Scripts/definition_syntax_t03: MissingCompileTimeError Language/Libraries_and_Scripts/definition_syntax_t04: MissingCompileTimeError diff --git a/tests/language/language_kernel.status b/tests/language/language_kernel.status index cadd82a994e..c4dfb1bd140 100644 --- a/tests/language/language_kernel.status +++ b/tests/language/language_kernel.status @@ -3,7 +3,6 @@ # BSD-style license that can be found in the LICENSE file. [ $compiler == dartk || $compiler == dartkp ] -bad_constructor_test/05: CompileTimeError bad_initializer1_negative_test: Crash body_less_constructor_wrong_arg_negative_test: Crash built_in_identifier_test/01: CompileTimeError @@ -258,9 +257,6 @@ getter_parameters_test/01: MissingCompileTimeError getter_parameters_test/02: MissingCompileTimeError getter_parameters_test/03: MissingCompileTimeError getter_parameters_test/04: MissingCompileTimeError -getter_setter_in_lib_test: RuntimeError -if_null_assignment_behavior_test/09: RuntimeError -if_null_assignment_behavior_test/10: RuntimeError if_null_assignment_behavior_test/15: RuntimeError implicit_scope_test: Crash import_private_test/01: MissingCompileTimeError @@ -374,7 +370,6 @@ regress_28217_test/none: MissingCompileTimeError regress_28278_test: RuntimeError # Mirrors Issue runtime_type_function_test: RuntimeError scope_variable_test/01: MissingCompileTimeError -setter4_test: CompileTimeError static_field3_test/03: CompileTimeError static_field3_test/04: CompileTimeError static_parameter_test/01: MissingCompileTimeError diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status index 87871c57f63..49666b71d3a 100644 --- a/tests/standalone/standalone.status +++ b/tests/standalone/standalone.status @@ -438,23 +438,27 @@ io/named_pipe_script_test: RuntimeError [ ($compiler == dartk || $compiler == dartkp) && ($runtime == vm || $runtime == dart_precompiled) ] assert_test: RuntimeError io/compile_all_test: Crash -io/directory_chdir_test: DartkCrash -io/directory_test: DartkCrash -io/directory_uri_test: DartkCrash -io/file_absolute_path_test: DartkCrash -io/file_uri_test: DartkCrash +io/directory_chdir_test: RuntimeError +io/directory_test: RuntimeError +io/directory_uri_test: RuntimeError +io/file_absolute_path_test: RuntimeError +io/file_uri_test: RuntimeError io/http_cookie_test: RuntimeError io/http_proxy_advanced_test: RuntimeError -io/link_async_test: DartkCrash -io/link_test: DartkCrash -io/link_uri_test: DartkCrash -io/platform_test: DartkCrash -io/resolve_symbolic_links_test: DartkCrash -io/skipping_dart2js_compilations_test: DartkCrash +io/http_proxy_test: Skip # Flaky. +io/http_reuse_server_port_test: Skip # Flaky. +io/http_server_close_response_after_error_test: Skip # Flaky +io/link_async_test: RuntimeError +io/link_test: RuntimeError +io/link_uri_test: RuntimeError +io/platform_test: RuntimeError +io/raw_datagram_socket_test: Skip # Flaky. +io/resolve_symbolic_links_test: RuntimeError +io/skipping_dart2js_compilations_test: Pass io/socket_many_connections_test: RuntimeError -io/test_harness_analyzer_test: DartkCrash -io/test_runner_test: DartkCrash -io/uri_platform_test: DartkCrash +io/test_harness_analyzer_test: Pass +io/test_runner_test: Pass +io/uri_platform_test: RuntimeError map_insert_remove_oom_test: Crash package/package1_test: CompileTimeError package/package_test: CompileTimeError @@ -463,9 +467,6 @@ package/scenarios/invalid/invalid_utf8_test: DartkCrash package/scenarios/invalid/non_existent_packages_file_test: DartkCrash package/scenarios/invalid/same_package_twice_test: DartkCrash package/scenarios/packages_dir_only/packages_dir_only_test: CompileTimeError -io/http_proxy_test: Skip # Flaky. -io/http_reuse_server_port_test: Skip # Flaky. -io/raw_datagram_socket_test: Skip # Flaky. [ $runtime == flutter ] io/raw_datagram_socket_test: Crash # Flutter Issue 9115