Skip to content

Commit

Permalink
Handle use of fromEnvironment from serialized data.
Browse files Browse the repository at this point in the history
This CL removes the [intEnvironment], [boolEnvironment] and [stringEnvironment]
from [Compiler] and instead determine .fromEnvironment directly from the
constructor element itself.

R=sigmund@google.com

Review URL: https://codereview.chromium.org/2033383002 .
  • Loading branch information
johnniwinther committed Jun 6, 2016
1 parent 0b926d6 commit b11a92f
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 60 deletions.
23 changes: 10 additions & 13 deletions pkg/compiler/lib/src/compile_time_constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -927,10 +927,7 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
assert(normalizedArguments != null);
concreteArguments = normalizedArguments;
}

if (constructor == compiler.intEnvironment ||
constructor == compiler.boolEnvironment ||
constructor == compiler.stringEnvironment) {
if (constructor.isFromEnvironmentConstructor) {
return createFromEnvironmentConstant(node, constructedType, constructor,
callStructure, normalizedArguments, concreteArguments);
} else {
Expand Down Expand Up @@ -974,7 +971,7 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
return null;
}

if (constructor == compiler.intEnvironment &&
if (constructor.isIntFromEnvironmentConstructor &&
!(defaultValue.isNull || defaultValue.isInt)) {
DartType type = defaultValue.getType(coreTypes);
reporter.reportErrorMessage(
Expand All @@ -984,7 +981,7 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
return null;
}

if (constructor == compiler.boolEnvironment &&
if (constructor.isBoolFromEnvironmentConstructor &&
!(defaultValue.isNull || defaultValue.isBool)) {
DartType type = defaultValue.getType(coreTypes);
reporter.reportErrorMessage(
Expand All @@ -994,7 +991,7 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
return null;
}

if (constructor == compiler.stringEnvironment &&
if (constructor.isStringFromEnvironmentConstructor &&
!(defaultValue.isNull || defaultValue.isString)) {
DartType type = defaultValue.getType(coreTypes);
reporter.reportErrorMessage(
Expand All @@ -1014,13 +1011,13 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
if (concreteArguments.length > 1) {
defaultValue = concreteArguments[1].expression;
}
if (constructor == compiler.intEnvironment) {
if (constructor.isIntFromEnvironmentConstructor) {
expression =
new IntFromEnvironmentConstantExpression(name, defaultValue);
} else if (constructor == compiler.boolEnvironment) {
} else if (constructor.isBoolFromEnvironmentConstructor) {
expression =
new BoolFromEnvironmentConstantExpression(name, defaultValue);
} else if (constructor == compiler.stringEnvironment) {
} else if (constructor.isStringFromEnvironmentConstructor) {
expression =
new StringFromEnvironmentConstantExpression(name, defaultValue);
}
Expand All @@ -1030,11 +1027,11 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {

if (value == null) {
return createEvaluatedConstant(defaultValue);
} else if (constructor == compiler.intEnvironment) {
} else if (constructor.isIntFromEnvironmentConstructor) {
int number = int.parse(value, onError: (_) => null);
return createEvaluatedConstant(
(number == null) ? defaultValue : constantSystem.createInt(number));
} else if (constructor == compiler.boolEnvironment) {
} else if (constructor.isBoolFromEnvironmentConstructor) {
if (value == 'true') {
return createEvaluatedConstant(constantSystem.createBool(true));
} else if (value == 'false') {
Expand All @@ -1043,7 +1040,7 @@ class CompileTimeConstantEvaluator extends Visitor<AstConstant> {
return createEvaluatedConstant(defaultValue);
}
} else {
assert(constructor == compiler.stringEnvironment);
assert(constructor.isStringFromEnvironmentConstructor);
return createEvaluatedConstant(
constantSystem.createString(new DartString.literal(value)));
}
Expand Down
15 changes: 0 additions & 15 deletions pkg/compiler/lib/src/compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,6 @@ abstract class Compiler implements LibraryLoaderListener {
Element loadLibraryFunction;
Element functionApplyMethod;

/// The [int.fromEnvironment] constructor.
ConstructorElement intEnvironment;

/// The [bool.fromEnvironment] constructor.
ConstructorElement boolEnvironment;

/// The [String.fromEnvironment] constructor.
ConstructorElement stringEnvironment;

// TODO(zarah): Remove this map and incorporate compile-time errors
// in the model.
/// Tracks elements with compile-time errors.
Expand Down Expand Up @@ -623,12 +614,6 @@ abstract class Compiler implements LibraryLoaderListener {
mirrorSystemGetNameFunction = cls.lookupLocalMember('getName');
} else if (mirrorsUsedClass == cls) {
mirrorsUsedConstructor = cls.constructors.head;
} else if (coreClasses.intClass == cls) {
intEnvironment = cls.lookupConstructor(Identifiers.fromEnvironment);
} else if (coreClasses.stringClass == cls) {
stringEnvironment = cls.lookupConstructor(Identifiers.fromEnvironment);
} else if (coreClasses.boolClass == cls) {
boolEnvironment = cls.lookupConstructor(Identifiers.fromEnvironment);
}
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/compiler/lib/src/elements/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
library elements.common;

import '../common/names.dart' show Names, Uris;
import '../common/names.dart' show Identifiers, Names, Uris;
import '../core_types.dart' show CoreClasses;
import '../dart_types.dart' show DartType, InterfaceType, FunctionType;
import '../util/util.dart' show Link;
Expand Down Expand Up @@ -607,3 +607,35 @@ abstract class AbstractFieldElementCommon implements AbstractFieldElement {
setter != null && setter.isAbstract;
}
}

abstract class ConstructorElementCommon implements ConstructorElement {
@override
bool get isFromEnvironmentConstructor {
return name == Identifiers.fromEnvironment &&
library.isDartCore &&
(enclosingClass.name == 'bool' ||
enclosingClass.name == 'int' ||
enclosingClass.name == 'String');
}

@override
bool get isIntFromEnvironmentConstructor {
return name == Identifiers.fromEnvironment &&
library.isDartCore &&
enclosingClass.name == 'int';
}

@override
bool get isBoolFromEnvironmentConstructor {
return name == Identifiers.fromEnvironment &&
library.isDartCore &&
enclosingClass.name == 'bool';
}

@override
bool get isStringFromEnvironmentConstructor {
return name == Identifiers.fromEnvironment &&
library.isDartCore &&
enclosingClass.name == 'String';
}
}
9 changes: 9 additions & 0 deletions pkg/compiler/lib/src/elements/elements.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,15 @@ abstract class ConstructorElement extends FunctionElement
/// `int.fromEnvironment`, or `String.fromEnvironment`.
bool get isFromEnvironmentConstructor;

/// `true` if this constructor is `int.fromEnvironment`.
bool get isIntFromEnvironmentConstructor;

/// `true` if this constructor is `bool.fromEnvironment`.
bool get isBoolFromEnvironmentConstructor;

/// `true` if this constructor is `String.fromEnvironment`.
bool get isStringFromEnvironmentConstructor;

/// Use [enclosingClass] instead.
@deprecated
get enclosingElement;
Expand Down
17 changes: 4 additions & 13 deletions pkg/compiler/lib/src/elements/modelx.dart
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ abstract class ElementX extends Element with ElementCommon {
}
}

class ErroneousElementX extends ElementX implements ErroneousElement {
class ErroneousElementX extends ElementX
with ConstructorElementCommon
implements ErroneousElement {
final MessageKind messageKind;
final Map messageArguments;

Expand Down Expand Up @@ -283,9 +285,6 @@ class ErroneousElementX extends ElementX implements ErroneousElement {
throw new UnsupportedError("isEffectiveTargetMalformed");
}

@override
bool get isFromEnvironmentConstructor => false;

@override
List<DartType> get typeVariables => unsupported();
}
Expand Down Expand Up @@ -2167,21 +2166,13 @@ abstract class ConstantConstructorMixin implements ConstructorElement {
}
}

bool get isFromEnvironmentConstructor {
return name == 'fromEnvironment' &&
library.isDartCore &&
(enclosingClass.name == 'bool' ||
enclosingClass.name == 'int' ||
enclosingClass.name == 'String');
}

/// Returns the empty list of type variables by default.
@override
List<DartType> get typeVariables => functionSignature.typeVariables;
}

abstract class ConstructorElementX extends FunctionElementX
with ConstantConstructorMixin
with ConstantConstructorMixin, ConstructorElementCommon
implements ConstructorElement {
bool isRedirectingGenerative = false;

Expand Down
21 changes: 12 additions & 9 deletions pkg/compiler/lib/src/inferrer/type_graph_nodes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -494,15 +494,18 @@ class MemberTypeInformation extends ElementTypeInformation
}

Compiler compiler = inferrer.compiler;
if (element.declaration == compiler.intEnvironment) {
giveUp(inferrer);
return compiler.typesTask.intType.nullable();
} else if (element.declaration == compiler.boolEnvironment) {
giveUp(inferrer);
return compiler.typesTask.boolType.nullable();
} else if (element.declaration == compiler.stringEnvironment) {
giveUp(inferrer);
return compiler.typesTask.stringType.nullable();
if (element.isConstructor) {
ConstructorElement constructor = element;
if (constructor.isIntFromEnvironmentConstructor) {
giveUp(inferrer);
return compiler.typesTask.intType.nullable();
} else if (constructor.isBoolFromEnvironmentConstructor) {
giveUp(inferrer);
return compiler.typesTask.boolType.nullable();
} else if (constructor.isStringFromEnvironmentConstructor) {
giveUp(inferrer);
return compiler.typesTask.stringType.nullable();
}
}
return null;
}
Expand Down
20 changes: 11 additions & 9 deletions pkg/compiler/lib/src/serialization/modelz.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,8 @@ abstract class ConstructorElementZ extends DeserializedElementZ
FunctionTypedElementMixin,
ParametersMixin,
TypedElementMixin,
MemberElementMixin
MemberElementMixin,
ConstructorElementCommon
implements
ConstructorElement,
// TODO(johnniwinther): Sort out whether a constructor is a method.
Expand All @@ -1177,14 +1178,6 @@ abstract class ConstructorElementZ extends DeserializedElementZ
@override
bool get isExternal => _decoder.getBool(Key.IS_EXTERNAL);

bool get isFromEnvironmentConstructor {
return name == 'fromEnvironment' &&
library.isDartCore &&
(enclosingClass.name == 'bool' ||
enclosingClass.name == 'int' ||
enclosingClass.name == 'String');
}

ConstantConstructor get constantConstructor {
if (isConst && _constantConstructor == null) {
ObjectDecoder data =
Expand Down Expand Up @@ -1412,6 +1405,15 @@ class ForwardingConstructorElementZ extends ElementZ
@override
bool get isFromEnvironmentConstructor => false;

@override
bool get isIntFromEnvironmentConstructor => false;

@override
bool get isBoolFromEnvironmentConstructor => false;

@override
bool get isStringFromEnvironmentConstructor => false;

@override
bool get isRedirectingFactory => false;

Expand Down
6 changes: 6 additions & 0 deletions tests/compiler/dart2js/serialization/test_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@ main() {
''',
'a.dart': '''
void foo() {}
''',
}),

const Test('fromEnvironment constants', const {
'main.dart': '''
main() => const String.fromEnvironment("foo");
''',
}),
];
Expand Down

0 comments on commit b11a92f

Please sign in to comment.