(dart2js): enable new-rti by default

This change:
* adds the `--use-old-rti` flag to revert to the old behavior
* enables the new behavior by default
* changes the -rti- builders to run the old rti instead of the new rti
* documents the change in CHANGELOG.md

I've kept around the logic as `useNewRti` to avoid swapping all the conditions
in the compiler.

Change-Id: I773ac33b658cb60f72e0b6beef83375abec31bad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127492
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>

Author: sigmundch

CHANGELOG.md

@@ -54,6 +54,58 @@ The Linter was updated to `0.1.106`, which includes:
   dependencies by default. Instead they are precompiled on first `pub run`.
   Use `pub get --precompile` to get the previous behavior.
 
+#### dart2js
+
+A new representation of runtime types was enabled by default.
+
+This change is part of a long term goal of making runtime checks cheaper and
+more flexible for upcoming changes in the language. The new representation
+disentangles how types and classes are represented and makes types first-class
+to the compiler.  This makes it possible to do certain kind of optimizations on
+type checks that were not possible before and will enable us to model
+non-nullable types in the near future.
+
+This change should not affect the semantics of your application, but it has some
+relatively small visible effects that we want to highlight:
+
+* Types are now canonicalized, this fixes a long standing bug that Types could
+  not be used in switch cases (issue [17207][]).
+
+* Code-size changes may be visible, but the difference is small overall. It is
+  more visible on smaller apps because the new implementation includes more
+  helper methods. On large apps we have even seen an overall code-size
+  reduction.
+
+* Certain checks are a lot faster.  This is less noticeable if you are compiling
+  apps with `-O3` where checks are omitted altogether. Even with `-O3`, the
+  performance of some `is` checks used by your app may improve.
+
+* When using `-O3` and `-O4` incorrect type annotations could surface as errors.
+  The old type representation was accidentally lenient on some invalid type
+  annotations. We have only encountered this issue on programs that were not
+  tested properly at the js-interop program boundary.
+
+* `Type.toString` has a small change that is rarely visible. For a long time
+  dart2js has had support to erase unused type variables. Today, when dart2js is
+  given `--lax-runtime-type-to-string` (currently included in `-O2`, `-O3`, and
+  `-O4`) and it decides to erase the type variable of a class `Foo<T>`, then it
+  compiles expressions like `foo.runtimeType.toString()` to print `Foo`. With
+  the new representation, this will show `Foo<erased>` instead. This change may
+  be visible in error messages produced by type checks involving erased types.
+
+Because types and classes are represented separately, we will likely reevaluate
+restrictions of deferred libraries in the near future. For example, we could
+support referring to deferred types because types can be downloaded while
+classes are not.
+
+In the unlikely case you run into any issues, please file a bug so we can
+investigate. You can temporarily force the old type representation by passing
+`--use-old-rti` to dart2js if necessary, but our goal is to delete the old type
+representation soon.
+
+
+[17207]: https://github.com/dart-lang/sdk/issues/17207
+
 ## 2.7.0 - 2019-12-11
 
 **Extension methods** -- which we shipped in preview in 2.6.0 -- are no longer

pkg/compiler/lib/src/commandline_options.dart

