Skip to content

Commit

Permalink
[vm/ffi] Support inline arrays in Structs
Browse files Browse the repository at this point in the history
Adds support for single dimension inline arrays in structs. Multi-
dimensional arrays will be supported in a future CL.

This CL adds:
- CFE static error checks for inline arrays.
- CFE transformations for inline arrays.
- VM consumption of inline array fields for NativeType.
- Test generator support for inline arrays + generated tests.

Previous CLs added support for inline arrays in:
- analyzer https://dart-review.googlesource.com/c/sdk/+/183684
  - updated in this CL to new API.
- ABI calculation https://dart-review.googlesource.com/c/sdk/+/183682

Closes: #35763

Open issue: #45101

CFE transformations are tested with expectation files:
TEST=pkg/front_end/testcases/(.*)/ffi_struct_inline_array.dart

Trampolines and CArray API are tested with end-to-end Dart tests:
TEST=tests/ffi(_2)/(.*)by_value(.*)test.dart
TEST=tests/ffi(_2)/inline_array_test.dart

Compile-time errors (both CFE and analyzer) are tested in:
TEST=tests/ffi(_2)/vmspecific_static_checks_test.dart

Change-Id: I014c0e4153f1b885638adce80de6ab3cac8e6bb2
Cq-Include-Trybots: luci.dart.try:dart-sdk-linux-try,dart-sdk-mac-try,dart-sdk-win-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-mac-release-x64-try,vm-kernel-nnbd-win-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-precomp-android-release-arm_x64-try,analyzer-analysis-server-linux-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183640
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
  • Loading branch information
