Skip to content

Commit

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

Also, this CL fixes some tests to not use nullable fields in Structs.

Bug: #38491

Change-Id: Ia972f31475859aa57e012adf39b636919995b183
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194788
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
dcharkes authored and commit-bot@chromium.org committed Apr 12, 2021
1 parent ac2412c commit f162db3
Show file tree
Hide file tree
Showing 11 changed files with 259 additions and 84 deletions.
35 changes: 19 additions & 16 deletions pkg/analyzer/lib/src/dart/error/ffi_code.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class FfiCode extends AnalyzerErrorCode {
*/
static const FfiCode FIELD_IN_STRUCT_WITH_INITIALIZER = FfiCode(
name: 'FIELD_IN_STRUCT_WITH_INITIALIZER',
message: "Fields in subclasses of 'Struct' can't have initializers.",
message:
"Fields in subclasses of 'Struct' and 'Union' can't have initializers.",
correction:
"Try removing the initializer and marking the field as external.");

Expand All @@ -61,7 +62,8 @@ class FfiCode extends AnalyzerErrorCode {
*/
static const FfiCode FIELD_INITIALIZER_IN_STRUCT = FfiCode(
name: 'FIELD_INITIALIZER_IN_STRUCT',
message: "Constructors in subclasses of 'Struct' can't have field "
message:
"Constructors in subclasses of 'Struct' and 'Union' can't have field "
"initializers.",
correction: "Try removing the field initializer and marking the field as"
" external.");
Expand All @@ -72,7 +74,8 @@ class FfiCode extends AnalyzerErrorCode {
*/
static const FfiCode GENERIC_STRUCT_SUBCLASS = FfiCode(
name: 'GENERIC_STRUCT_SUBCLASS',
message: "The class '{0}' can't extend 'Struct' because it is generic.",
message:
"The class '{0}' can't extend 'Struct' or 'Union' because it is generic.",
correction: "Try removing the type parameters from '{0}'.");

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

/**
* No parameters.
Expand Down Expand Up @@ -202,9 +205,9 @@ class FfiCode extends AnalyzerErrorCode {
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'.",
"subtype of 'Struct' or 'Union'.",
correction: "Try using a native integer, 'Float', 'Double', 'Pointer', "
"or subtype of 'Struct'.");
"or subtype of 'Struct' or 'Union'.");

/**
* No parameters.
Expand Down Expand Up @@ -252,7 +255,7 @@ class FfiCode extends AnalyzerErrorCode {
static const FfiCode SUBTYPE_OF_FFI_CLASS_IN_EXTENDS = FfiCode(
name: 'SUBTYPE_OF_FFI_CLASS',
message: "The class '{0}' can't extend '{1}'.",
correction: "Try extending 'Struct'.",
correction: "Try extending 'Struct' or 'Union'.",
uniqueName: 'SUBTYPE_OF_FFI_CLASS_IN_EXTENDS',
);

Expand All @@ -264,7 +267,7 @@ class FfiCode extends AnalyzerErrorCode {
static const FfiCode SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS = FfiCode(
name: 'SUBTYPE_OF_FFI_CLASS',
message: "The class '{0}' can't implement '{1}'.",
correction: "Try extending 'Struct'.",
correction: "Try extending 'Struct' or 'Union'.",
uniqueName: 'SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS',
);

Expand All @@ -276,7 +279,7 @@ class FfiCode extends AnalyzerErrorCode {
static const FfiCode SUBTYPE_OF_FFI_CLASS_IN_WITH = FfiCode(
name: 'SUBTYPE_OF_FFI_CLASS',
message: "The class '{0}' can't mix in '{1}'.",
correction: "Try extending 'Struct'.",
correction: "Try extending 'Struct' or 'Union'.",
uniqueName: 'SUBTYPE_OF_FFI_CLASS_IN_WITH',
);

Expand All @@ -288,8 +291,8 @@ class FfiCode extends AnalyzerErrorCode {
static const FfiCode SUBTYPE_OF_STRUCT_CLASS_IN_EXTENDS = FfiCode(
name: 'SUBTYPE_OF_STRUCT_CLASS',
message: "The class '{0}' can't extend '{1}' because '{1}' is a subtype of "
"'Struct'.",
correction: "Try extending 'Struct' directly.",
"'Struct' or 'Union'.",
correction: "Try extending 'Struct' or 'Union' directly.",
uniqueName: 'SUBTYPE_OF_STRUCT_CLASS_IN_EXTENDS',
);

Expand All @@ -302,8 +305,8 @@ class FfiCode extends AnalyzerErrorCode {
name: 'SUBTYPE_OF_STRUCT_CLASS',
message:
"The class '{0}' can't implement '{1}' because '{1}' is a subtype of "
"'Struct'.",
correction: "Try extending 'Struct' directly.",
"'Struct' or 'Union'.",
correction: "Try extending 'Struct' or 'Union' directly.",
uniqueName: 'SUBTYPE_OF_STRUCT_CLASS_IN_IMPLEMENTS',
);

Expand All @@ -315,8 +318,8 @@ class FfiCode extends AnalyzerErrorCode {
static const FfiCode SUBTYPE_OF_STRUCT_CLASS_IN_WITH = FfiCode(
name: 'SUBTYPE_OF_STRUCT_CLASS',
message: "The class '{0}' can't mix in '{1}' because '{1}' is a subtype of "
"'Struct'.",
correction: "Try extending 'Struct' directly.",
"'Struct' or 'Union'.",
correction: "Try extending 'Struct' or 'Union' directly.",
uniqueName: 'SUBTYPE_OF_STRUCT_CLASS_IN_WITH',
);

Expand Down
81 changes: 44 additions & 37 deletions pkg/analyzer/lib/src/generated/ffi_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

static const _structClassName = 'Struct';

static const _unionClassName = 'Union';

/// The type system used to check types.
final TypeSystemImpl typeSystem;

Expand All @@ -49,41 +51,43 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

/// A flag indicating whether we are currently visiting inside a subclass of
/// `Struct`.
bool inStruct = false;
bool inCompound = false;

/// Subclass of `Struct` we are currently visiting, or `null`.
ClassDeclaration? struct;
/// Subclass of `Struct` or `Union` we are currently visiting, or `null`.
ClassDeclaration? compound;

/// Initialize a newly created verifier.
FfiVerifier(this.typeSystem, this._errorReporter);

@override
void visitClassDeclaration(ClassDeclaration node) {
inStruct = false;
struct = null;
inCompound = false;
compound = null;
// Only the Allocator, Opaque and Struct class may be extended.
var extendsClause = node.extendsClause;
if (extendsClause != null) {
final TypeName superclass = extendsClause.superclass;
final ffiClass = superclass.ffiClass;
if (ffiClass != null) {
final className = ffiClass.name;
if (className == _structClassName) {
inStruct = true;
struct = node;
if (className == _structClassName || className == _unionClassName) {
inCompound = true;
compound = node;
if (node.declaredElement!.isEmptyStruct) {
_errorReporter
.reportErrorForNode(FfiCode.EMPTY_STRUCT, node, [node.name]);
}
_validatePackedAnnotation(node.metadata);
if (className == _structClassName) {
_validatePackedAnnotation(node.metadata);
}
} else if (className != _allocatorClassName &&
className != _opaqueClassName) {
_errorReporter.reportErrorForNode(
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS,
superclass.name,
[node.name.name, superclass.name.name]);
}
} else if (superclass.isStructSubtype) {
} else if (superclass.isCompoundSubtype) {
_errorReporter.reportErrorForNode(
FfiCode.SUBTYPE_OF_STRUCT_CLASS_IN_EXTENDS,
superclass,
Expand All @@ -101,7 +105,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if (typename.ffiClass != null) {
_errorReporter.reportErrorForNode(
subtypeOfFfiCode, typename, [node.name, typename.name]);
} else if (typename.isStructSubtype) {
} else if (typename.isCompoundSubtype) {
_errorReporter.reportErrorForNode(
subtypeOfStructCode, typename, [node.name, typename.name]);
}
Expand All @@ -122,7 +126,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}
}

if (inStruct && node.declaredElement!.typeParameters.isNotEmpty) {
if (inCompound && node.declaredElement!.typeParameters.isNotEmpty) {
_errorReporter.reportErrorForNode(
FfiCode.GENERIC_STRUCT_SUBCLASS, node.name, [node.name]);
}
Expand All @@ -131,7 +135,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

@override
void visitConstructorFieldInitializer(ConstructorFieldInitializer node) {
if (inStruct) {
if (inCompound) {
_errorReporter.reportErrorForNode(
FfiCode.FIELD_INITIALIZER_IN_STRUCT, node);
}
Expand All @@ -140,8 +144,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {

@override
void visitFieldDeclaration(FieldDeclaration node) {
if (inStruct) {
_validateFieldsInStruct(node);
if (inCompound) {
_validateFieldsInCompound(node);
}
super.visitFieldDeclaration(node);
}
Expand Down Expand Up @@ -248,7 +252,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
case _PrimitiveDartType.none:
break;
}
if (nativeType.isStructSubtype) {
if (nativeType.isCompoundSubtype) {
return true;
}
if (nativeType.isPointer) {
Expand Down Expand Up @@ -311,10 +315,10 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
final nativeArgumentType = nativeType.typeArguments.single;
return _isValidFfiNativeType(nativeArgumentType,
allowVoid: true, allowEmptyStruct: true, allowHandle: true) ||
nativeArgumentType.isStructSubtype ||
nativeArgumentType.isCompoundSubtype ||
nativeArgumentType.isNativeType;
}
if (nativeType.isStructSubtype) {
if (nativeType.isCompoundSubtype) {
if (!allowEmptyStruct) {
if (nativeType.element.isEmptyStruct) {
// TODO(dartbug.com/36780): This results in an error message not
Expand Down Expand Up @@ -548,8 +552,8 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
}

/// Validate that the fields declared by the given [node] meet the
/// requirements for fields within a struct class.
void _validateFieldsInStruct(FieldDeclaration node) {
/// requirements for fields within a struct or union class.
void _validateFieldsInCompound(FieldDeclaration node) {
if (node.isStatic) {
return;
}
Expand All @@ -576,18 +580,18 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
final arrayDimensions = declaredType.arrayDimensions;
_validateSizeOfAnnotation(fieldType, annotations, arrayDimensions);
final arrayElement = declaredType.arrayElementType;
if (arrayElement.isStructSubtype) {
if (arrayElement.isCompoundSubtype) {
final elementClass = (arrayElement as InterfaceType).element;
_validatePackingNesting(struct!.declaredElement!, elementClass,
_validatePackingNesting(compound!.declaredElement!, elementClass,
errorNode: fieldType);
}
} else if (declaredType.isStructSubtype) {
} else if (declaredType.isCompoundSubtype) {
final clazz = (declaredType as InterfaceType).element;
if (clazz.isEmptyStruct) {
_errorReporter
.reportErrorForNode(FfiCode.EMPTY_STRUCT, node, [clazz.name]);
}
_validatePackingNesting(struct!.declaredElement!, clazz,
_validatePackingNesting(compound!.declaredElement!, clazz,
errorNode: fieldType);
} else {
_errorReporter.reportErrorForNode(FfiCode.INVALID_FIELD_TYPE_IN_STRUCT,
Expand Down Expand Up @@ -632,7 +636,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
if ((FT as FunctionType).returnType.isVoid ||
R.isPointer ||
R.isHandle ||
R.isStructSubtype) {
R.isCompoundSubtype) {
if (argCount != 1) {
_errorReporter.reportErrorForNode(
FfiCode.INVALID_EXCEPTION_VALUE, node.argumentList.arguments[1]);
Expand Down Expand Up @@ -975,7 +979,7 @@ extension on ClassElement {
return false;
} else if (declaredType.isPointer) {
return false;
} else if (declaredType.isStructSubtype) {
} else if (declaredType.isCompoundSubtype) {
return false;
} else if (declaredType.isArray) {
return false;
Expand Down Expand Up @@ -1087,22 +1091,25 @@ extension on DartType {
return false;
}

bool get isStruct {
bool get isCompound {
final self = this;
if (self is InterfaceType) {
final element = self.element;
return element.name == FfiVerifier._structClassName && element.isFfiClass;
final name = element.name;
return (name == FfiVerifier._structClassName ||
name == FfiVerifier._unionClassName) &&
element.isFfiClass;
}
return false;
}

/// Returns `true` if this is a struct type, i.e. a subtype of `Struct`.
bool get isStructSubtype {
bool get isCompoundSubtype {
final self = this;
if (self is InterfaceType) {
final superType = self.element.supertype;
if (superType != null) {
return superType.isStruct;
return superType.isCompound;
}
}
return false;
Expand All @@ -1115,17 +1122,17 @@ extension on TypeName {
return name.staticElement.ffiClass;
}

/// Return `true` if this represents a subtype of `Struct`.
bool get isStructSubtype {
/// Return `true` if this represents a subtype of `Struct` or `Union`.
bool get isCompoundSubtype {
var element = name.staticElement;
if (element is ClassElement) {
bool isStruct(InterfaceType? type) {
return type != null && type.isStruct;
bool isCompound(InterfaceType? type) {
return type != null && type.isCompound;
}

return isStruct(element.supertype) ||
element.interfaces.any(isStruct) ||
element.mixins.any(isStruct);
return isCompound(element.supertype) ||
element.interfaces.any(isCompound) ||
element.mixins.any(isCompound);
}
return false;
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/analyzer/lib/src/test_utilities/mock_sdk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,18 @@ class Pointer<T extends NativeType> extends NativeType {
[Object exceptionalReturn]) {}
}
final Pointer<Never> nullptr = Pointer.fromAddress(0);
extension NativeFunctionPointer<NF extends Function>
on Pointer<NativeFunction<NF>> {
external DF asFunction<DF extends Function>();
}
class Struct extends NativeType {}
class _Compound extends NativeType {}
class Struct extends _Compound {}
class Union extends _Compound {}
class Packed {
final int memberAlignment;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,21 @@ class FieldInStructWithInitializerTest extends PubPackageResolutionTest {
await assertErrorsInCode(r'''
import 'dart:ffi';
class C extends Struct {
Pointer? p = null;
Pointer p = nullptr;
}
''', [
error(FfiCode.FIELD_IN_STRUCT_WITH_INITIALIZER, 55, 1),
error(FfiCode.FIELD_IN_STRUCT_WITH_INITIALIZER, 54, 1),
]);
}

test_instance_withInitializer2() async {
await assertErrorsInCode(r'''
import 'dart:ffi';
class C extends Union {
Pointer p = nullptr;
}
''', [
error(FfiCode.FIELD_IN_STRUCT_WITH_INITIALIZER, 53, 1),
]);
}

Expand Down
Loading

0 comments on commit f162db3

Please sign in to comment.