commit d41e41990401af0019b930776b78f3c8f281b628
author Nicholas Shahan <nshahan@google.com> Sat Feb 22 01:20:21 2020 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Sat Feb 22 01:20:21 2020 +0000
tree a303b0aef7212f364b416cece734ed0814071ec6
parent a578c646c3386b5d6aff0bd4b8b99e8d74e1050a

[dartdevc] Fix nullability of function types and typedefs

These fixes also prepare for an upcoming change to normalize FutureOr
types.

- Cache and emit the hoisted function type at the top level without
  nullability wrappers.
- Always emit nullability wrappers at the use site.
- Fix issue where sometimes the return type would be double wrapped.

Change-Id: If4a0c94e62eca626f271cc3c259c813911075d9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136404
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>

pkg/dev_compiler/lib/src/kernel/compiler.dart

diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index fbd18ba..41ef67c 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -1375,7 +1375,8 @@
     FunctionType result;
     if (!f.positionalParameters.any(isCovariantParameter) &&
         !f.namedParameters.any(isCovariantParameter)) {
-      result = f.computeThisFunctionType(member.enclosingLibrary.nonNullable);
+      // Avoid tagging a member as Function? or Function*
+      result = f.computeThisFunctionType(Nullability.nonNullable);
     } else {
       DartType reifyParameter(VariableDeclaration p) => isCovariantParameter(p)
           ? _coreTypes.objectRawType(member.enclosingLibrary.nullable)
@@ -1386,7 +1387,7 @@
       // TODO(jmesserly): do covariant type parameter bounds also need to be
       // reified as `Object`?
       result = FunctionType(f.positionalParameters.map(reifyParameter).toList(),
-          f.returnType, Nullability.legacy,
+          f.returnType, Nullability.nonNullable,
           namedParameters: f.namedParameters.map(reifyNamedParameter).toList()
             ..sort(),
           typeParameters: f
@@ -2543,7 +2544,10 @@
   js_ast.Expression _emitFunctionTagged(js_ast.Expression fn, FunctionType type,
       {bool topLevel = false}) {
     var lazy = topLevel && !_canEmitTypeAtTopLevel(type);
-    var typeRep = visitFunctionType(type, lazy: lazy);
+    var typeRep = visitFunctionType(
+        // Avoid tagging a closure as Function? or Function*
+        type.withNullability(Nullability.nonNullable),
+        lazy: lazy);
     return runtimeCall(lazy ? 'lazyFn(#, #)' : 'fn(#, #)', [fn, typeRep]);
   }
 
@@ -2743,14 +2747,12 @@
         : namedTypes.add(param));
     var allNamedTypes = type.namedParameters;
 
-    var returnType = _emitNullabilityWrapper(
-        _emitType(type.returnType), type.returnType.nullability);
+    var returnType = _emitType(type.returnType);
     var requiredArgs = _emitTypeNames(requiredTypes, requiredParams, member);
 
     List<js_ast.Expression> typeParts;
     if (allNamedTypes.isNotEmpty) {
       assert(optionalTypes.isEmpty);
-      // TODO(vsm): The old pageloader may require annotations here.
       var namedArgs = _emitTypeProperties(namedTypes);
       var requiredNamedArgs = _emitTypeProperties(requiredNamedTypes);
       typeParts = [returnType, requiredArgs, namedArgs, requiredNamedArgs];
@@ -2816,9 +2818,13 @@
       helperCall = 'fnType(#)';
     }
     var typeRep = runtimeCall(helperCall, [typeParts]);
-    return _cacheTypes
+    // Avoid caching the nullability of the function type itself so it can be
+    // shared by nullable, non-nullable, and legacy versions at the use site.
+    typeRep = _cacheTypes
         ? _typeTable.nameFunctionType(type, typeRep, lazy: lazy)
         : typeRep;
+
+    return _emitNullabilityWrapper(typeRep, type.nullability);
   }
 
   js_ast.Expression _emitAnnotatedFunctionType(

sdk_nnbd/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart

diff --git a/sdk_nnbd/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart b/sdk_nnbd/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
index a69f6fe..ed0e58a 100644
--- a/sdk_nnbd/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
+++ b/sdk_nnbd/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
@@ -757,7 +757,6 @@
 
   @JSExportName('as')
   as_T(obj, [@notNull bool isImplicit = false]) {
-    if (obj == null) return obj;
     if (JS('!', 'typeof # == "function"', obj)) {
       var actual = JS('', '#[#]', obj, _runtimeType);
       // If there's no actual type, it's a JS function.
@@ -981,13 +980,13 @@
 
   @JSExportName('as')
   as_T(obj) {
-    if (obj == null || is_T(obj)) return obj;
+    if (is_T(obj)) return obj;
     return castError(obj, this, false);
   }
 
   @JSExportName('_check')
   check_T(obj) {
-    if (obj == null || is_T(obj)) return obj;
+    if (is_T(obj)) return obj;
     return castError(obj, this, true);
   }
 }