Skip to content

Commit 260cfc0

Browse files
vsmenonvsmenon
committed
Make nonnullability configurable
John: I added code in the JS codegen to query which primitives are non-nullable. [email protected], [email protected] Review URL: https://chromereviews.googleplex.com/155407013
1 parent eb3c51e commit 260cfc0

File tree

14 files changed

+491
-428
lines changed

14 files changed

+491
-428
lines changed

‎pkg/dev_compiler/lib/config.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/// Configuration of DDC rule set.
2+
library ddc.config;
3+
4+
class Configuration {
5+
// TODO(vsm): Configure via compiler flag?
6+
static const nonnullableTypes = const <String>['int', 'double'];
7+
}

‎pkg/dev_compiler/lib/runtime/dart_runtime.dart

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ library ddc.runtime.dart_runtime;
22

33
import 'dart:mirrors';
44

5+
import 'package:ddc/config.dart';
6+
57
dynamic dload(dynamic obj, String field) {
68
var symbol = new Symbol(field);
79
var mirror = reflect(obj);
@@ -74,10 +76,22 @@ bool isGroundType(Type type) {
7476
return true;
7577
}
7678

79+
final _primitiveMap = {
80+
'int': int,
81+
'double': double,
82+
'num': num,
83+
'bool': bool,
84+
'String': String,
85+
};
86+
87+
Set<Type> _primitives = () {
88+
var types = Configuration.nonnullableTypes;
89+
var set = new Set<Type>.from(types.map((t) => _primitiveMap[t]));
90+
return set;
91+
}();
92+
7793
bool isPrimitiveType(Type t) {
78-
// TODO(vsm): Expand to bool or other types?
79-
if (t == int || t == double) return true;
80-
return false;
94+
return _primitives.contains(t);
8195
}
8296

8397
class Arity {

‎pkg/dev_compiler/lib/src/checker/checker.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ class CodeChecker extends RecursiveAstVisitor {
542542
var initializer = variable.initializer;
543543
if (initializer != null) {
544544
variable.initializer = checkAssignment(initializer, dartType);
545-
} else if (_rules.maybePrimitiveType(dartType)) {
545+
} else if (_rules.maybeNonNullableType(dartType)) {
546546
var element = variable.element;
547547
if (element is FieldElement && !element.isStatic) {
548548
// Initialized - possibly implicitly - during construction.

‎pkg/dev_compiler/lib/src/checker/rules.dart

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:ddc_analyzer/src/generated/ast.dart';
44
import 'package:ddc_analyzer/src/generated/element.dart';
55
import 'package:ddc_analyzer/src/generated/resolver.dart';
66

7+
import 'package:ddc/config.dart';
78
import 'package:ddc/src/info.dart';
89
import 'package:ddc/src/options.dart';
910
import 'package:ddc/src/report.dart' show CheckerReporter;
@@ -28,8 +29,8 @@ abstract class TypeRules {
2829
bool isIntType(DartType t) => t == provider.intType;
2930
bool isNumType(DartType t) => t == provider.intType.superclass;
3031
bool isStringType(DartType t) => t == provider.stringType;
31-
bool isPrimitiveType(DartType t) => false;
32-
bool maybePrimitiveType(DartType t) => false;
32+
bool isNonNullableType(DartType t) => false;
33+
bool maybeNonNullableType(DartType t) => false;
3334

3435
StaticInfo checkAssignment(Expression expr, DartType t);
3536

@@ -85,11 +86,31 @@ class DartRules extends TypeRules {
8586
class RestrictedRules extends TypeRules {
8687
final CheckerReporter _reporter;
8788
final RulesOptions options;
88-
final List<DartType> _primitives;
89+
final List<DartType> _nonnullableTypes;
90+
91+
DartType _typeFromName(String name) {
92+
switch (name) {
93+
case 'int':
94+
return provider.intType;
95+
case 'double':
96+
return provider.doubleType;
97+
case 'num':
98+
return provider.numType;
99+
case 'bool':
100+
return provider.boolType;
101+
case 'String':
102+
return provider.stringType;
103+
default:
104+
throw new UnsupportedError('Unsupported non-nullable type $name');
105+
}
106+
}
89107

90108
RestrictedRules(TypeProvider provider, this._reporter, {this.options})
91-
: _primitives = [provider.intType, provider.doubleType],
92-
super(provider);
109+
: _nonnullableTypes = <DartType>[],
110+
super(provider) {
111+
var types = Configuration.nonnullableTypes;
112+
_nonnullableTypes.addAll(types.map(_typeFromName));
113+
}
93114

94115
DartType getStaticType(Expression expr) {
95116
var type = expr.staticType;
@@ -98,20 +119,20 @@ class RestrictedRules extends TypeRules {
98119
return provider.dynamicType;
99120
}
100121

101-
bool isPrimitiveType(DartType t) => _primitives.contains(t);
122+
bool isNonNullableType(DartType t) => _nonnullableTypes.contains(t);
102123

103-
bool maybePrimitiveType(DartType t) {
124+
bool maybeNonNullableType(DartType t) {
104125
// Return true iff t *may* be a primitive type.
105126
// If t is a generic type parameter, return true if it may be
106127
// instantiated as a primitive.
107-
if (isPrimitiveType(t)) {
128+
if (isNonNullableType(t)) {
108129
return true;
109130
} else if (t is TypeParameterType) {
110131
var bound = t.element.bound;
111132
if (bound == null) {
112133
return true;
113134
}
114-
return _primitives.any((DartType p) => isSubTypeOf(p, bound));
135+
return _nonnullableTypes.any((DartType p) => isSubTypeOf(p, bound));
115136
} else {
116137
return false;
117138
}
@@ -251,7 +272,7 @@ class RestrictedRules extends TypeRules {
251272
// FIXME: Can this be anything besides null?
252273
if (t1.isBottom) {
253274
// Return false iff t2 *may* be a primitive type.
254-
return !maybePrimitiveType(t2);
275+
return !maybeNonNullableType(t2);
255276
}
256277
if (t2.isBottom) return false;
257278

‎pkg/dev_compiler/lib/src/codegen/js_codegen.dart

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,16 +104,23 @@ var $_libraryName;
104104
var from = node.baseType;
105105
var to = node.convertedType;
106106

107-
// num to int or num to double is just a null check.
107+
// All Dart number types map to a JS double.
108108
if (rules.isNumType(from) &&
109109
(rules.isIntType(to) || rules.isDoubleType(to))) {
110110
// TODO(jmesserly): a lot of these checks are meaningless, as people use
111111
// `num` to mean "any kind of number" rather than "could be null".
112112
// The core libraries especially suffer from this problem, with many of
113113
// the `num` methods returning `num`.
114-
out.write('dart.notNull(');
115-
node.expression.accept(this);
116-
out.write(')');
114+
if (!rules.isNonNullableType(from) && rules.isNonNullableType(to)) {
115+
// Converting from a nullable number to a non-nullable number
116+
// only requires a null check.
117+
out.write('dart.notNull(');
118+
node.expression.accept(this);
119+
out.write(')');
120+
} else {
121+
// A no-op in JavaScript.
122+
node.expression.accept(this);
123+
}
117124
return;
118125
}
119126

@@ -517,7 +524,7 @@ $name.prototype[Symbol.iterator] = function() {
517524
expression.accept(this);
518525
} else {
519526
var type = rules.elementType(field.element);
520-
if (rules.maybePrimitiveType(type)) {
527+
if (rules.maybeNonNullableType(type)) {
521528
out.write('dart.as(null, ');
522529
_writeTypeName(type);
523530
out.write(')');
@@ -1052,11 +1059,25 @@ $name.prototype[Symbol.iterator] = function() {
10521059
rules.isBoolType(t) ||
10531060
rules.isNumType(t);
10541061

1062+
bool typeIsNonNullablePrimitiveInJS(DartType t) =>
1063+
typeIsPrimitiveInJS(t) && rules.isNonNullableType(t);
1064+
10551065
bool binaryOperationIsPrimitive(DartType leftT, DartType rightT) =>
10561066
typeIsPrimitiveInJS(leftT) && typeIsPrimitiveInJS(rightT);
10571067

10581068
bool unaryOperationIsPrimitive(DartType t) => typeIsPrimitiveInJS(t);
10591069

1070+
void notNull(Expression expr) {
1071+
var type = rules.getStaticType(expr);
1072+
if (rules.isNonNullableType(type)) {
1073+
expr.accept(this);
1074+
} else {
1075+
out.write('dart.notNull(');
1076+
expr.accept(this);
1077+
out.write(')');
1078+
}
1079+
}
1080+
10601081
@override
10611082
void visitBinaryExpression(BinaryExpression node) {
10621083
var op = node.operator;
@@ -1094,15 +1115,15 @@ $name.prototype[Symbol.iterator] = function() {
10941115
if (op.type == TokenType.TILDE_SLASH) {
10951116
// `a ~/ b` is equivalent to `(a / b).truncate()`
10961117
out.write('(');
1097-
lhs.accept(this);
1118+
notNull(lhs);
10981119
out.write(' / ');
1099-
rhs.accept(this);
1120+
notNull(rhs);
11001121
out.write(').truncate()');
11011122
} else {
11021123
// TODO(vsm): When do Dart ops not map to JS?
1103-
lhs.accept(this);
1124+
notNull(lhs);
11041125
out.write(' $op ');
1105-
rhs.accept(this);
1126+
notNull(rhs);
11061127
}
11071128
} else if (rules.isDynamicTarget(lhs)) {
11081129
// dynamic dispatch
@@ -1152,7 +1173,7 @@ $name.prototype[Symbol.iterator] = function() {
11521173
var dispatchType = rules.getStaticType(expr);
11531174
if (unaryOperationIsPrimitive(dispatchType)) {
11541175
// TODO(vsm): When do Dart ops not map to JS?
1155-
expr.accept(this);
1176+
notNull(expr);
11561177
out.write('$op');
11571178
} else {
11581179
// TODO(vsm): Figure out operator calling convention / dispatch.
@@ -1169,7 +1190,7 @@ $name.prototype[Symbol.iterator] = function() {
11691190
if (unaryOperationIsPrimitive(dispatchType)) {
11701191
// TODO(vsm): When do Dart ops not map to JS?
11711192
out.write('$op');
1172-
expr.accept(this);
1193+
notNull(expr);
11731194
} else {
11741195
// TODO(vsm): Figure out operator calling convention / dispatch.
11751196
out.write('/* Unimplemented postfix operator: $node */');

‎pkg/dev_compiler/test/codegen/expect/DeltaBlue/DeltaBlue.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ var DeltaBlue;
5353
}
5454
satisfy(mark) {
5555
this.chooseMethod(dart.as(mark, core.int));
56-
if (!this.isSatisfied()) {
56+
if (!dart.notNull(this.isSatisfied())) {
5757
if (dart.equals(this.strength, REQUIRED)) {
5858
core.print("Could not satisfy a required constraint!");
5959
}
@@ -64,7 +64,7 @@ var DeltaBlue;
6464
let overridden = out.determinedBy;
6565
if (overridden !== null) overridden.markUnsatisfied();
6666
out.determinedBy = this;
67-
if (!DeltaBlue.planner.addPropagate(this, dart.as(mark, core.int))) core.print("Cycle encountered");
67+
if (!dart.notNull(DeltaBlue.planner.addPropagate(this, dart.as(mark, core.int)))) core.print("Cycle encountered");
6868
out.mark = dart.as(mark, core.int);
6969
return overridden;
7070
}
@@ -87,15 +87,15 @@ var DeltaBlue;
8787
this.satisfied = false;
8888
}
8989
chooseMethod(mark) {
90-
this.satisfied = (this.myOutput.mark !== mark) && Strength.stronger(this.strength, this.myOutput.walkStrength);
90+
this.satisfied = dart.notNull((this.myOutput.mark !== mark)) && dart.notNull(Strength.stronger(this.strength, this.myOutput.walkStrength));
9191
}
9292
isSatisfied() { return this.satisfied; }
9393
markInputs(mark) {
9494
}
9595
output() { return this.myOutput; }
9696
recalculate() {
9797
this.myOutput.walkStrength = this.strength;
98-
this.myOutput.stay = !this.isInput();
98+
this.myOutput.stay = !dart.notNull(this.isInput());
9999
if (this.myOutput.stay) this.execute();
100100
}
101101
markUnsatisfied() {
@@ -138,10 +138,10 @@ var DeltaBlue;
138138
}
139139
chooseMethod(mark) {
140140
if (this.v1.mark === mark) {
141-
this.direction = (this.v2.mark !== mark && Strength.stronger(this.strength, this.v2.walkStrength)) ? FORWARD : NONE;
141+
this.direction = (dart.notNull(this.v2.mark !== mark) && dart.notNull(Strength.stronger(this.strength, this.v2.walkStrength))) ? FORWARD : NONE;
142142
}
143143
if (this.v2.mark === mark) {
144-
this.direction = (this.v1.mark !== mark && Strength.stronger(this.strength, this.v1.walkStrength)) ? BACKWARD : NONE;
144+
this.direction = (dart.notNull(this.v1.mark !== mark) && dart.notNull(Strength.stronger(this.strength, this.v1.walkStrength))) ? BACKWARD : NONE;
145145
}
146146
if (Strength.weaker(this.v1.walkStrength, this.v2.walkStrength)) {
147147
this.direction = Strength.stronger(this.strength, this.v1.walkStrength) ? BACKWARD : NONE;
@@ -171,7 +171,7 @@ var DeltaBlue;
171171
}
172172
inputsKnown(mark) {
173173
let i = this.input();
174-
return i.mark === mark || i.stay || i.determinedBy === null;
174+
return dart.notNull(dart.notNull(i.mark === mark) || dart.notNull(i.stay)) || dart.notNull(i.determinedBy === null);
175175
}
176176
removeFromGraph() {
177177
if (this.v1 !== null) this.v1.removeConstraint(this);
@@ -210,7 +210,7 @@ var DeltaBlue;
210210
recalculate() {
211211
let ihn = this.input(), out = this.output();
212212
out.walkStrength = Strength.weakest(this.strength, ihn.walkStrength);
213-
out.stay = ihn.stay && this.scale.stay && this.offset.stay;
213+
out.stay = dart.notNull(dart.notNull(ihn.stay) && dart.notNull(this.scale.stay)) && dart.notNull(this.offset.stay);
214214
if (out.stay) this.execute();
215215
}
216216
}
@@ -274,7 +274,7 @@ var DeltaBlue;
274274
let todo = sources;
275275
while (todo.length > 0) {
276276
let c = todo.removeLast();
277-
if (c.output().mark !== mark && c.inputsKnown(mark)) {
277+
if (dart.notNull(c.output().mark !== mark) && dart.notNull(c.inputsKnown(mark))) {
278278
plan.addConstraint(c);
279279
c.output().mark = mark;
280280
this.addConstraintsConsumingTo(c.output(), todo);
@@ -286,7 +286,7 @@ var DeltaBlue;
286286
let sources = new List.from([]);
287287
for (let i = 0; i < constraints.length; i++) {
288288
let c = constraints.get(i);
289-
if (c.isInput() && c.isSatisfied()) sources.add(c);
289+
if (dart.notNull(c.isInput()) && dart.notNull(c.isSatisfied())) sources.add(c);
290290
}
291291
return this.makePlan(sources);
292292
}
@@ -313,12 +313,12 @@ var DeltaBlue;
313313
let v = todo.removeLast();
314314
for (let i = 0; i < v.constraints.length; i++) {
315315
let c = v.constraints.get(i);
316-
if (!c.isSatisfied()) unsatisfied.add(c);
316+
if (!dart.notNull(c.isSatisfied())) unsatisfied.add(c);
317317
}
318318
let determining = v.determinedBy;
319319
for (let i = 0; i < v.constraints.length; i++) {
320320
let next = v.constraints.get(i);
321-
if (!dart.equals(next, determining) && next.isSatisfied()) {
321+
if (dart.notNull(!dart.equals(next, determining)) && dart.notNull(next.isSatisfied())) {
322322
next.recalculate();
323323
todo.add(next.output());
324324
}
@@ -330,7 +330,7 @@ var DeltaBlue;
330330
let determining = v.determinedBy;
331331
for (let i = 0; i < v.constraints.length; i++) {
332332
let c = v.constraints.get(i);
333-
if (!dart.equals(c, determining) && c.isSatisfied()) coll.add(c);
333+
if (dart.notNull(!dart.equals(c, determining)) && dart.notNull(c.isSatisfied())) coll.add(c);
334334
}
335335
}
336336
}

0 commit comments

Comments
 (0)