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

Unified Diff: pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart

Issue 2788153002: Create separate scopes for constructors, setters, and other members. (Closed)
Patch Set: One more flaky standalone/io test. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart
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 d7598fd056935e29db2a8ea7d4acfe8a774c18ce..45b17e597b90736e39df436e4336873dd2b672b3 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<String, MemberBuilder> members = currentDeclaration.members;
+ Map<String, MemberBuilder> constructors = currentDeclaration.constructors;
+ Map<String, MemberBuilder> 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<ConstructorReferenceBuilder>.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<MetadataBuilder> metadata,
int modifiers,
- ConstructorReferenceBuilder constructorName,
+ ConstructorReferenceBuilder constructorNameReference,
List<FormalParameterBuilder> formals,
AsyncMarker asyncModifier,
ConstructorReferenceBuilder redirectionTarget,
@@ -225,11 +261,13 @@ class KernelLibraryBuilder
// Nested declaration began in `OutlineBuilder.beginFactoryMethod`.
DeclarationBuilder<KernelTypeBuilder> 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;
-}

Powered by Google App Engine
This is Rietveld 408576698