Skip to content

Commit

Permalink
[analyzer/ffi] Support packed Structss
Browse files Browse the repository at this point in the history
This CL does not add packed structs 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.

Bug: #38158

Split off https://dart-review.googlesource.com/c/sdk/+/186143.

Change-Id: I9c9ac9154e3dec0e05242f57cf7dddf8406df5fb
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/+/191700
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
  • Loading branch information
dcharkes authored and commit-bot@chromium.org committed Mar 17, 2021
1 parent bed0589 commit 3c55d8b
Show file tree
Hide file tree
Showing 8 changed files with 360 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/analyzer/lib/error/error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,9 @@ const List<ErrorCode> 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,
Expand Down
29 changes: 29 additions & 0 deletions pkg/analyzer/lib/src/dart/error/ffi_code.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
107 changes: 107 additions & 0 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,16 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
/// `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) {
Expand All @@ -66,10 +70,12 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
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(
Expand Down Expand Up @@ -561,12 +567,20 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
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()]);
Expand Down Expand Up @@ -664,6 +678,54 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
}

/// Validate that the [annotations] include at most one packed annotation.
void _validatePackedAnnotation(NodeList<Annotation> 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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,12 @@ extension NativeFunctionPointer<NF extends Function>
class Struct extends NativeType {}
class Packed {
final int memberAlignment;
const Packed(this.memberAlignment);
}
abstract class DynamicLibrary {}
extension DynamicLibraryExtension on DynamicLibrary {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Uint8> 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<Uint8> notEmpty;
}
''');
}
}
52 changes: 52 additions & 0 deletions pkg/analyzer/test/src/diagnostics/packed_annotation_test.dart
Original file line number Diff line number Diff line change
@@ -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<Uint8> notEmpty;
}
''', [
error(FfiCode.PACKED_ANNOTATION, 31, 10),
]);
}

test_no_error_1() async {
await assertNoErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
external Pointer<Uint8> notEmpty;
}
''');
}

test_no_error_2() async {
await assertNoErrorsInCode(r'''
import 'dart:ffi';
@Packed(1)
class C extends Struct {
external Pointer<Uint8> notEmpty;
}
''');
}
}
Loading

0 comments on commit 3c55d8b

Please sign in to comment.