Start implementing logic for determining when formal parameters need type checks.

pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart

Index: pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
diff --git a/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart b/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
index 0ac33b1cfa5778b4dc989b7325e7f8619bb01868..245e2c0d06bbb92aa87b03ce868ac57f411cda02 100644
--- a/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
+++ b/pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart
@@ -4,13 +4,14 @@
 
 import 'package:front_end/src/base/instrumentation.dart';
 import 'package:front_end/src/dependency_walker.dart' as dependencyWalker;
-import 'package:front_end/src/fasta/problems.dart' show unhandled;
 import 'package:front_end/src/fasta/kernel/kernel_shadow_ast.dart';

[ahe, 2017/08/23 09:55:58]

Please don't use package import for importing from the same package.

Doing so makes it harder to visually inspect the URI for misusing references to
/src/ of another package.

In addition, it makes it really hard to compile files inside the package, see
https://github.com/dart-lang/sdk/issues/30454

[Paul Berry, 2017/08/23 17:09:10]

We talked about this before and as I recall we decided that self-package imports
were preferable.  See https://codereview.chromium.org/2726793003/#msg3

[ahe, 2017/08/24 09:23:48]

No, we didn't decide that. You argued for using package imports, and I moved all
main methods out of lib/.

In fact, I've been very consistent about not using package imports to front_end
in the fasta directory. And that's a popular style:

ls pkg/*/lib/*.dart third_party/pkg/*/lib/*.dart | xargs grep -E "^import " |
grep package:

In addition, the style guide doesn't support you your statement that "... we
decided to do instead was to tell people to always use a "package:" URI when
referring to a file in "lib/" ..."

This is from https://www.dartlang.org/tools/pub/package-layout:

Tip for web apps: For the best performance when developing with dartdevc, put
implementation files under /lib/src, instead of elsewhere under /lib. Also,
avoid imports of package:package_name/src/....

And:

When you use libraries from within your own package, even code in src, you can
(and should) still use package: to import them. 

I don't think the consensus exists. I think the analysis_server is the cause of
the confusion, and frankly it should be fixed to avoid exacerbating this
problem: it is really hard to read code reviews when you have to stop and
analyze if an import of /src/ is valid.

+import 'package:front_end/src/fasta/problems.dart' show unhandled;
 import 'package:front_end/src/fasta/type_inference/type_inference_listener.dart';
 import 'package:front_end/src/fasta/type_inference/type_inferrer.dart';
 import 'package:front_end/src/fasta/type_inference/type_schema_environment.dart';
 import 'package:kernel/ast.dart'
     show
+        BottomType,
         Class,
         DartType,
         DynamicType,
@@ -21,13 +22,13 @@ import 'package:kernel/ast.dart'
         Member,
         Procedure,
         TypeParameter,
-        TypeParameterType;
+        TypeParameterType,
+        VariableDeclaration;
 import 'package:kernel/class_hierarchy.dart';
 import 'package:kernel/core_types.dart';
 import 'package:kernel/type_algebra.dart';
 
 import '../deprecated_problems.dart' show Crash;
-
 import '../messages.dart' show getLocationFromNode;
 
 /// Data structure for tracking dependencies among fields, getters, and setters
@@ -135,15 +136,19 @@ abstract class TypeInferenceEngine {
 
   CoreTypes get coreTypes;
 
-  /// Creates a type inferrer for use inside of a method body declared in a file
-  /// with the given [uri].
-  TypeInferrer createLocalTypeInferrer(
-      Uri uri, TypeInferenceListener listener, InterfaceType thisType);
+  /// Annotates the formal parameters of any methods in [cls] to indicate the
+  /// circumstances in which they require runtime type checks.
+  void computeFormalSafety(Class cls);
 
   /// Creates a disabled type inferrer (intended for debugging and profiling
   /// only).
   TypeInferrer createDisabledTypeInferrer();
 
+  /// Creates a type inferrer for use inside of a method body declared in a file
+  /// with the given [uri].
+  TypeInferrer createLocalTypeInferrer(
+      Uri uri, TypeInferenceListener listener, InterfaceType thisType);
+
   /// Creates a [TypeInferrer] object which is ready to perform type inference
   /// on the given [field].
   TypeInferrer createTopLevelTypeInferrer(TypeInferenceListener listener,
@@ -268,6 +273,28 @@ abstract class TypeInferenceEngineImpl extends TypeInferenceEngine {
     }
   }
 
+  @override
+  void computeFormalSafety(Class cls) {
+    if (cls.typeParameters.isEmpty) return;
+    var pessimization = Substitution.fromPairs(cls.typeParameters,
+        new List.filled(cls.typeParameters.length, const BottomType()));
+    for (var procedure in cls.procedures) {
+      if (procedure.isStatic) continue;
+      void compute(VariableDeclaration formal) {
+        KernelVariableDeclaration kernelVariableDeclaration = formal;
+        var pessimisticType = pessimization.substituteType(formal.type);
+        if (!typeSchemaEnvironment.isSubtypeOf(formal.type, pessimisticType)) {

[ahe, 2017/08/23 09:55:58]

I think this needs documentation. As far as I understand, you're trying to
recognize the following situation:

class A<T> {
  foo(A argument) {}
}

Why not:

var type = formal.type;
if (!(type is TypeParameterType && type.parameter.parent == cls)) { ... }

[Paul Berry, 2017/08/23 17:09:10]

I assume you mean:

class A<T> {
  foo(T argument) {}
}
That would be sufficient to identify the case you mention above, but it would
not identify other cases where the type parameter appears in the type of the
formal parameter in covariant fashion, e.g.:

class A<T> {
  foo(List<T> argument) {}
}

Or:

class A<T> {
  foo(T bar()) {}
}

Note however that not all uses of T in the type of foo's parameter are bad. 
This one is ok:

class A<T> {
  foo(void bar(T argument)) {}
}

I've added comments explaining the reasoning process.  In a follow up CL I'll
add additional tests to cover these corner cases.

[ahe, 2017/08/24 09:35:26]

Yes.
Thank you, I looked at your comments and it now makes sense to me. Next time, it
would be nice if you ensure that what you commit is uploaded as new patch set
(not that it needs another lgtm, but it's easier to comment if needed).

+          kernelVariableDeclaration.isSemiSafe = true;
+          instrumentation?.record(Uri.parse(cls.fileUri), formal.fileOffset,
+              'checkFormal', new InstrumentationValueLiteral('semiSafe'));
+        }
+      }
+
+      procedure.function.positionalParameters.forEach(compute);
+      procedure.function.namedParameters.forEach(compute);
+    }
+  }
+
   /// Creates an [AccessorNode] to track dependencies of the given [member].
   AccessorNode createAccessorNode(KernelMember member);