Skip to content

Commit 023bfc0

Browse files
author
John Messerly
committed
remove ignoreTypes flag, it doesn't work, see #134
If we wanted to bring this back, we should implement it differently, IMO Instead simply replace all type annotations with `dynamic` [email protected] Review URL: https://codereview.chromium.org/1163393002
1 parent 20d14a6 commit 023bfc0

File tree

3 files changed

+7
-24
lines changed

3 files changed

+7
-24
lines changed

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ abstract class TypeRules {
7575

7676
DartType elementType(Element e);
7777

78-
bool isDynamic(DartType t);
7978
bool isDynamicTarget(Expression expr);
8079
bool isDynamicCall(Expression call);
8180
}
8281

82+
// TODO(jmesserly): this is unused.
8383
class DartRules extends TypeRules {
8484
DartRules(TypeProvider provider) : super(provider);
8585

@@ -465,16 +465,13 @@ class RestrictedRules extends TypeRules {
465465
return (e as dynamic).type;
466466
}
467467

468-
bool isDynamic(DartType t) => options.ignoreTypes || t.isDynamic;
469-
470468
/// Returns `true` if the target expression is dynamic.
471-
bool isDynamicTarget(Expression target) =>
472-
options.ignoreTypes || utils.isDynamicTarget(target);
469+
// TODO(jmesserly): remove this in favor of utils? Or a static method here?
470+
bool isDynamicTarget(Expression target) => utils.isDynamicTarget(target);
473471

474472
/// Returns `true` if the expression is a dynamic function call or method
475473
/// invocation.
476474
bool isDynamicCall(Expression call) {
477-
if (options.ignoreTypes) return true;
478475
var t = getTypeAsCaller(call);
479476
// TODO(leafp): This will currently return true if t is Function
480477
// This is probably the most correct thing to do for now, since

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1879,10 +1879,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor {
18791879
var leftType = getStaticType(left);
18801880
var rightType = getStaticType(right);
18811881

1882-
// TODO(jmesserly): this may not work correctly with options.ignoreTypes,
1883-
// because that results in unreliable type annotations. See issue #134,
1884-
// probably the checker/resolver is the right place to implement that, by
1885-
// replacing staticTypes with `dynamic` as needed, so codegen "just works".
18861882
var code;
18871883
if (op.type.isEqualityOperator) {
18881884
// If we statically know LHS or RHS is null we can generate a clean check.

pkg/dev_compiler/lib/src/options.dart

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,8 @@ class RulesOptions extends TypeOptions {
8080
/// Whether to inject casts between Dart assignable types.
8181
final bool relaxedCasts;
8282

83-
/// Whether to use static types for code generation.
84-
final bool ignoreTypes;
85-
86-
RulesOptions({this.inferDownwards: inferDownwardsDefault,
87-
this.relaxedCasts: true, this.ignoreTypes: false});
83+
RulesOptions(
84+
{this.inferDownwards: inferDownwardsDefault, this.relaxedCasts: true});
8885
}
8986

9087
class JSCodeOptions {
@@ -188,10 +185,6 @@ class CompilerOptions implements RulesOptions, ResolverOptions, JSCodeOptions {
188185
@override
189186
final List<String> nonnullableTypes;
190187

191-
/// Whether to use static types for code generation.
192-
@override
193-
final bool ignoreTypes;
194-
195188
/// Whether to emit the source map files.
196189
@override
197190
final bool emitSourceMaps;
@@ -205,8 +198,8 @@ class CompilerOptions implements RulesOptions, ResolverOptions, JSCodeOptions {
205198
final Map<String, String> customUrlMappings;
206199

207200
CompilerOptions({this.checkSdk: false, this.dumpInfo: false,
208-
this.dumpInfoFile, this.forceCompile: false, this.ignoreTypes: false,
209-
this.outputDir, this.useColors: true, this.relaxedCasts: true,
201+
this.dumpInfoFile, this.forceCompile: false, this.outputDir,
202+
this.useColors: true, this.relaxedCasts: true,
210203
this.useMultiPackage: false, this.packageRoot: 'packages/',
211204
this.packagePaths: const <String>[], this.resources: const <String>[],
212205
this.inferDownwards: RulesOptions.inferDownwardsDefault,
@@ -272,7 +265,6 @@ CompilerOptions parseOptions(List<String> argv) {
272265
dumpInfo: dumpInfo,
273266
dumpInfoFile: args['dump-info-file'],
274267
forceCompile: args['force-compile'] || serverMode,
275-
ignoreTypes: args['ignore-types'],
276268
outputDir: outputDir,
277269
relaxedCasts: args['relaxed-casts'],
278270
useColors: useColors,
@@ -310,8 +302,6 @@ final ArgParser argParser = new ArgParser()
310302
abbr: 's', help: 'Typecheck sdk libs', defaultsTo: false)
311303
..addFlag('mock-sdk',
312304
abbr: 'm', help: 'Use a mock Dart SDK', defaultsTo: false)
313-
..addFlag('ignore-types',
314-
help: 'Ignore types during codegen', defaultsTo: false)
315305
..addFlag('relaxed-casts',
316306
help: 'Cast between Dart assignable types', defaultsTo: true)
317307
..addOption('nonnullable',

0 commit comments

Comments
 (0)