[vm/ffi] Deprecate Pointer.elementAt and sizeOf generic calls

The analyzer and CFE now report a warning on calls to
`Pointer<T extends NativeType>.elementAt()` and
`sizeOf<T extends NativeType>()` where `T` is a generic.

Adapted from https://dart-review.googlesource.com/c/sdk/+/178200 to
only deprecate but not error out on generic calls.

Does not roll forward `package:ffi` to a version with the `Allocator`
but keeps the generic `sizeOf` invocation. This causes extra warnings
in the pkg/front_end testcases.

Issue: #38721

TEST=tests/ffi/data_test.dart
TEST=tests/ffi/sizeof_test.dart
TEST=tests/ffi/structs_test.dart
TEST=tests/ffi/vmspecific_static_checks_test.dart

Change-Id: I8f41c4dc04fc44e7e6c540ba87a3f41604130fe9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180560
Reviewed-by: Vyacheslav Egorov <[email protected]>

Author: dcharkes

pkg/analyzer/lib/src/generated/ffi_verifier.dart

@@ -174,6 +174,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
         if (_isPointer(enclosingElement)) {
           if (element.name == 'fromFunction') {
             _validateFromFunction(node, element);
+          } else if (element.name == 'elementAt') {
+            _validateElementAt(node);
           }
         }
       }
@@ -187,6 +189,15 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
           _validateLookupFunction(node);
         }
       }
+    } else if (element is FunctionElement) {
+      Element enclosingElement = element.enclosingElement;
+      if (enclosingElement is CompilationUnitElement) {
+        if (element.library.name == 'dart.ffi') {
+          if (element.name == 'sizeOf') {
+            _validateSizeOf(node);
+          }
+        }
+      }
     }
     super.visitMethodInvocation(node);
   }