@@ -77,6 +77,7 @@ class Flags {
   static const String useContentSecurityPolicy = '--csp';
   static const String useMultiSourceInfo = '--use-multi-source-info';
   static const String useNewSourceInfo = '--use-new-source-info';
+  static const String useOldRti = '--use-old-rti';
   static const String verbose = '--verbose';
   static const String progress = '--show-internal-progress';
   static const String version = '--version';

pkg/compiler/lib/src/common_elements.dart

@@ -1497,10 +1497,9 @@ class CommonElementsImpl
 
   ClassEntity _typeLiteralClass;
   @override
-  ClassEntity get typeLiteralClass =>
-      _typeLiteralClass ??= _options.experimentNewRti
-          ? _findRtiClass('_Type')
-          : _findHelperClass('TypeImpl');
+  ClassEntity get typeLiteralClass => _typeLiteralClass ??= _options.useNewRti
+      ? _findRtiClass('_Type')
+      : _findHelperClass('TypeImpl');
 
   ClassEntity _constMapLiteralClass;
   @override
@@ -1787,7 +1786,7 @@ class CommonElementsImpl
       _findHelperFunction('throwNoSuchMethod');
 
   @override
-  FunctionEntity get createRuntimeType => _options.experimentNewRti
+  FunctionEntity get createRuntimeType => _options.useNewRti
       ? _findRtiFunction('createRuntimeType')
       : _findHelperFunction('createRuntimeType');
 

pkg/compiler/lib/src/dart2js.dart

@@ -468,6 +468,7 @@ Future<api.CompilationResult> compile(List<String> argv,
     new OptionHandler(Flags.generateCodeWithCompileTimeErrors, ignoreOption),
     new OptionHandler(Flags.useMultiSourceInfo, passThrough),
     new OptionHandler(Flags.useNewSourceInfo, passThrough),
+    new OptionHandler(Flags.useOldRti, passThrough),
     new OptionHandler(Flags.testMode, passThrough),
 
     // Experimental features.
@@ -482,7 +483,7 @@ Future<api.CompilationResult> compile(List<String> argv,
     new OptionHandler(Flags.experimentStartupFunctions, passThrough),
     new OptionHandler(Flags.experimentToBoolean, passThrough),
     new OptionHandler(Flags.experimentCallInstrumentation, passThrough),
-    new OptionHandler(Flags.experimentNewRti, passThrough),
+    new OptionHandler(Flags.experimentNewRti, ignoreOption),
 
     // The following three options must come last.
     new OptionHandler('-D.+=.*', addInEnvironment),

pkg/compiler/lib/src/js_backend/backend_impact.dart

@@ -168,7 +168,7 @@ class BackendImpacts {
   BackendImpact get typeVariableBoundCheck {
     return _typeVariableBoundCheck ??= new BackendImpact(staticUses: [
       _commonElements.throwTypeError,
-      if (_options.experimentNewRti)
+      if (_options.useNewRti)
         _commonElements.checkTypeBound
       else
         _commonElements.assertIsSubtype,
@@ -435,7 +435,7 @@ class BackendImpacts {
       _commonElements.typeLiteralClass
     ], staticUses: [
       _commonElements.createRuntimeType,
-      if (_options.experimentNewRti) _commonElements.typeLiteralMaker,
+      if (_options.useNewRti) _commonElements.typeLiteralMaker,
     ]);
   }
 
@@ -785,7 +785,7 @@ class BackendImpacts {
       _genericInstantiation[typeArgumentCount] ??=
           new BackendImpact(staticUses: [
         _commonElements.getInstantiateFunction(typeArgumentCount),
-        ..._options.experimentNewRti
+        ..._options.useNewRti
             ? [
                 _commonElements.instantiatedGenericFunctionTypeNewRti,
                 _commonElements.closureFunctionType
@@ -802,7 +802,7 @@ class BackendImpacts {
   BackendImpact _newRtiImpact;
 
   // TODO(sra): Split into refined impacts.
-  BackendImpact get newRtiImpact => _newRtiImpact ??= _options.experimentNewRti
+  BackendImpact get newRtiImpact => _newRtiImpact ??= _options.useNewRti
       ? BackendImpact(staticUses: [
           _commonElements.findType,
           _commonElements.instanceType,

pkg/compiler/lib/src/js_backend/codegen_listener.dart

@@ -122,7 +122,7 @@ class CodegenEnqueuerListener extends EnqueuerListener {
     }
 
     // TODO(fishythefish): Avoid registering unnecessary impacts.
-    if (_options.experimentNewRti && !_isNewRtiUsed) {
+    if (_options.useNewRti && !_isNewRtiUsed) {
       WorldImpactBuilderImpl newRtiImpact = new WorldImpactBuilderImpl();
       newRtiImpact.registerStaticUse(StaticUse.staticInvoke(
           _commonElements.rtiAddRulesMethod, CallStructure.TWO_ARGS));
@@ -189,7 +189,7 @@ class CodegenEnqueuerListener extends EnqueuerListener {
           _elementEnvironment.getThisType(_commonElements
               .getInstantiationClass(constant.typeArguments.length))));
 
-      if (_options.experimentNewRti) {
+      if (_options.useNewRti) {
         impactBuilder.registerStaticUse(StaticUse.staticInvoke(
             _commonElements.instantiatedGenericFunctionTypeNewRti,
             CallStructure.TWO_ARGS));

pkg/compiler/lib/src/js_backend/constant_emitter.dart

@@ -238,7 +238,7 @@ class ConstantEmitter extends ModularConstantEmitter {
         .toList(growable: false);
     jsAst.ArrayInitializer array = new jsAst.ArrayInitializer(elements);
     jsAst.Expression value = _makeConstantList(array);
-    if (_options.experimentNewRti) {
+    if (_options.useNewRti) {
       return maybeAddListTypeArgumentsNewRti(constant, constant.type, value);
     } else {
       return maybeAddTypeArguments(constant, constant.type, value);
@@ -261,7 +261,7 @@ class ConstantEmitter extends ModularConstantEmitter {
     ];
 
     if (_rtiNeed.classNeedsTypeArguments(classElement)) {
-      if (_options.experimentNewRti) {
+      if (_options.useNewRti) {
         arguments.add(_reifiedTypeNewRti(sourceType));
       } else {
         arguments
@@ -352,7 +352,7 @@ class ConstantEmitter extends ModularConstantEmitter {
     }
 
     if (_rtiNeed.classNeedsTypeArguments(classElement)) {
-      if (_options.experimentNewRti) {
+      if (_options.useNewRti) {
         arguments.add(_reifiedTypeNewRti(constant.type));
       } else {
         arguments
@@ -373,7 +373,7 @@ class ConstantEmitter extends ModularConstantEmitter {
   jsAst.Expression visitType(TypeConstantValue constant, [_]) {
     DartType type = constant.representedType.unaliased;
 
-    if (_options.experimentNewRti) {
+    if (_options.useNewRti) {
       assert(!type.containsTypeVariables);
 
       jsAst.Expression recipe = _rtiRecipeEncoder.encodeGroundRecipe(
@@ -428,7 +428,7 @@ class ConstantEmitter extends ModularConstantEmitter {
       }
     });
     if (_rtiNeed.classNeedsTypeArguments(constant.type.element)) {
-      if (_options.experimentNewRti) {
+      if (_options.useNewRti) {
         fields.add(_reifiedTypeNewRti(constant.type));
       } else {
         fields
@@ -446,7 +446,7 @@ class ConstantEmitter extends ModularConstantEmitter {
     List<jsAst.Expression> fields = <jsAst.Expression>[
       _constantReferenceGenerator(constant.function)
     ];
-    if (_options.experimentNewRti) {
+    if (_options.useNewRti) {
       fields
           .add(_reifiedTypeNewRti(InterfaceType(cls, constant.typeArguments)));
     } else {
@@ -502,7 +502,7 @@ class ConstantEmitter extends ModularConstantEmitter {
   }
 
   jsAst.Expression _reifiedTypeNewRti(DartType type) {
-    assert(_options.experimentNewRti);
+    assert(_options.useNewRti);
     assert(!type.containsTypeVariables);
     return TypeReference(TypeExpressionRecipe(type))..forConstant = true;
   }

pkg/compiler/lib/src/js_backend/resolution_listener.dart

@@ -443,7 +443,7 @@ class ResolutionEnqueuerListener extends EnqueuerListener {
       _registerBackendImpact(impactBuilder, _impacts.traceHelper);
     }
 
-    if (_options.experimentNewRti) {
+    if (_options.useNewRti) {
       _registerBackendImpact(impactBuilder, _impacts.rtiAddRules);
     }
 

pkg/compiler/lib/src/js_emitter/instantiation_stub_generator.dart

@@ -81,7 +81,7 @@ class InstantiationStubGenerator {
       parameters.add(new jsAst.Parameter(jsName));
     }
 
-    if (_options.experimentNewRti) {
+    if (_options.useNewRti) {
       for (int i = 0; i < targetSelector.typeArgumentCount; i++) {
         arguments.add(js('this.#.#[#]', [
           _namer.rtiFieldJsName,
@@ -122,7 +122,7 @@ class InstantiationStubGenerator {
     jsAst.Name operatorSignature =
         _namer.asName(_namer.fixedNames.operatorSignature);
 
-    jsAst.Fun function = _options.experimentNewRti
+    jsAst.Fun function = _options.useNewRti
         ? _generateSignatureNewRti(functionField)
         : _generateSignatureLegacy(functionField);
 

pkg/compiler/lib/src/js_emitter/metadata_collector.dart

@@ -203,7 +203,7 @@ class MetadataCollector implements jsAst.TokenFinalizer {
   jsAst.Expression addTypeInOutputUnit(DartType type, OutputUnit outputUnit) {
     _typesMap[outputUnit] ??= new Map<DartType, _BoundMetadataEntry>();
     return _typesMap[outputUnit].putIfAbsent(type, () {
-      if (_options.experimentNewRti) {
+      if (_options.useNewRti) {
         return new _BoundMetadataEntry(_computeTypeRepresentationNewRti(type));
       } else {
         return new _BoundMetadataEntry(_computeTypeRepresentation(type));