Skip to content

Commit a855030

Browse files
dcharkesCommit Bot
authored and
Commit Bot
committed
[vm/ffi] Allow struct nesting with looser packing
C compilers don't enforce nesting rules. `dart:ffi` was enforcing these rules causing issues for binding to C libraries having looser packing for nested structs than outer structs. This CL completely removes the error from the analyzer and CFE. (As an alternative we could have kept a hint/lint, but this might create more noise than value.) TEST=pkg/analyzer/test/src/diagnostics/packed_nesting_non_packed_test.dart Closes: #46644 Change-Id: Iae2d5c885546f7799bc6dea2f8cd7dd508216a0c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247382 Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Aske Simon Christensen <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent e11d6b4 commit a855030

File tree

15 files changed

+2
-508
lines changed

15 files changed

+2
-508
lines changed

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4633,36 +4633,6 @@ const MessageCode messageFfiPackedAnnotationAlignment = const MessageCode(
46334633
problemMessage:
46344634
r"""Only packing to 1, 2, 4, 8, and 16 bytes is supported.""");
46354635

4636-
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
4637-
const Template<
4638-
Message Function(
4639-
String name,
4640-
String
4641-
name2)> templateFfiPackedNestingNonPacked = const Template<
4642-
Message Function(String name, String name2)>(
4643-
problemMessageTemplate:
4644-
r"""Nesting the non-packed or less tightly packed struct '#name' in a packed struct '#name2' is not supported.""",
4645-
withArguments: _withArgumentsFfiPackedNestingNonPacked);
4646-
4647-
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
4648-
const Code<Message Function(String name, String name2)>
4649-
codeFfiPackedNestingNonPacked =
4650-
const Code<Message Function(String name, String name2)>(
4651-
"FfiPackedNestingNonPacked",
4652-
);
4653-
4654-
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
4655-
Message _withArgumentsFfiPackedNestingNonPacked(String name, String name2) {
4656-
if (name.isEmpty) throw 'No name provided';
4657-
name = demangleMixinApplicationName(name);
4658-
if (name2.isEmpty) throw 'No name provided';
4659-
name2 = demangleMixinApplicationName(name2);
4660-
return new Message(codeFfiPackedNestingNonPacked,
4661-
problemMessage:
4662-
"""Nesting the non-packed or less tightly packed struct '${name}' in a packed struct '${name2}' is not supported.""",
4663-
arguments: {'name': name, 'name2': name2});
4664-
}
4665-
46664636
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
46674637
const Template<Message Function(String name)> templateFfiSizeAnnotation =
46684638
const Template<Message Function(String name)>(

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,8 +1193,6 @@ FfiCode.PACKED_ANNOTATION:
11931193
status: needsEvaluation
11941194
FfiCode.PACKED_ANNOTATION_ALIGNMENT:
11951195
status: needsEvaluation
1196-
FfiCode.PACKED_NESTING_NON_PACKED:
1197-
status: needsEvaluation
11981196
FfiCode.SIZE_ANNOTATION_DIMENSIONS:
11991197
status: needsEvaluation
12001198
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS:

pkg/analyzer/lib/error/error.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,6 @@ const List<ErrorCode> errorCodeValues = [
539539
FfiCode.NON_SIZED_TYPE_ARGUMENT,
540540
FfiCode.PACKED_ANNOTATION,
541541
FfiCode.PACKED_ANNOTATION_ALIGNMENT,
542-
FfiCode.PACKED_NESTING_NON_PACKED,
543542
FfiCode.SIZE_ANNOTATION_DIMENSIONS,
544543
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_EXTENDS,
545544
FfiCode.SUBTYPE_OF_FFI_CLASS_IN_IMPLEMENTS,

pkg/analyzer/lib/src/dart/error/ffi_code.g.dart

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -369,19 +369,6 @@ class FfiCode extends AnalyzerErrorCode {
369369
hasPublishedDocs: true,
370370
);
371371

372-
/// Parameters:
373-
/// 0: the name of the outer struct
374-
/// 1: the name of the struct being nested
375-
static const FfiCode PACKED_NESTING_NON_PACKED = FfiCode(
376-
'PACKED_NESTING_NON_PACKED',
377-
"Nesting the non-packed or less tightly packed struct '{0}' in a packed "
378-
"struct '{1}' isn't supported.",
379-
correctionMessage:
380-
"Try packing the nested struct or packing the nested struct more "
381-
"tightly.",
382-
hasPublishedDocs: true,
383-
);
384-
385372
/// No parameters.
386373
static const FfiCode SIZE_ANNOTATION_DIMENSIONS = FfiCode(
387374
'SIZE_ANNOTATION_DIMENSIONS',

pkg/analyzer/lib/src/generated/ffi_verifier.dart

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -943,12 +943,6 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
943943
}
944944
final arrayDimensions = declaredType.arrayDimensions;
945945
_validateSizeOfAnnotation(fieldType, annotations, arrayDimensions);
946-
final arrayElement = declaredType.arrayElementType;
947-
if (arrayElement.isCompoundSubtype) {
948-
final elementClass = (arrayElement as InterfaceType).element;
949-
_validatePackingNesting(compound!.declaredElement!, elementClass,
950-
errorNode: fieldType);
951-
}
952946
} else if (declaredType.isCompoundSubtype) {
953947
final clazz = (declaredType as InterfaceType).element;
954948
if (clazz.isEmptyStruct) {
@@ -957,8 +951,6 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
957951
clazz.supertype!.getDisplayString(withNullability: false)
958952
]);
959953
}
960-
_validatePackingNesting(compound!.declaredElement!, clazz,
961-
errorNode: fieldType);
962954
} else {
963955
_errorReporter.reportErrorForNode(FfiCode.INVALID_FIELD_TYPE_IN_STRUCT,
964956
fieldType, [fieldType.toSource()]);
@@ -1124,28 +1116,6 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
11241116
}
11251117
}
11261118

