Skip to content

Commit

Permalink
[analyzer/ffi] Support inline arrays in Structs
Browse files Browse the repository at this point in the history 
This CL does not add inline arrays to `dart:ffi` yet, only to the mock
SDK in the analyzer for testing the analyzer. This means we can review
and land it separately.

Closes: #44747
Bug: #35763

Change-Id: I7df6a61ea4cfa522afd12194dd2f3573eb79b3ef
Cq-Include-Trybots: luci.dart.try:analyzer-analysis-server-linux-try,analyzer-linux-release-try,analyzer-nnbd-linux-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183684
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
  • Loading branch information
@dcharkes
dcharkes authored and [email protected] committed Feb 9, 2021
1 parent ce81216 commit 5c4a916
Show file tree
Hide file tree
Showing 8 changed files with 270 additions and 15 deletions.
3 changes: 3 additions & 0 deletions pkg/analyzer/lib/error/error.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ const List<ErrorCode> errorCodeValues = [
FfiCode.EMPTY_STRUCT,
FfiCode.EMPTY_STRUCT_WARNING,
FfiCode.EXTRA_ANNOTATION_ON_STRUCT_FIELD,
FfiCode.EXTRA_SIZE_ANNOTATION_CARRAY,
FfiCode.FIELD_IN_STRUCT_WITH_INITIALIZER,
FfiCode.FIELD_INITIALIZER_IN_STRUCT,
FfiCode.GENERIC_STRUCT_SUBCLASS,
Expand All @@ -470,11 +471,13 @@ const List<ErrorCode> errorCodeValues = [
FfiCode.MISSING_ANNOTATION_ON_STRUCT_FIELD,
FfiCode.MISSING_EXCEPTION_VALUE,
FfiCode.MISSING_FIELD_TYPE_IN_STRUCT,
FfiCode.MISSING_SIZE_ANNOTATION_CARRAY,
FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE,
FfiCode.MUST_BE_A_SUBTYPE,
FfiCode.NON_CONSTANT_TYPE_ARGUMENT,
FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING,
FfiCode.NON_NATIVE_FUNCTION_TYPE_ARGUMENT_TO_POINTER,
FfiCode.NON_SIZED_TYPE_ARGUMENT,
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS,
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS,
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_WITH,
Expand Down
35 changes: 33 additions & 2 deletions pkg/analyzer/lib/src/dart/error/ffi_code.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ class FfiCode extends AnalyzerErrorCode {
"indicating the native type.",
correction: "Try removing the extra annotation.");

/**
* No parameters.
*/
static const FfiCode EXTRA_SIZE_ANNOTATION_CARRAY = FfiCode(
name: 'EXTRA_SIZE_ANNOTATION_CARRAY',
message: "'CArray's must have exactly one 'CArraySize' annotation.",
correction: "Try removing the extra annotation.");

/**
* No parameters.
*/
Expand Down Expand Up @@ -97,9 +105,11 @@ 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', 'Pointer', or subtype of 'Struct'.",
"be declared as 'int', 'double', 'CArray', 'Pointer', or subtype of "
"'Struct'.",
correction:
"Try using 'int', 'double', 'Pointer', or subtype of 'Struct'.");
"Try using 'int', 'double', 'CArray', 'Pointer', or subtype of "
"'Struct'.");

/**
* No parameters.
Expand Down Expand Up @@ -142,6 +152,14 @@ class FfiCode extends AnalyzerErrorCode {
"'int', 'double' or 'Pointer'.",
correction: "Try using 'int', 'double' or 'Pointer'.");

/**
* No parameters.
*/
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.");

/**
* Parameters:
* 0: the type that should be a valid dart:ffi native type.
Expand Down Expand Up @@ -200,6 +218,19 @@ class FfiCode extends AnalyzerErrorCode {
"'NativeFunction' in order to use 'asFunction'.",
correction: "Try changing the type argument to be a 'NativeFunction'.");

/**
* Parameters:
* 0: the type of the field
*/
static const FfiCode NON_SIZED_TYPE_ARGUMENT = FfiCode(
name: 'NON_SIZED_TYPE_ARGUMENT',
message:
"Type arguments to '{0}' can't have the type '{1}'. They can only "
"be declared as native integer, 'Float', 'Double', 'Pointer', or "
"subtype of Struct'.",
correction: "Try using a native integer, 'Float', 'Double', 'Pointer', "
"or subtype of 'Struct'.");

