diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart index fd9f76736eb6..9395186ea7a3 100644 --- a/pkg/analyzer/lib/error/error.dart +++ b/pkg/analyzer/lib/error/error.dart @@ -476,6 +476,9 @@ const List errorCodeValues = [ FfiCode.NON_CONSTANT_TYPE_ARGUMENT, FfiCode.NON_NATIVE_FUNCTION_TYPE_ARGUMENT_TO_POINTER, FfiCode.NON_SIZED_TYPE_ARGUMENT, + FfiCode.PACKED_ANNOTATION, + FfiCode.PACKED_ANNOTATION_ALIGNMENT, + FfiCode.PACKED_NESTING_NON_PACKED, FfiCode.SIZE_ANNOTATION_DIMENSIONS, FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS, FfiCode.SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS, diff --git a/pkg/analyzer/lib/src/dart/error/ffi_code.dart b/pkg/analyzer/lib/src/dart/error/ffi_code.dart index 08f603fc42bd..dc8e1062ff65 100644 --- a/pkg/analyzer/lib/src/dart/error/ffi_code.dart +++ b/pkg/analyzer/lib/src/dart/error/ffi_code.dart @@ -206,6 +206,35 @@ class FfiCode extends AnalyzerErrorCode { correction: "Try using a native integer, 'Float', 'Double', 'Pointer', " "or subtype of 'Struct'."); + /** + * No parameters. + */ + static const FfiCode PACKED_ANNOTATION = FfiCode( + name: 'PACKED_ANNOTATION', + message: "Structs must have at most one 'Packed' annotation.", + correction: "Try removing extra 'Packed' annotations."); + + /** + * No parameters. + */ + static const FfiCode PACKED_ANNOTATION_ALIGNMENT = FfiCode( + name: 'PACKED_ANNOTATION_ALIGNMENT', + message: "Only packing to 1, 2, 4, 8, and 16 bytes is supported.", + correction: + "Try changing the 'Packed' annotation alignment to 1, 2, 4, 8, or 16."); + + /** + * Parameters: + * 0: the name of the outer struct + * 1: the name of the struct being nested + */ + static const FfiCode PACKED_NESTING_NON_PACKED = FfiCode( + name: 'PACKED_NESTING_NON_PACKED', + message: + "Nesting the non-packed or less tightly packed struct '{0}' in a packed struct '{1}' is not supported.", + correction: + "Try packing the nested struct or packing the nested struct more tightly."); + /** * No parameters. */ diff --git a/pkg/analyzer/lib/src/generated/ffi_verifier.dart b/pkg/analyzer/lib/src/generated/ffi_verifier.dart index d65781b353ad..bd826d49cf71 100644 --- a/pkg/analyzer/lib/src/generated/ffi_verifier.dart +++ b/pkg/analyzer/lib/src/generated/ffi_verifier.dart @@ -51,12 +51,16 @@ class FfiVerifier extends RecursiveAstVisitor { /// `Struct`. bool inStruct = false; + /// Subclass of `Struct` we are currently visiting, or `null`. + ClassDeclaration? struct; + /// Initialize a newly created verifier. FfiVerifier(this.typeSystem, this._errorReporter); @override void visitClassDeclaration(ClassDeclaration node) { inStruct = false; + struct = null; // Only the Allocator, Opaque and Struct class may be extended. var extendsClause = node.extendsClause; if (extendsClause != null) { @@ -66,10 +70,12 @@ class FfiVerifier extends RecursiveAstVisitor { final className = ffiClass.name; if (className == _structClassName) { inStruct = true; + struct = node; if (node.declaredElement!.isEmptyStruct) { _errorReporter .reportErrorForNode(FfiCode.EMPTY_STRUCT, node, [node.name]); } + _validatePackedAnnotation(node.metadata); } else if (className != _allocatorClassName && className != _opaqueClassName) { _errorReporter.reportErrorForNode( @@ -561,12 +567,20 @@ class FfiVerifier extends RecursiveAstVisitor { } final arrayDimensions = declaredType.arrayDimensions; _validateSizeOfAnnotation(fieldType, annotations, arrayDimensions); + final arrayElement = declaredType.arrayElementType; + if (arrayElement.isStructSubtype) { + final elementClass = (arrayElement as InterfaceType).element; + _validatePackingNesting(struct!.declaredElement!, elementClass, + errorNode: fieldType); + } } else if (declaredType.isStructSubtype) { final clazz = (declaredType as InterfaceType).element; if (clazz.isEmptyStruct) { _errorReporter .reportErrorForNode(FfiCode.EMPTY_STRUCT, node, [clazz.name]); } + _validatePackingNesting(struct!.declaredElement!, clazz, + errorNode: fieldType); } else { _errorReporter.reportErrorForNode(FfiCode.INVALID_FIELD_TYPE_IN_STRUCT, fieldType, [fieldType.toSource()]); @@ -664,6 +678,54 @@ class FfiVerifier extends RecursiveAstVisitor { } } + /// Validate that the [annotations] include at most one packed annotation. + void _validatePackedAnnotation(NodeList annotations) { + final ffiPackedAnnotations = + annotations.where((annotation) => annotation.isPacked).toList(); + + if (ffiPackedAnnotations.isEmpty) { + return; + } + + if (ffiPackedAnnotations.length > 1) { + final extraAnnotations = ffiPackedAnnotations.skip(1); + for (final annotation in extraAnnotations) { + _errorReporter.reportErrorForNode( + FfiCode.PACKED_ANNOTATION, annotation); + } + } + + // Check number of dimensions. + final annotation = ffiPackedAnnotations.first; + final value = annotation.elementAnnotation?.packedMemberAlignment; + if (![1, 2, 4, 8, 16].contains(value)) { + _errorReporter.reportErrorForNode( + FfiCode.PACKED_ANNOTATION_ALIGNMENT, annotation); + } + } + + void _validatePackingNesting(ClassElement outer, ClassElement nested, + {required TypeAnnotation errorNode}) { + final outerPacking = outer.structPacking; + if (outerPacking == null) { + // No packing for outer class, so we're done. + return; + } + bool error = false; + final nestedPacking = nested.structPacking; + if (nestedPacking == null) { + // The outer struct packs, but the nested struct does not. + error = true; + } else if (outerPacking < nestedPacking) { + // The outer struct packs tighter than the nested struct. + error = true; + } + if (error) { + _errorReporter.reportErrorForNode(FfiCode.PACKED_NESTING_NON_PACKED, + errorNode, [nested.name, outer.name]); + } + } + void _validateRefIndexed(IndexExpression node) { var targetType = node.realTarget.staticType; if (!_isValidFfiNativeType(targetType, @@ -774,6 +836,30 @@ enum _PrimitiveDartType { none, } +extension on Annotation { + bool get isPacked { + final element = this.element; + return element is ConstructorElement && + element.ffiClass != null && + element.enclosingElement.name == 'Packed'; + } +} + +extension on ElementAnnotation { + bool get isPacked { + final element = this.element; + return element is ConstructorElement && + element.ffiClass != null && + element.enclosingElement.name == 'Packed'; + } + + int get packedMemberAlignment { + assert(isPacked); + final value = computeConstantValue(); + return value!.getField('memberAlignment')!.toIntValue()!; + } +} + extension on Element? { /// Return `true` if this represents the extension `AllocatorAlloc`. bool get isAllocatorExtension { @@ -855,6 +941,17 @@ extension on ClassElement { bool get isFfiClass { return library.name == FfiVerifier._dartFfiLibraryName; } + + int? get structPacking { + final packedAnnotations = + metadata.where((annotation) => annotation.isPacked); + + if (packedAnnotations.isEmpty) { + return null; + } + + return packedAnnotations.first.packedMemberAlignment; + } } extension on ExtensionElement { @@ -886,6 +983,16 @@ extension on DartType { return dimensions; } + DartType get arrayElementType { + DartType iterator = this; + while (iterator is InterfaceType && + iterator.element.name == FfiVerifier._arrayClassName && + iterator.element.isFfiClass) { + iterator = iterator.typeArguments.single; + } + return iterator; + } + bool get isPointer { final self = this; return self is InterfaceType && self.element.isPointer; diff --git a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart index f43dcc291856..a0ea9391986f 100644 --- a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart +++ b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart @@ -677,6 +677,12 @@ extension NativeFunctionPointer class Struct extends NativeType {} +class Packed { + final int memberAlignment; + + const Packed(this.memberAlignment); +} + abstract class DynamicLibrary {} extension DynamicLibraryExtension on DynamicLibrary { diff --git a/pkg/analyzer/test/src/diagnostics/packed_annotation_alignment_test.dart b/pkg/analyzer/test/src/diagnostics/packed_annotation_alignment_test.dart new file mode 100644 index 000000000000..b5e922307b63 --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/packed_annotation_alignment_test.dart @@ -0,0 +1,41 @@ +// 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(PackedAnnotationAlignment); + }); +} + +@reflectiveTest +class PackedAnnotationAlignment extends PubPackageResolutionTest { + test_error() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(3) +class C extends Struct { + external Pointer notEmpty; +} +''', [ + error(FfiCode.PACKED_ANNOTATION_ALIGNMENT, 20, 10), + ]); + } + + test_no_error() async { + await assertNoErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(1) +class C extends Struct { + external Pointer notEmpty; +} +'''); + } +} diff --git a/pkg/analyzer/test/src/diagnostics/packed_annotation_test.dart b/pkg/analyzer/test/src/diagnostics/packed_annotation_test.dart new file mode 100644 index 000000000000..5ebf7017d53c --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/packed_annotation_test.dart @@ -0,0 +1,52 @@ +// 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(PackedAnnotation); + }); +} + +@reflectiveTest +class PackedAnnotation extends PubPackageResolutionTest { + test_error() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(1) +@Packed(1) +class C extends Struct { + external Pointer notEmpty; +} +''', [ + error(FfiCode.PACKED_ANNOTATION, 31, 10), + ]); + } + + test_no_error_1() async { + await assertNoErrorsInCode(r''' +import 'dart:ffi'; + +class C extends Struct { + external Pointer notEmpty; +} +'''); + } + + test_no_error_2() async { + await assertNoErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(1) +class C extends Struct { + external Pointer notEmpty; +} +'''); + } +} diff --git a/pkg/analyzer/test/src/diagnostics/packed_nesting_non_packed_test.dart b/pkg/analyzer/test/src/diagnostics/packed_nesting_non_packed_test.dart new file mode 100644 index 000000000000..97511391c404 --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/packed_nesting_non_packed_test.dart @@ -0,0 +1,116 @@ +// 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(PackedAnnotationNestingNonPacked); + }); +} + +@reflectiveTest +class PackedAnnotationNestingNonPacked extends PubPackageResolutionTest { + test_error_1() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +class TestStruct1603 extends Struct { + external Pointer notEmpty; +} + +@Packed(1) +class TestStruct1603Packed extends Struct { + external Pointer notEmpty; + + external TestStruct1603 nestedNotPacked; +} +''', [ + error(FfiCode.PACKED_NESTING_NON_PACKED, 200, 14), + ]); + } + + test_error_2() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(8) +class TestStruct1604 extends Struct { + external Pointer notEmpty; +} + +@Packed(1) +class TestStruct1604Packed extends Struct { + external Pointer notEmpty; + + external TestStruct1604 nestedLooselyPacked; +} +''', [ + error(FfiCode.PACKED_NESTING_NON_PACKED, 211, 14), + ]); + } + + test_error_3() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +class TestStruct1603 extends Struct { + external Pointer notEmpty; +} + +@Packed(1) +class TestStruct1605Packed extends Struct { + external Pointer notEmpty; + + @Array(2) + external Array nestedNotPacked; +} +''', [ + error(FfiCode.PACKED_NESTING_NON_PACKED, 212, 21), + ]); + } + + test_error_4() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(8) +class TestStruct1604 extends Struct { + external Pointer notEmpty; +} + +@Packed(1) +class TestStruct1606Packed extends Struct { + external Pointer notEmpty; + + @Array(2) + external Array nestedLooselyPacked; +} +''', [ + error(FfiCode.PACKED_NESTING_NON_PACKED, 223, 21), + ]); + } + + test_no_error() async { + await assertNoErrorsInCode(r''' +import 'dart:ffi'; + +@Packed(1) +class TestStruct1604 extends Struct { + external Pointer notEmpty; +} + +@Packed(1) +class TestStruct1606Packed extends Struct { + external Pointer notEmpty; + + @Array(2) + external Array nestedLooselyPacked; +} +'''); + } +} diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart index 3916b7520c59..9d666bb146a8 100644 --- a/pkg/analyzer/test/src/diagnostics/test_all.dart +++ b/pkg/analyzer/test/src/diagnostics/test_all.dart @@ -498,6 +498,9 @@ import 'override_on_non_overriding_method_test.dart' as override_on_non_overriding_method; import 'override_on_non_overriding_setter_test.dart' as override_on_non_overriding_setter; +import 'packed_annotation_alignment_test.dart' as packed_annotation_alignment; +import 'packed_annotation_test.dart' as packed_annotation; +import 'packed_nesting_non_packed_test.dart' as packed_nesting_non_packed; import 'part_of_different_library_test.dart' as part_of_different_library; import 'part_of_non_part_test.dart' as part_of_non_part; import 'prefix_collides_with_top_level_member_test.dart' @@ -1000,6 +1003,9 @@ main() { override_on_non_overriding_getter.main(); override_on_non_overriding_method.main(); override_on_non_overriding_setter.main(); + packed_annotation.main(); + packed_annotation_alignment.main(); + packed_nesting_non_packed.main(); part_of_different_library.main(); part_of_non_part.main(); prefix_collides_with_top_level_member.main();