dcharkes authored and commit-bot@chromium.org committed Feb 24, 2021
1 parent 3a4a4f5 commit d45fd0f
Show file tree
Hide file tree
Showing 42 changed files with 6,549 additions and 263 deletions.
22 changes: 22 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Expand Up @@ -3903,6 +3903,28 @@ Message _withArgumentsFfiNotStatic(String name) {
arguments: {'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name)> templateFfiSizeAnnotation =
const Template<Message Function(String name)>(
messageTemplate:
r"""Field '#name' must have exactly one 'Array' annotation.""",
withArguments: _withArgumentsFfiSizeAnnotation);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeFfiSizeAnnotation =
const Code<Message Function(String name)>(
"FfiSizeAnnotation",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsFfiSizeAnnotation(String name) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
return new Message(codeFfiSizeAnnotation,
message: """Field '${name}' must have exactly one 'Array' annotation.""",
arguments: {'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name)> templateFfiStructGeneric =
const Template<Message Function(String name)>(
Expand Down
10 changes: 5 additions & 5 deletions pkg/analyzer/lib/src/dart/error/ffi_code.dart
Expand Up @@ -44,7 +44,7 @@ class FfiCode extends AnalyzerErrorCode {
*/
static const FfiCode EXTRA_SIZE_ANNOTATION_CARRAY = FfiCode(
name: 'EXTRA_SIZE_ANNOTATION_CARRAY',
message: "'CArray's must have exactly one 'CArraySize' annotation.",
message: "'Array's must have exactly one 'Array' annotation.",
correction: "Try removing the extra annotation.");

/**
Expand Down Expand Up @@ -94,10 +94,10 @@ class FfiCode extends AnalyzerErrorCode {
name: 'INVALID_FIELD_TYPE_IN_STRUCT',
message:
"Fields in struct classes can't have the type '{0}'. They can only "
"be declared as 'int', 'double', 'CArray', 'Pointer', or subtype of "
"be declared as 'int', 'double', 'Array', 'Pointer', or subtype of "
"'Struct'.",
correction:
"Try using 'int', 'double', 'CArray', 'Pointer', or subtype of "
"Try using 'int', 'double', 'Array', 'Pointer', or subtype of "
"'Struct'.");

/**
Expand Down Expand Up @@ -146,8 +146,8 @@ class FfiCode extends AnalyzerErrorCode {
*/
static const FfiCode MISSING_SIZE_ANNOTATION_CARRAY = FfiCode(
name: 'MISSING_SIZE_ANNOTATION_CARRAY',
message: "'CArray's must have exactly one 'CArraySize' annotation.",
correction: "Try adding a 'CArraySize' annotation.");
message: "'Array's must have exactly one 'Array' annotation.",
correction: "Try adding a 'Array' annotation.");

/**
* Parameters:
Expand Down
24 changes: 12 additions & 12 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
Expand Up @@ -18,7 +18,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
static const _allocatorClassName = 'Allocator';
static const _allocateExtensionMethodName = 'call';
static const _allocatorExtensionName = 'AllocatorAlloc';
static const _cArrayClassName = 'CArray';
static const _cArrayClassName = 'Array';
static const _dartFfiLibraryName = 'dart.ffi';
static const _opaqueClassName = 'Opaque';

Expand Down Expand Up @@ -159,7 +159,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (element is MethodElement) {
var enclosingElement = element.enclosingElement;
if (enclosingElement.isNativeStructPointerExtension ||
enclosingElement.isNativeStructCArrayExtension) {
enclosingElement.isNativeStructArrayExtension) {
if (element.name == '[]') {
_validateRefIndexed(node);
}
Expand Down Expand Up @@ -279,7 +279,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
bool _isValidFfiNativeType(DartType? nativeType,
{bool allowVoid = false,
bool allowEmptyStruct = false,
bool allowCArray = false}) {
bool allowArray = false}) {
if (nativeType is InterfaceType) {
// Is it a primitive integer/double type (or ffi.Void if we allow it).
final primitiveType = _primitiveNativeType(nativeType);
Expand Down Expand Up @@ -310,7 +310,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (nativeType.isOpaqueSubtype) {
return true;
}
if (allowCArray && nativeType.isCArray) {
if (allowArray && nativeType.isArray) {
return _isValidFfiNativeType(nativeType.typeArguments.single,
allowVoid: false, allowEmptyStruct: false);
}
Expand Down Expand Up @@ -550,7 +550,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
_validateAnnotations(fieldType, annotations, _PrimitiveDartType.double);
} else if (declaredType.isPointer) {
_validateNoAnnotations(annotations);
} else if (declaredType.isCArray) {
} else if (declaredType.isArray) {
final typeArg = (declaredType as InterfaceType).typeArguments.single;
if (!_isSized(typeArg)) {
_errorReporter.reportErrorForNode(FfiCode.NON_SIZED_TYPE_ARGUMENT,
Expand Down Expand Up @@ -663,7 +663,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
void _validateRefIndexed(IndexExpression node) {
var targetType = node.realTarget.staticType;
if (!_isValidFfiNativeType(targetType,
allowVoid: false, allowEmptyStruct: true, allowCArray: true)) {
allowVoid: false, allowEmptyStruct: true, allowArray: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['[]']);
Expand Down Expand Up @@ -714,7 +714,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
final element = annotation.element;
return element is ConstructorElement &&
element.ffiClass != null &&
element.enclosingElement.name == 'CArraySize';
element.enclosingElement.name == 'Array';
}).toList();

if (ffiSizeAnnotations.isEmpty) {
Expand Down Expand Up @@ -766,10 +766,10 @@ extension on Element? {
element.isFfiExtension;
}

bool get isNativeStructCArrayExtension {
bool get isNativeStructArrayExtension {
final element = this;
return element is ExtensionElement &&
element.name == 'StructCArray' &&
element.name == 'StructArray' &&
element.isFfiExtension;
}

Expand Down Expand Up @@ -821,7 +821,7 @@ extension on ClassElement {
return false;
} else if (declaredType.isStructSubtype) {
return false;
} else if (declaredType.isCArray) {
} else if (declaredType.isArray) {
return false;
}
}
Expand All @@ -840,8 +840,8 @@ extension on ExtensionElement {
}

extension on DartType {
/// Return `true` if this represents the class `CArray`.
bool get isCArray {
/// Return `true` if this represents the class `Array`.
bool get isArray {
final self = this;
if (self is InterfaceType) {
final element = self.element;
Expand Down
8 changes: 2 additions & 6 deletions pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Expand Up @@ -690,12 +690,8 @@ class DartRepresentationOf {
const DartRepresentationOf(String nativeType);
}
class CArray<T extends NativeType> extends NativeType {}
class CArraySize {
final int numberOfElements;
const CArraySize(this.numberOfElements);
class Array<T extends NativeType> extends NativeType {
external const factory Array(int dimension1);
}
extension StructPointer<T extends Struct> on Pointer<T> {
Expand Down
Expand Up @@ -9,19 +9,19 @@ import '../dart/resolution/context_collection_resolution.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(ExtraSizeAnnotationCArray);
defineReflectiveTests(ExtraSizeAnnotationArray);
});
}

@reflectiveTest
class ExtraSizeAnnotationCArray extends PubPackageResolutionTest {
class ExtraSizeAnnotationArray extends PubPackageResolutionTest {
test_one() async {
await assertNoErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
CArray<Uint8> a0;
@Array(8)
Array<Uint8> a0;
}
''');
}
Expand All @@ -31,12 +31,12 @@ class C extends Struct {
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
@CArraySize(8)
CArray<Uint8> a0;
@Array(8)
@Array(8)
Array<Uint8> a0;
}
''', [
error(FfiCode.EXTRA_SIZE_ANNOTATION_CARRAY, 64, 14),
error(FfiCode.EXTRA_SIZE_ANNOTATION_CARRAY, 59, 9),
]);
}
}
Expand Up @@ -9,19 +9,19 @@ import '../dart/resolution/context_collection_resolution.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(MissingSizeAnnotationCArray);
defineReflectiveTests(MissingSizeAnnotationArray);
});
}

@reflectiveTest
class MissingSizeAnnotationCArray extends PubPackageResolutionTest {
class MissingSizeAnnotationArray extends PubPackageResolutionTest {
test_one() async {
await assertNoErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
CArray<Uint8> a0;
@Array(8)
Array<Uint8> a0;
}
''');
}
Expand All @@ -31,10 +31,10 @@ class C extends Struct {
import 'dart:ffi';
class C extends Struct {
CArray<Uint8> a0;
Array<Uint8> a0;
}
''', [
error(FfiCode.MISSING_SIZE_ANNOTATION_CARRAY, 47, 13),
error(FfiCode.MISSING_SIZE_ANNOTATION_CARRAY, 47, 12),
]);
}
}
Expand Up @@ -20,8 +20,8 @@ class NonSizedTypeArgument extends PubPackageResolutionTest {
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
CArray<Uint8> a0;
@Array(8)
Array<Uint8> a0;
}
''');
}
Expand All @@ -31,11 +31,11 @@ class C extends Struct {
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
CArray<CArray<Uint8>> a0;
@Array(8)
Array<Array<Uint8>> a0;
}
''', [
error(FfiCode.NON_SIZED_TYPE_ARGUMENT, 64, 21),
error(FfiCode.NON_SIZED_TYPE_ARGUMENT, 59, 19),
]);
}
}
1 change: 1 addition & 0 deletions pkg/front_end/lib/src/api_unstable/vm.dart
Expand Up @@ -64,6 +64,7 @@ export '../fasta/fasta_codes.dart'
templateFfiFieldNoAnnotation,
templateFfiFieldNull,
templateFfiNotStatic,
templateFfiSizeAnnotation,
templateFfiStructGeneric,
templateFfiTypeInvalid,
templateFfiTypeMismatch;
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/messages.status
Expand Up @@ -329,6 +329,7 @@ FfiFieldInitializer/analyzerCode: Fail
FfiFieldNoAnnotation/analyzerCode: Fail
FfiFieldNull/analyzerCode: Fail
FfiNotStatic/analyzerCode: Fail
FfiSizeAnnotation/analyzerCode: Fail
FfiStructAnnotation/analyzerCode: Fail
FfiStructGeneric/analyzerCode: Fail
FfiTypeInvalid/analyzerCode: Fail
Expand Down
5 changes: 5 additions & 0 deletions pkg/front_end/messages.yaml
Expand Up @@ -4263,6 +4263,11 @@ FfiExtendsOrImplementsSealedClass:
template: "Class '#name' cannot be extended or implemented."
external: test/ffi_test.dart

FfiSizeAnnotation:
# Used by dart:ffi
template: "Field '#name' must have exactly one 'Array' annotation."
external: test/ffi_test.dart

FfiStructGeneric:
# Used by dart:ffi
template: "Struct '#name' should not be generic."
Expand Down
14 changes: 14 additions & 0 deletions pkg/front_end/testcases/nnbd/ffi_struct_inline_array.dart
@@ -0,0 +1,14 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:ffi';

import "package:ffi/ffi.dart";

class StructInlineArray extends Struct {
@Array(8)
external Array<Uint8> a0;
}

main() {}
@@ -0,0 +1,27 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:ffi" as ffi;

import "dart:ffi";
import "package:ffi/ffi.dart";

class StructInlineArray extends ffi::Struct {
synthetic constructor •() → self::StructInlineArray
: super ffi::Struct::•()
;
@#C2
external get a0() → ffi::Array<ffi::Uint8>;
@#C2
external set a0(ffi::Array<ffi::Uint8> #externalFieldValue) → void;
}
static method main() → dynamic {}

constants {
#C1 = 8
#C2 = ffi::_ArraySize<ffi::NativeType> {dimension1:#C1}
}


Constructor coverage from constants:
org-dartlang-testcase:///ffi_struct_inline_array.dart:
- _ArraySize. (from org-dartlang-sdk:///sdk/lib/ffi/ffi.dart:107:9)

0 comments on commit d45fd0f

Please sign in to comment.