/**
* Parameters:
* 0: the name of the subclass
Expand Down
109 changes: 96 additions & 13 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
static const _allocatorClassName = 'Allocator';
static const _allocateExtensionMethodName = 'call';
static const _allocatorExtensionName = 'AllocatorAlloc';
static const _carrayClassName = 'CArray';
static const _dartFfiLibraryName = 'dart.ffi';
static const _opaqueClassName = 'Opaque';

Expand Down Expand Up @@ -158,7 +159,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (element is MethodElement) {
var enclosingElement = element.enclosingElement;
if (enclosingElement is ExtensionElement) {
if (_isNativeStructPointerExtension(enclosingElement)) {
if (_isNativeStructPointerExtension(enclosingElement) ||
_isNativeStructCArrayExtension(enclosingElement)) {
if (element.name == '[]') {
_validateRefIndexed(node);
}
Expand Down Expand Up @@ -242,6 +244,12 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
element.name == _allocatorExtensionName &&
element.library?.name == _dartFfiLibraryName;

/// Return `true` if the given [element] represents the class `CArray`.
bool _isCArray(Element? element) =>
element != null &&
element.name == _carrayClassName &&
element.library?.name == _dartFfiLibraryName;

/// Return `true` if the [typeName] is the name of a type from `dart:ffi`.
bool _isDartFfiClass(TypeName typeName) =>
_isDartFfiElement(typeName.name.staticElement);
Expand Down Expand Up @@ -274,6 +282,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
structFieldCount++;
} else if (_isStructClass(declaredType)) {
structFieldCount++;
} else if (_isCArray(declaredType.element)) {
structFieldCount++;
}
}
return structFieldCount == 0;
Expand Down Expand Up @@ -301,6 +311,9 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
element.name == 'NativeFunctionPointer' &&
element.library?.name == _dartFfiLibraryName;

bool _isNativeStructCArrayExtension(Element element) =>
element.name == 'StructCArray' && element.library?.name == 'dart.ffi';

bool _isNativeStructPointerExtension(Element element) =>
element.name == 'StructPointer' && element.library?.name == 'dart.ffi';

Expand Down Expand Up @@ -348,6 +361,29 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
return false;
}

/// Returns `true` if [nativeType] is a C type that has a size.
bool _isSized(DartType nativeType) {
switch (_primitiveNativeType(nativeType)) {
case _PrimitiveDartType.double:
return true;
case _PrimitiveDartType.int:
return true;
case _PrimitiveDartType.void_:
return false;
case _PrimitiveDartType.handle:
return false;
case _PrimitiveDartType.none:
break;
}
if (_isStructClass(nativeType)) {
return true;
}
if (_isPointer(nativeType.element)) {
return true;
}
return false;
}

/// Returns `true` iff [nativeType] is a struct type.
bool _isStructClass(DartType nativeType) {
if (nativeType is InterfaceType) {
Expand Down Expand Up @@ -389,12 +425,14 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
nativeType.optionalParameterTypes.isNotEmpty) {
return false;
}
if (!_isValidFfiNativeType(nativeType.returnType, true, false)) {
if (!_isValidFfiNativeType(nativeType.returnType,
allowVoid: true, allowEmptyStruct: false)) {
return false;
}

for (final DartType typeArg in nativeType.normalParameterTypes) {
if (!_isValidFfiNativeType(typeArg, false, false)) {
if (!_isValidFfiNativeType(typeArg,
allowVoid: false, allowEmptyStruct: false)) {
return false;
}
}
Expand All @@ -404,9 +442,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}

/// Validates that the given [nativeType] is a valid dart:ffi native type.
// TODO(https://dartbug.com/44747): Change to named arguments.
bool _isValidFfiNativeType(
DartType? nativeType, bool allowVoid, bool allowEmptyStruct) {
bool _isValidFfiNativeType(DartType? nativeType,
{bool allowVoid = false,
bool allowEmptyStruct = false,
bool allowCArray = false}) {
if (nativeType is InterfaceType) {
// Is it a primitive integer/double type (or ffi.Void if we allow it).
final primitiveType = _primitiveNativeType(nativeType);
Expand All @@ -419,7 +458,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
if (_isPointerInterfaceType(nativeType)) {
final nativeArgumentType = nativeType.typeArguments.single;
return _isValidFfiNativeType(nativeArgumentType, true, true) ||
return _isValidFfiNativeType(nativeArgumentType,
allowVoid: true, allowEmptyStruct: true) ||
_isStructClass(nativeArgumentType) ||
_isNativeTypeInterfaceType(nativeArgumentType);
}
Expand All @@ -436,6 +476,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (_isOpaqueClass(nativeType)) {
return true;
}
if (allowCArray && _isCArray(nativeType.element)) {
return _isValidFfiNativeType(nativeType.typeArguments.single,
allowVoid: false, allowEmptyStruct: false);
}
} else if (nativeType is FunctionType) {
return _isValidFfiNativeFunctionType(nativeType);
}
Expand Down Expand Up @@ -484,7 +528,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
return;
}
final DartType dartType = typeArgumentTypes[0];
if (!_isValidFfiNativeType(dartType, true, true)) {
if (!_isValidFfiNativeType(dartType,
allowVoid: true, allowEmptyStruct: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT,
Expand Down Expand Up @@ -647,7 +692,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
targetType.typeArguments.length == 1) {
final DartType T = targetType.typeArguments[0];

if (!_isValidFfiNativeType(T, true, true)) {
if (!_isValidFfiNativeType(T, allowVoid: true, allowEmptyStruct: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING,
Expand Down Expand Up @@ -677,6 +722,13 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
_validateAnnotations(fieldType, annotations, _PrimitiveDartType.double);
} else if (_isPointer(declaredType.element)) {
_validateNoAnnotations(annotations);
} else if (_isCArray(declaredType.element)) {
final typeArg = (declaredType as InterfaceType).typeArguments.single;
if (!_isSized(typeArg)) {
_errorReporter.reportErrorForNode(FfiCode.NON_SIZED_TYPE_ARGUMENT,
fieldType, [_carrayClassName, typeArg.toString()]);
}
_validateSizeOfAnnotation(fieldType, annotations);
} else if (_isStructClass(declaredType)) {
final clazz = (declaredType as InterfaceType).element;
if (_isEmptyStruct(clazz)) {
Expand Down Expand Up @@ -782,7 +834,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

void _validateRefIndexed(IndexExpression node) {
var targetType = node.realTarget.staticType;
if (!_isValidFfiNativeType(targetType, false, true)) {
if (!_isValidFfiNativeType(targetType,
allowVoid: false, allowEmptyStruct: true, allowCArray: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['[]']);
Expand All @@ -793,7 +846,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
/// `Pointer<T extends Struct>.ref`.
void _validateRefPrefixedIdentifier(PrefixedIdentifier node) {
var targetType = node.prefix.staticType!;
if (!_isValidFfiNativeType(targetType, false, true)) {
if (!_isValidFfiNativeType(targetType,
allowVoid: false, allowEmptyStruct: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
Expand All @@ -802,7 +856,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

void _validateRefPropertyAccess(PropertyAccess node) {
var targetType = node.realTarget.staticType;
if (!_isValidFfiNativeType(targetType, false, true)) {
if (!_isValidFfiNativeType(targetType,
allowVoid: false, allowEmptyStruct: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT, errorNode, ['ref']);
Expand All @@ -815,13 +870,41 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
return;
}
final DartType T = typeArgumentTypes[0];
if (!_isValidFfiNativeType(T, true, true)) {
if (!_isValidFfiNativeType(T, allowVoid: true, allowEmptyStruct: true)) {
final AstNode errorNode = node;
_errorReporter.reportErrorForNode(
FfiCode.NON_CONSTANT_TYPE_ARGUMENT_WARNING, errorNode, ['sizeOf']);
}
}

/// Validate that the [annotations] include exactly one size annotation. If
/// an error is produced that cannot be associated with an annotation,
/// associate it with the [errorNode].
void _validateSizeOfAnnotation(
AstNode errorNode, NodeList<Annotation> annotations) {
final ffiSizeAnnotations = annotations.where((annotation) {
if (!_isDartFfiElement(annotation.element)) {
return false;
}
if (annotation.element is! ConstructorElement) {
return false;
}
return annotation.element?.enclosingElement?.name == 'CArraySize';
}).toList();

if (ffiSizeAnnotations.isEmpty) {
_errorReporter.reportErrorForNode(
FfiCode.MISSING_SIZE_ANNOTATION_CARRAY, errorNode);
}
if (ffiSizeAnnotations.length > 1) {
final extraAnnotations = ffiSizeAnnotations.skip(1);
for (final annotation in extraAnnotations) {
_errorReporter.reportErrorForNode(
FfiCode.EXTRA_SIZE_ANNOTATION_CARRAY, annotation);
}
}
}

/// 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) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,14 @@ class DartRepresentationOf {
const DartRepresentationOf(String nativeType);
}
class CArray<T extends NativeType> extends NativeType {}
class CArraySize {
final int numberOfElements;
const CArraySize(this.numberOfElements);
}
extension StructPointer<T extends Struct> on Pointer<T> {
external T get ref;
Expand Down
42 changes: 42 additions & 0 deletions pkg/analyzer/test/src/diagnostics/extra_size_annotation_carray_test.dart
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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 'package:analyzer/src/dart/error/ffi_code.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

import '../dart/resolution/context_collection_resolution.dart';

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

@reflectiveTest
class ExtraSizeAnnotationCArray extends PubPackageResolutionTest {
test_one() async {
await assertNoErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
CArray<Uint8> a0;
}
''');
}

test_two() async {
await assertErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
@CArraySize(8)
@CArraySize(8)
CArray<Uint8> a0;
}
''', [
error(FfiCode.EXTRA_SIZE_ANNOTATION_CARRAY, 64, 14),
]);
}
}
Loading

0 comments on commit 5c4a916

Please sign in to comment.