@@ -618,6 +629,24 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  void _validateElementAt(MethodInvocation node) {
+    Expression target = node.realTarget;
+    DartType targetType = target.staticType;
+    if (targetType is InterfaceType &&
+        _isPointer(targetType.element) &&
+        targetType.typeArguments.length == 1) {
+      final DartType T = targetType.typeArguments[0];
+
+      if (!_isValidFfiNativeType(T, true, true)) {
+        final AstNode errorNode = node;
+        _errorReporter.reportErrorForNode(
+            FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING,
+            errorNode,
+            ['elementAt']);
+      }
+    }
+  }
+
   /// Validate that the fields declared by the given [node] meet the
   /// requirements for fields within a struct class.
   void _validateFieldsInStruct(FieldDeclaration node) {
@@ -771,6 +800,15 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  void _validateSizeOf(MethodInvocation node) {
+    final DartType T = node.typeArgumentTypes[0];
+    if (!_isValidFfiNativeType(T, true, true)) {
+      final AstNode errorNode = node;
+      _errorReporter.reportErrorForNode(
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, errorNode, ['sizeOf']);
+    }
+  }
+
   /// Validate that the given [typeArgument] has a constant value. Return `true`
   /// if a diagnostic was produced because it isn't constant.
   bool _validateTypeArgument(TypeAnnotation typeArgument, String functionName) {

pkg/front_end/testcases/general_nnbd_opt_out/ffi_sample.dart.weak.transformed.expect

@@ -1,3 +1,8 @@
+//
+// Problems outside component:
+//
+// third_party/pkg/ffi/lib/src/allocation.dart: Info: Support for using non-constant type arguments 'T' in this FFI API is deprecated and will be removed in the next stable version of Dart. Rewrite the code to ensure that type arguments are compile time constants referring to a valid native type.
+//
 library;
 import self as self;
 import "dart:core" as core;

pkg/front_end/testcases/nnbd/ffi_sample.dart.strong.transformed.expect

@@ -1,3 +1,8 @@
+//
+// Problems outside component:
+//
+// third_party/pkg/ffi/lib/src/allocation.dart: Info: Support for using non-constant type arguments 'T' in this FFI API is deprecated and will be removed in the next stable version of Dart. Rewrite the code to ensure that type arguments are compile time constants referring to a valid native type.
+//
 library /*isNonNullableByDefault*/;
 import self as self;
 import "dart:core" as core;

pkg/front_end/testcases/nnbd/ffi_sample.dart.weak.transformed.expect

@@ -1,3 +1,8 @@
+//
+// Problems outside component:
+//
+// third_party/pkg/ffi/lib/src/allocation.dart: Info: Support for using non-constant type arguments 'T' in this FFI API is deprecated and will be removed in the next stable version of Dart. Rewrite the code to ensure that type arguments are compile time constants referring to a valid native type.
+//
 library /*isNonNullableByDefault*/;
 import self as self;
 import "dart:core" as core;

pkg/vm/lib/transformations/ffi.dart

@@ -223,6 +223,7 @@ class FfiTransformer extends Transformer {
   final Procedure structPointerElemAt;
   final Procedure asFunctionMethod;
   final Procedure asFunctionInternal;
+  final Procedure sizeOfMethod;
   final Procedure lookupFunctionMethod;
   final Procedure fromFunctionMethod;
   final Field addressOfField;
@@ -292,6 +293,7 @@ class FfiTransformer extends Transformer {
             index.getMember('dart:ffi', 'NativeFunctionPointer', 'asFunction'),
         asFunctionInternal =
             index.getTopLevelMember('dart:ffi', '_asFunctionInternal'),
+        sizeOfMethod = index.getTopLevelMember('dart:ffi', 'sizeOf'),
         lookupFunctionMethod = index.getMember(
             'dart:ffi', 'DynamicLibraryExtension', 'lookupFunction'),
         fromFunctionMethod =

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -181,6 +181,15 @@ class _FfiUseSiteTransformer extends FfiTransformer {
 
         // TODO(http://dartbug.com/38721): Replace calls with direct
         // constructor invocations.
+      } else if (target == sizeOfMethod) {
+        final DartType nativeType = node.arguments.types[0];
+
+        if (!isFfiLibrary) {
+          _warningNativeTypeValid(nativeType, node);
+        }
+
+        // TODO(http://dartbug.com/38721): Replace calls with constant
+        // expressions.
       } else if (target == lookupFunctionMethod) {
         final DartType nativeType = InterfaceType(
             nativeFunctionClass, Nullability.legacy, [node.arguments.types[0]]);
@@ -384,6 +393,9 @@ class _FfiUseSiteTransformer extends FfiTransformer {
         final DartType pointerType =
             node.receiver.getStaticType(_staticTypeContext);
         final DartType nativeType = _pointerTypeGetTypeArg(pointerType);
+
+        _warningNativeTypeValid(nativeType, node);
+
         if (nativeType is TypeParameterType) {
           // Do not rewire generic invocations.
           return node;

sdk/lib/ffi/ffi.dart

@@ -25,6 +25,10 @@ part "struct.dart";
 /// Number of bytes used by native type T.
 ///
 /// Includes padding and alignment of structs.
+///
+/// Support for invoking this function with non-constant [T] will be removed in
+/// the next stable version of Dart and it will become mandatory to invoke it
+/// with a compile-time constant [T].
 external int sizeOf<T extends NativeType>();
 
 /// Represents a pointer into the native C memory corresponding to "NULL", e.g.
@@ -62,6 +66,10 @@ class Pointer<T extends NativeType> extends NativeType {
   external int get address;
 
   /// Pointer arithmetic (takes element size into account).
+  ///
+  /// Support for invoking this method with non-constant [T] will be removed in
+  /// the next stable version of Dart and it will become mandatory to invoke it
+  /// with a compile-time constant [T].
   external Pointer<T> elementAt(int index);
 
   /// Cast Pointer<T> to a Pointer<V>.

tests/ffi/data_test.dart

@@ -47,10 +47,8 @@ void main() {
   testEquality();
   testAllocateVoid();
   testAllocateNativeFunction();
-  testSizeOfGeneric();
   testSizeOfVoid();
   testSizeOfNativeFunction();
-  testSizeOfNativeType();
   testDynamicInvocation();
   testMemoryAddressTruncation();
   testNullptrCast();
@@ -434,17 +432,6 @@ void testAllocateNativeFunction() {
   });
 }
 
-void testSizeOfGeneric() {
-  int generic<T extends Pointer>() {
-    int size;
-    size = sizeOf<T>();
-    return size;
-  }
-
-  int size = generic<Pointer<Int64>>();
-  Expect.isTrue(size == 8 || size == 4);
-}
-
 void testSizeOfVoid() {
   Expect.throws(() {
     sizeOf<Void>();
@@ -457,12 +444,6 @@ void testSizeOfNativeFunction() {
   });
 }
 
-void testSizeOfNativeType() {
-  Expect.throws(() {
-    sizeOf();
-  });
-}
-
 void testDynamicInvocation() {
   dynamic p = calloc<Int8>();
   Expect.throws(() {

tests/ffi/vmspecific_static_checks_test.dart

@@ -57,6 +57,10 @@ void main() {
   testAllocateGeneric();
   testAllocateNativeType();
   testRefStruct();
+  testSizeOfGeneric();
+  testSizeOfNativeType();
+  testElementAtGeneric();
+  testElementAtNativeType();
 }
 
 typedef Int8UnOp = Int8 Function(Int8);
@@ -618,3 +622,42 @@ T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: ok
 
 T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: ok
     p[0]; //# 1202: ok
+
+void testSizeOfGeneric() {
+  int generic<T extends Pointer>() {
+    int size = sizeOf<IntPtr>();
+    size = sizeOf<T>(); //# 1300: ok
+    return size;
+  }
+
+  int size = generic<Pointer<Int64>>();
+}
+
+void testSizeOfNativeType() {
+  try {
+    sizeOf(); //# 1301: ok
+  } catch (e) {
+    print(e);
+  }
+}
+
+void testElementAtGeneric() {
+  Pointer<T> generic<T extends NativeType>(Pointer<T> pointer) {
+    Pointer<T> returnValue = pointer;
+    returnValue = returnValue.elementAt(1); //# 1310: ok
+    return returnValue;
+  }
+
+  Pointer<Int8> p = calloc();
+  p.elementAt(1);
+  generic(p);
+  calloc.free(p);
+}
+
+void testElementAtNativeType() {
+  Pointer<Int8> p = calloc();
+  p.elementAt(1);
+  Pointer<NativeType> p2 = p;
+  p2.elementAt(1); //# 1311: ok
+  calloc.free(p);
+}

tests/ffi_2/data_test.dart

@@ -47,10 +47,8 @@ void main() {
   testEquality();
   testAllocateVoid();
   testAllocateNativeFunction();
-  testSizeOfGeneric();
   testSizeOfVoid();
   testSizeOfNativeFunction();
-  testSizeOfNativeType();
   testDynamicInvocation();
   testMemoryAddressTruncation();
   testNullptrCast();
@@ -434,17 +432,6 @@ void testAllocateNativeFunction() {
   });
 }
 
-void testSizeOfGeneric() {
-  int generic<T extends Pointer>() {
-    int size;
-    size = sizeOf<T>();
-    return size;
-  }
-
-  int size = generic<Pointer<Int64>>();
-  Expect.isTrue(size == 8 || size == 4);
-}
-
 void testSizeOfVoid() {
   Expect.throws(() {
     sizeOf<Void>();
@@ -457,12 +444,6 @@ void testSizeOfNativeFunction() {
   });
 }
 
-void testSizeOfNativeType() {
-  Expect.throws(() {
-    sizeOf();
-  });
-}
-
 void testDynamicInvocation() {
   dynamic p = calloc<Int8>();
   Expect.throws(() {

tests/ffi_2/vmspecific_static_checks_test.dart

@@ -57,6 +57,10 @@ void main() {
   testAllocateGeneric();
   testAllocateNativeType();
   testRefStruct();
+  testSizeOfGeneric();
+  testSizeOfNativeType();
+  testElementAtGeneric();
+  testElementAtNativeType();
 }
 
 typedef Int8UnOp = Int8 Function(Int8);
@@ -616,3 +620,42 @@ T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: ok
 
 T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: ok
     p[0]; //# 1202: ok
+
+void testSizeOfGeneric() {
+  int generic<T extends Pointer>() {
+    int size = sizeOf<IntPtr>();
+    size = sizeOf<T>(); //# 1300: ok
+    return size;
+  }
+
+  int size = generic<Pointer<Int64>>();
+}
+
+void testSizeOfNativeType() {
+  try {
+    sizeOf(); //# 1301: ok
+  } catch (e) {
+    print(e);
+  }
+}
+
+void testElementAtGeneric() {
+  Pointer<T> generic<T extends NativeType>(Pointer<T> pointer) {
+    Pointer<T> returnValue = pointer;
+    returnValue = returnValue.elementAt(1); //# 1310: ok
+    return returnValue;
+  }
+
+  Pointer<Int8> p = calloc();
+  p.elementAt(1);
+  generic(p);
+  calloc.free(p);
+}
+
+void testElementAtNativeType() {
+  Pointer<Int8> p = calloc();
+  p.elementAt(1);
+  Pointer<NativeType> p2 = p;
+  p2.elementAt(1); //# 1311: ok
+  calloc.free(p);
+}