Skip to content

Commit f972214

Browse files
dcharkesCommit Bot
authored and
Commit Bot
committed
Revert "[cfe/ffi] Improve FFI call mismatched types compile errors"
This reverts commit 206fdf1. Reason for revert: Flutter issues are caught by this CL, preventing Flutter from building. flutter/flutter#108309 Original change's description: > [cfe/ffi] Improve FFI call mismatched types compile errors > > This CL fixes two issues. > > 1. `FfiNative`s now check the Dart and native type for compatiblity. > 2. Both `FfiNative`, `asFunction`, and `lookupFunction` check the type > correspondence between native and Dart type with a subtype check of > the expected Dart type and the provided Dart type. For functions, > any return type is a subtype of a void type. This is fine for Dart, > but not for native calls. This CL manually checks the return type > for void. > > This CL does not fix the inconsistency between `asFunction` and > `FfiNative` with regard to allowing more strict return types than > `Object` for `Handle`s > Issue: #49518 > > Analyzer fixes in follow up CL. > > TEST=tests/ffi/vmspecific_static_checks_ffinative_test.dart > > Closes: #49471 > Change-Id: Ibc7bd6a1a0db59cc5fa5d755d76999fd7e9a06a4 > Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,analyzer-mac-release-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/252601 > Reviewed-by: Martin Kustermann <[email protected]> > Commit-Queue: Daco Harkes <[email protected]> [email protected],[email protected] Change-Id: Id82b129d491adcc94cdd685a0a0f6a43248c71f2 No-Presubmit: true No-Tree-Checks: true No-Try: true Issue: #49518 Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,analyzer-mac-release-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/252662 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Daco Harkes <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent 1ab217a commit f972214

File tree

4 files changed

+5
-27
lines changed

4 files changed

+5
-27
lines changed

pkg/vm/lib/transformations/ffi/common.dart

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,14 +1120,7 @@ class FfiTransformer extends Transformer {
11201120
if (dartType == correspondingDartType) return;
11211121
if (env.isSubtypeOf(correspondingDartType, dartType,
11221122
SubtypeCheckMode.ignoringNullabilities)) {
1123-
// If subtype, manually check the return type is not void.
1124-
if (dartType is! FunctionType || correspondingDartType is! FunctionType) {
1125-
return;
1126-
} else if ((dartType.returnType is VoidType) ==
1127-
(correspondingDartType.returnType is VoidType)) {
1128-
return;
1129-
}
1130-
// One of the return types is void, the other isn't, report error.
1123+
return;
11311124
}
11321125
diagnosticReporter.report(
11331126
templateFfiTypeMismatch.withArguments(dartType, correspondingDartType,

pkg/vm/lib/transformations/ffi/native.dart

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,9 @@ class FfiNativeTransformer extends FfiTransformer {
117117
// Replaces return type with Object if it is Handle.
118118
DartType _wrapReturnType(DartType dartReturnType, DartType ffiReturnType) {
119119
if (env.isSubtypeOf(
120-
ffiReturnType,
121-
handleClass.getThisType(coreTypes, Nullability.nonNullable),
122-
SubtypeCheckMode.ignoringNullabilities) &&
123-
dartReturnType is! VoidType) {
120+
ffiReturnType,
121+
handleClass.getThisType(coreTypes, Nullability.nonNullable),
122+
SubtypeCheckMode.ignoringNullabilities)) {
124123
return objectClass.getThisType(coreTypes, dartReturnType.nullability);
125124
}
126125
return dartReturnType;
@@ -412,8 +411,7 @@ class FfiNativeTransformer extends FfiTransformer {
412411
nativeFunctionClass, Nullability.legacy, [ffiFunctionType]);
413412
try {
414413
ensureNativeTypeValid(nativeType, node);
415-
ensureNativeTypeToDartType(nativeType, wrappedDartFunctionType, node,
416-
allowHandle: true);
414+
ensureNativeTypeToDartType(nativeType, wrappedDartFunctionType, node);
417415
ensureLeafCallDoesNotUseHandles(nativeType, isLeaf, node);
418416
} on FfiStaticTypeError {
419417
// It's OK to swallow the exception because the diagnostics issued will

tests/ffi/vmspecific_static_checks_ffinative_test.dart

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ class NativeClassy extends NativeFieldWrapperClass1 {
3737
// Error: Missing receiver in FfiNative annotation.
3838
@FfiNative<Void Function(IntPtr)>('doesntmatter') //# 05: compile-time error
3939
external void badMissingReceiver(int v); //# 05: compile-time error
40-
41-
// Error: wrong return type.
42-
@FfiNative<Handle Function(Pointer<Void>, Uint32, Uint32, Handle)>('doesntmatter') //# 49471: compile-time error
43-
external void toImageSync(int width, int height, Object outImage); //# 49471: compile-time error
4440
}
4541

4642
// Error: Too many FfiNative parameters.

tests/ffi/vmspecific_static_checks_test.dart

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ void main() {
6464
testAsFunctionTakesHandle();
6565
testLookupFunctionReturnsHandle();
6666
testAsFunctionReturnsHandle();
67-
testReturnVoidNotVoid();
6867
}
6968

7069
typedef Int8UnOp = Int8 Function(Int8);
@@ -821,11 +820,3 @@ class MyFinalizableStruct extends Struct
821820
{
822821
external Pointer<Void> field;
823822
}
824-
825-
void testReturnVoidNotVoid() {
826-
// Taking a more specific argument is okay.
827-
testLibrary //# 49471: compile-time error
828-
.lookupFunction< //# 49471: compile-time error
829-
Handle Function(), //# 49471: compile-time error
830-
void Function()>("doesntmatter"); //# 49471: compile-time error
831-
}

0 commit comments

Comments
 (0)