1127-
void _validatePackingNesting(ClassElement outer, ClassElement nested,
1128-
{required TypeAnnotation errorNode}) {
1129-
final outerPacking = outer.structPacking;
1130-
if (outerPacking == null) {
1131-
// No packing for outer class, so we're done.
1132-
return;
1133-
}
1134-
bool error = false;
1135-
final nestedPacking = nested.structPacking;
1136-
if (nestedPacking == null) {
1137-
// The outer struct packs, but the nested struct does not.
1138-
error = true;
1139-
} else if (outerPacking < nestedPacking) {
1140-
// The outer struct packs tighter than the nested struct.
1141-
error = true;
1142-
}
1143-
if (error) {
1144-
_errorReporter.reportErrorForNode(FfiCode.PACKED_NESTING_NON_PACKED,
1145-
errorNode, [nested.name, outer.name]);
1146-
}
1147-
}
1148-
11491119
void _validateRefIndexed(IndexExpression node) {
11501120
var targetType = node.realTarget.staticType;
11511121
if (!_isValidFfiNativeType(targetType,
@@ -1484,17 +1454,6 @@ extension on ClassElement {
14841454
bool get isFfiClass {
14851455
return library.name == FfiVerifier._dartFfiLibraryName;
14861456
}
1487-
1488-
int? get structPacking {
1489-
final packedAnnotations =
1490-
metadata.where((annotation) => annotation.isPacked);
1491-
1492-
if (packedAnnotations.isEmpty) {
1493-
return null;
1494-
}
1495-
1496-
return packedAnnotations.first.packedMemberAlignment;
1497-
}
14981457
}
14991458

15001459
extension on ExtensionElement {
@@ -1533,16 +1492,6 @@ extension on DartType {
15331492
return dimensions;
15341493
}
15351494

1536-
DartType get arrayElementType {
1537-
DartType iterator = this;
1538-
while (iterator is InterfaceType &&
1539-
iterator.element.name == FfiVerifier._arrayClassName &&
1540-
iterator.element.isFfiClass) {
1541-
iterator = iterator.typeArguments.single;
1542-
}
1543-
return iterator;
1544-
}
1545-
15461495
bool get isAbiSpecificInteger {
15471496
final self = this;
15481497
if (self is InterfaceType) {

pkg/analyzer/messages.yaml

Lines changed: 0 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -17172,91 +17172,6 @@ FfiCode:
1717217172
external Pointer<Uint8> notEmpty;
1717317173
}
1717417174
```
17175-
PACKED_NESTING_NON_PACKED:
17176-
problemMessage: "Nesting the non-packed or less tightly packed struct '{0}' in a packed struct '{1}' isn't supported."
17177-
correctionMessage: Try packing the nested struct or packing the nested struct more tightly.
17178-
comment: |-
17179-
Parameters:
17180-
0: the name of the outer struct
17181-
1: the name of the struct being nested
17182-
hasPublishedDocs: true
17183-
documentation: |-
17184-
#### Description
17185-
17186-
The analyzer produces this diagnostic when a subclass of `Struct` that is
17187-
annotated as being `Packed` declares a field whose type is also a subclass
17188-
of `Struct` and the field's type is either not packed or is packed less
17189-
tightly.
17190-
17191-
For more information about FFI, see [C interop using dart:ffi][ffi].
17192-
17193-
#### Example
17194-
17195-
The following code produces this diagnostic because the class `Outer`,
17196-
which is a subclass of `Struct` and is packed on 1-byte boundaries,
17197-
declared a field whose type (`Inner`) is packed on 8-byte boundaries:
17198-
17199-
```dart
17200-
import 'dart:ffi';
17201-
17202-
@Packed(8)
17203-
class Inner extends Struct {
17204-
external Pointer<Uint8> notEmpty;
17205-
}
17206-
17207-
@Packed(1)
17208-
class Outer extends Struct {
17209-
external Pointer<Uint8> notEmpty;
17210-
17211-
external [!Inner!] nestedLooselyPacked;
17212-
}
17213-
```
17214-
17215-
#### Common fixes
17216-
17217-
If the inner struct should be packed more tightly, then change the
17218-
argument to the inner struct's `Packed` annotation:
17219-
17220-
```dart
17221-
import 'dart:ffi';
17222-
17223-
@Packed(1)
17224-
class Inner extends Struct {
17225-
external Pointer<Uint8> notEmpty;
17226-
}
17227-
17228-
@Packed(1)
17229-
class Outer extends Struct {
17230-
external Pointer<Uint8> notEmpty;
17231-
17232-
external Inner nestedLooselyPacked;
17233-
}
17234-
```
17235-
17236-
If the outer struct should be packed less tightly, then change the
17237-
argument to the outer struct's `Packed` annotation:
17238-
17239-
```dart
17240-
import 'dart:ffi';
17241-
17242-
@Packed(8)
17243-
class Inner extends Struct {
17244-
external Pointer<Uint8> notEmpty;
17245-
}
17246-
17247-
@Packed(8)
17248-
class Outer extends Struct {
17249-
external Pointer<Uint8> notEmpty;
17250-
17251-
external Inner nestedLooselyPacked;
17252-
}
17253-
```
17254-
17255-
If the inner struct doesn't have an annotation and should be packed, then
17256-
add an annotation.
17257-
17258-
If the inner struct doesn't have an annotation and the outer struct
17259-
shouldn't be packed, then remove its annotation.
1726017175
SIZE_ANNOTATION_DIMENSIONS:
1726117176
problemMessage: "'Array's must have an 'Array' annotation that matches the dimensions."
1726217177
correctionMessage: "Try adjusting the arguments in the 'Array' annotation."

pkg/analyzer/test/src/diagnostics/packed_nesting_non_packed_test.dart

Lines changed: 0 additions & 116 deletions
This file was deleted.

pkg/analyzer/test/src/diagnostics/test_all.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,6 @@ import 'override_on_non_overriding_setter_test.dart'
588588
as override_on_non_overriding_setter;
589589
import 'packed_annotation_alignment_test.dart' as packed_annotation_alignment;
590590
import 'packed_annotation_test.dart' as packed_annotation;
591-
import 'packed_nesting_non_packed_test.dart' as packed_nesting_non_packed;
592591
import 'part_of_different_library_test.dart' as part_of_different_library;
593592
import 'part_of_non_part_test.dart' as part_of_non_part;
594593
import 'positional_super_formal_parameter_with_positional_argument_test.dart'
@@ -1184,7 +1183,6 @@ main() {
11841183
override_on_non_overriding_setter.main();
11851184
packed_annotation.main();
11861185
packed_annotation_alignment.main();
1187-
packed_nesting_non_packed.main();
11881186
part_of_different_library.main();
11891187
part_of_non_part.main();
11901188
positional_super_formal_parameter_with_positional_argument.main();

0 commit comments

Comments
 (0)