Reland "[vm/ffi] Change Pointer<T extends Struct>.ref to use static type"

This CL changes the semantics of `Pointer<T extends Struct>.ref` to use
the compile-time `T` rather than the runtime `T`.

This enables tree shaking of subtypes of Struct and optimizing `.ref`.

Bug: #38721

TEST=tests/ffi/vmspecific_static_checks_test.dart
TEST=tests/ffi/*struct*test_.dart

Change-Id: Ie19bc3259d1cb721d0ce56d68e82d09dc3a4ad0e
Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,analyzer-nnbd-linux-release-try,app-kernel-linux-debug-x64-try,dart-sdk-linux-try,front-end-nnbd-linux-release-x64-try,pkg-linux-debug-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180190
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Aske Simon Christensen <[email protected]>

Author: dcharkes

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

@@ -785,7 +785,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     if (!_isValidFfiNativeType(targetType, false, true)) {
       final AstNode errorNode = node;
       _errorReporter.reportErrorForNode(
-          FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, errorNode, ['[]']);
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['[]']);
     }
   }
 
@@ -796,7 +796,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     if (!_isValidFfiNativeType(targetType, false, true)) {
       final AstNode errorNode = node;
       _errorReporter.reportErrorForNode(
-          FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, errorNode, ['ref']);
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
     }
   }
 
@@ -805,7 +805,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
     if (!_isValidFfiNativeType(targetType, false, true)) {
       final AstNode errorNode = node;
       _errorReporter.reportErrorForNode(
-          FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, errorNode, ['ref']);
+          FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
     }
   }
 

pkg/analyzer/test/src/diagnostics/non_constant_type_argument_test.dart

@@ -72,6 +72,6 @@ import 'dart:ffi';
 
 T genericRef<T extends Struct>(Pointer<T> p) =>
     p.ref;
-''', [error(FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, 72, 5)]);
+''', [error(FfiCode.NON_CONSTANT_TYPE_ARGUMENT, 72, 5)]);
   }
 }

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -184,13 +184,7 @@ class _FfiUseSiteTransformer extends FfiTransformer {
       if (target == structPointerRef || target == structPointerElemAt) {
         final DartType nativeType = node.arguments.types[0];
 
-        _warningNativeTypeValid(nativeType, node, allowStructItself: false);
-
-        // TODO(http://dartbug.com/38721): Change this to an error.
-        if (nativeType is TypeParameterType) {
-          // Do not rewire generic invocations.
-          return node;
-        }
+        _ensureNativeTypeValid(nativeType, node, allowStructItself: false);
 
         return _replaceRef(node);
       } else if (target == sizeOfMethod) {

runtime/lib/ffi.cc

@@ -116,7 +116,7 @@ static ObjectPtr LoadValueStruct(Zone* zone,
 DEFINE_NATIVE_ENTRY(Ffi_loadStruct, 0, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
   const AbstractType& pointer_type_arg =
-      AbstractType::Handle(pointer.type_argument());
+      AbstractType::Handle(arguments->NativeTypeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Integer, index, arguments->NativeArgAt(1));
 
   // TODO(36370): Make representation consistent with kUnboxedFfiIntPtr.

sdk/lib/_internal/vm/lib/ffi_patch.dart

@@ -204,6 +204,13 @@ double _loadDouble(Pointer pointer, int offsetInBytes) native "Ffi_loadDouble";
 Pointer<S> _loadPointer<S extends NativeType>(
     Pointer pointer, int offsetInBytes) native "Ffi_loadPointer";
 
+S _loadStructNoStruct<S extends Struct>(Pointer<S> pointer, int index) {
+  if (S == Struct) {
+    throw ArgumentError("S should be a subtype of Struct.");
+  }
+  return _loadStruct(pointer, index);
+}
+
 S _loadStruct<S extends Struct>(Pointer<S> pointer, int index)
     native "Ffi_loadStruct";
 
@@ -511,10 +518,10 @@ extension PointerPointer<T extends NativeType> on Pointer<Pointer<T>> {
 
 extension StructPointer<T extends Struct> on Pointer<T> {
   @patch
-  T get ref => _loadStruct(this, 0);
+  T get ref => _loadStructNoStruct(this, 0);
 
   @patch
-  T operator [](int index) => _loadStruct(this, index);
+  T operator [](int index) => _loadStructNoStruct(this, index);
 }
 
 extension NativePort on SendPort {

sdk/lib/ffi/ffi.dart

@@ -545,9 +545,7 @@ extension StructPointer<T extends Struct> on Pointer<T> {
   /// The [address] must be aligned according to the struct alignment rules of
   /// the platform.
   ///
-  /// Support for invoking this extension 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].
+  /// This extension method must be invoked with a compile-time constant [T].
   external T get ref;
 
   /// Creates a reference to access the fields of this struct backed by native
@@ -556,9 +554,7 @@ extension StructPointer<T extends Struct> on Pointer<T> {
   /// The [address] must be aligned according to the struct alignment rules of
   /// the platform.
   ///
-  /// Support for invoking this extension 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].
+  /// This extension method must be invoked with a compile-time constant [T].
   external T operator [](int index);
 }
 

tests/ffi/vmspecific_static_checks_test.dart

@@ -610,18 +610,18 @@ void testAllocateNativeType() {
 void testRefStruct() {
   final myStructPointer = calloc<TestStruct13>();
   Pointer<Struct> structPointer = myStructPointer;
-  structPointer.ref; //# 1330: ok
+  structPointer.ref; //# 1330: compile-time error
   calloc.free(myStructPointer);
 }
 
-T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: ok
-    p.ref; //# 1200: ok
+T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: compile-time error
+    p.ref; //# 1200: compile-time error
 
-T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: ok
-    p.cast<T>().ref; //# 1201: ok
+T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: compile-time error
+    p.cast<T>().ref; //# 1201: compile-time error
 
-T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: ok
-    p[0]; //# 1202: ok
+T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: compile-time error
+    p[0]; //# 1202: compile-time error
 
 void testSizeOfGeneric() {
   int generic<T extends Pointer>() {

tests/ffi_2/vmspecific_static_checks_test.dart

@@ -608,18 +608,18 @@ void testAllocateNativeType() {
 void testRefStruct() {
   final myStructPointer = calloc<TestStruct13>();
   Pointer<Struct> structPointer = myStructPointer;
-  structPointer.ref; //# 1330: ok
+  structPointer.ref; //# 1330: compile-time error
   calloc.free(myStructPointer);
 }
 
-T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: ok
-    p.ref; //# 1200: ok
+T genericRef<T extends Struct>(Pointer<T> p) => //# 1200: compile-time error
+    p.ref; //# 1200: compile-time error
 
-T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: ok
-    p.cast<T>().ref; //# 1201: ok
+T genericRef2<T extends Struct>(Pointer<T> p) => //# 1201: compile-time error
+    p.cast<T>().ref; //# 1201: compile-time error
 
-T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: ok
-    p[0]; //# 1202: ok
+T genericRef3<T extends Struct>(Pointer<T> p) => //# 1202: compile-time error
+    p[0]; //# 1202: compile-time error
 
 void testSizeOfGeneric() {
   int generic<T extends Pointer>() {