Skip to content

Commit ab1630b

Browse files
[pigeon] Adds @SwiftClass annotation (flutter#6372)
Adds annotation for @SwiftClass which will cause the swift generator to create a `class` instead of a `struct` in the generated Swift code. This will allow for recursive classes as well as allow for Objc interop, without forcing users to lose the potential benefits of structs if they prefer them instead. Also creates recursive data class integration test to check for coverage on all languages. fixes flutter/flutter#145175 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.) - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes]. - [x] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style]. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates [following repository CHANGELOG style]: https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests --------- Co-authored-by: Stuart Morgan <[email protected]>
1 parent 28d126c commit ab1630b

File tree

31 files changed

+5190
-159
lines changed

31 files changed

+5190
-159
lines changed

packages/pigeon/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 17.3.0
2+
3+
* [swift] Adds `@SwiftClass` annotation to allow choice between `struct` and `class` for data classes.
4+
* [cpp] Adds support for recursive data class definitions.
5+
16
## 17.2.0
27

38
* [dart] Adds implementation for `@ProxyApi`.

packages/pigeon/README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ Custom classes, nested datatypes, and enums are also supported.
2727

2828
Nullable enums in Objective-C generated code will be wrapped in a class to allow for nullability.
2929

30+
By default, custom classes in Swift are defined as structs.
31+
Structs don't support some features - recursive data, or Objective-C interop.
32+
Use the @SwiftClass annotation when defining the class to generate the data
33+
as a Swift class instead.
34+
3035
### Synchronous and Asynchronous methods
3136

3237
While all calls across platform channel APIs (such as pigeon methods) are asynchronous,

packages/pigeon/lib/ast.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,7 @@ class Class extends Node {
639639
Class({
640640
required this.name,
641641
required this.fields,
642+
this.isSwiftClass = false,
642643
this.documentationComments = const <String>[],
643644
});
644645

@@ -648,6 +649,12 @@ class Class extends Node {
648649
/// All the fields contained in the class.
649650
List<NamedType> fields;
650651

652+
/// Determines whether the defined class should be represented as a struct or
653+
/// a class in Swift generation.
654+
///
655+
/// Defaults to false, which would represent a struct.
656+
bool isSwiftClass;
657+
651658
/// List of documentation comments, separated by line.
652659
///
653660
/// Lines should not include the comment marker itself, but should include any

packages/pigeon/lib/cpp_generator.dart

Lines changed: 134 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,35 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
255255
_writeClassConstructor(root, indent, classDefinition, orderedFields,
256256
'Constructs an object setting all fields.');
257257

258+
// If any fields are pointer type, then the class requires a custom
259+
// copy constructor, so declare the rule-of-five group of functions.
260+
if (orderedFields.any((NamedType field) => _isPointerField(
261+
getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType)))) {
262+
final String className = classDefinition.name;
263+
// Add the default destructor, since unique_ptr destroys itself.
264+
_writeFunctionDeclaration(indent, '~$className', defaultImpl: true);
265+
// Declare custom copy/assign to deep-copy the pointer.
266+
_writeFunctionDeclaration(indent, className,
267+
isConstructor: true,
268+
isCopy: true,
269+
parameters: <String>['const $className& other']);
270+
_writeFunctionDeclaration(indent, 'operator=',
271+
returnType: '$className&',
272+
parameters: <String>['const $className& other']);
273+
// Re-add the default move operations, since they work fine with
274+
// unique_ptr.
275+
_writeFunctionDeclaration(indent, className,
276+
isConstructor: true,
277+
isCopy: true,
278+
parameters: <String>['$className&& other'],
279+
defaultImpl: true);
280+
_writeFunctionDeclaration(indent, 'operator=',
281+
returnType: '$className&',
282+
parameters: <String>['$className&& other'],
283+
defaultImpl: true,
284+
noexcept: true);
285+
}
286+
258287
for (final NamedType field in orderedFields) {
259288
addDocumentationComments(
260289
indent, field.documentationComments, _docCommentSpec);
@@ -313,7 +342,7 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
313342
final HostDatatype hostDatatype =
314343
getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType);
315344
indent.writeln(
316-
'${_valueType(hostDatatype)} ${_makeInstanceVariableName(field)};');
345+
'${_fieldType(hostDatatype)} ${_makeInstanceVariableName(field)};');
317346
}
318347
});
319348
}, nestCount: 0);
@@ -693,6 +722,13 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
693722
// All-field constructor.
694723
_writeClassConstructor(root, indent, classDefinition, orderedFields);
695724

725+
// Custom copy/assign to handle pointer fields, if necessary.
726+
if (orderedFields.any((NamedType field) => _isPointerField(
727+
getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType)))) {
728+
_writeCopyConstructor(root, indent, classDefinition, orderedFields);
729+
_writeAssignmentOperator(root, indent, classDefinition, orderedFields);
730+
}
731+
696732
// Getters and setters.
697733
for (final NamedType field in orderedFields) {
698734
_writeCppSourceClassField(
@@ -1169,15 +1205,69 @@ return EncodableValue(EncodableList{
11691205
initializers: initializerStrings);
11701206
}
11711207

1208+
void _writeCopyConstructor(Root root, Indent indent, Class classDefinition,
1209+
Iterable<NamedType> fields) {
1210+
final List<String> initializerStrings = fields.map((NamedType param) {
1211+
final String fieldName = _makeInstanceVariableName(param);
1212+
final HostDatatype hostType = getFieldHostDatatype(
1213+
param,
1214+
_shortBaseCppTypeForBuiltinDartType,
1215+
);
1216+
return '$fieldName(${_fieldValueExpression(hostType, 'other.$fieldName', sourceIsField: true)})';
1217+
}).toList();
1218+
_writeFunctionDefinition(indent, classDefinition.name,
1219+
scope: classDefinition.name,
1220+
parameters: <String>['const ${classDefinition.name}& other'],
1221+
initializers: initializerStrings);
1222+
}
1223+
1224+
void _writeAssignmentOperator(Root root, Indent indent, Class classDefinition,
1225+
Iterable<NamedType> fields) {
1226+
_writeFunctionDefinition(indent, 'operator=',
1227+
scope: classDefinition.name,
1228+
returnType: '${classDefinition.name}&',
1229+
parameters: <String>['const ${classDefinition.name}& other'], body: () {
1230+
for (final NamedType field in fields) {
1231+
final HostDatatype hostDatatype =
1232+
getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
1233+
1234+
final String ivarName = _makeInstanceVariableName(field);
1235+
final String otherIvar = 'other.$ivarName';
1236+
final String valueExpression;
1237+
if (_isPointerField(hostDatatype)) {
1238+
final String constructor =
1239+
'std::make_unique<${hostDatatype.datatype}>(*$otherIvar)';
1240+
valueExpression = hostDatatype.isNullable
1241+
? '$otherIvar ? $constructor : nullptr'
1242+
: constructor;
1243+
} else {
1244+
valueExpression = otherIvar;
1245+
}
1246+
indent.writeln('$ivarName = $valueExpression;');
1247+
}
1248+
indent.writeln('return *this;');
1249+
});
1250+
}
1251+
11721252
void _writeCppSourceClassField(CppOptions generatorOptions, Root root,
11731253
Indent indent, Class classDefinition, NamedType field) {
11741254
final HostDatatype hostDatatype =
11751255
getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
11761256
final String instanceVariableName = _makeInstanceVariableName(field);
11771257
final String setterName = _makeSetterName(field);
1178-
final String returnExpression = hostDatatype.isNullable
1179-
? '$instanceVariableName ? &(*$instanceVariableName) : nullptr'
1180-
: instanceVariableName;
1258+
final String returnExpression;
1259+
if (_isPointerField(hostDatatype)) {
1260+
// Convert std::unique_ptr<T> to either T* or const T&.
1261+
returnExpression = hostDatatype.isNullable
1262+
? '$instanceVariableName.get()'
1263+
: '*$instanceVariableName';
1264+
} else if (hostDatatype.isNullable) {
1265+
// Convert std::optional<T> to T*.
1266+
returnExpression =
1267+
'$instanceVariableName ? &(*$instanceVariableName) : nullptr';
1268+
} else {
1269+
returnExpression = instanceVariableName;
1270+
}
11811271

11821272
// Writes a setter treating the type as [type], to allow generating multiple
11831273
// setter variants.
@@ -1220,10 +1310,20 @@ return EncodableValue(EncodableList{
12201310
/// Returns the value to use when setting a field of the given type from
12211311
/// an argument of that type.
12221312
///
1223-
/// For non-nullable values this is just the variable itself, but for nullable
1224-
/// values this handles the conversion between an argument type (a pointer)
1225-
/// and the field type (a std::optional).
1226-
String _fieldValueExpression(HostDatatype type, String variable) {
1313+
/// For non-nullable and non-custom-class values this is just the variable
1314+
/// itself, but for other values this handles the conversion between an
1315+
/// argument type (a pointer or value/reference) and the field type
1316+
/// (a std::optional or std::unique_ptr).
1317+
String _fieldValueExpression(HostDatatype type, String variable,
1318+
{bool sourceIsField = false}) {
1319+
if (_isPointerField(type)) {
1320+
final String constructor = 'std::make_unique<${type.datatype}>';
1321+
// If the source is a pointer field, it always needs dereferencing.
1322+
final String maybeDereference = sourceIsField ? '*' : '';
1323+
return type.isNullable
1324+
? '$variable ? $constructor(*$variable) : nullptr'
1325+
: '$constructor($maybeDereference$variable)';
1326+
}
12271327
return type.isNullable
12281328
? '$variable ? ${_valueType(type)}(*$variable) : std::nullopt'
12291329
: variable;
@@ -1309,7 +1409,8 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
13091409
if (!hostType.isBuiltin &&
13101410
root.classes.any((Class c) => c.name == dartType.baseName)) {
13111411
if (preSerializeClasses) {
1312-
final String operator = hostType.isNullable ? '->' : '.';
1412+
final String operator =
1413+
hostType.isNullable || _isPointerField(hostType) ? '->' : '.';
13131414
encodableValue =
13141415
'EncodableValue($variableName${operator}ToEncodableList())';
13151416
} else {
@@ -1547,6 +1648,23 @@ String _valueType(HostDatatype type) {
15471648
return type.isNullable ? 'std::optional<$baseType>' : baseType;
15481649
}
15491650

1651+
/// Returns the C++ type to use when declaring a data class field for the
1652+
/// given type.
1653+
String _fieldType(HostDatatype type) {
1654+
return _isPointerField(type)
1655+
? 'std::unique_ptr<${type.datatype}>'
1656+
: _valueType(type);
1657+
}
1658+
1659+
/// Returns true if [type] should be stored as a pointer, rather than a
1660+
/// value type, in a data class.
1661+
bool _isPointerField(HostDatatype type) {
1662+
// Custom class types are stored as `unique_ptr`s since they can have
1663+
// arbitrary size, and can also be arbitrarily (including recursively)
1664+
// nested, so must be stored as pointers.
1665+
return !type.isBuiltin && !type.isEnum;
1666+
}
1667+
15501668
/// Returns the C++ type to use in an argument context without ownership
15511669
/// transfer for the given base type.
15521670
String _unownedArgumentType(HostDatatype type) {
@@ -1723,17 +1841,21 @@ void _writeFunctionDeclaration(
17231841
bool isStatic = false,
17241842
bool isVirtual = false,
17251843
bool isConstructor = false,
1844+
bool isCopy = false,
17261845
bool isPureVirtual = false,
17271846
bool isConst = false,
17281847
bool isOverride = false,
17291848
bool deleted = false,
1849+
bool defaultImpl = false,
17301850
bool inlineNoop = false,
1851+
bool noexcept = false,
17311852
void Function()? inlineBody,
17321853
}) {
17331854
assert(!(isVirtual && isOverride), 'virtual is redundant with override');
17341855
assert(isVirtual || !isPureVirtual, 'pure virtual methods must be virtual');
17351856
assert(returnType == null || !isConstructor,
17361857
'constructors cannot have return types');
1858+
assert(!(deleted && defaultImpl), 'a function cannot be deleted and default');
17371859
_writeFunction(
17381860
indent,
17391861
inlineNoop || (inlineBody != null)
@@ -1746,12 +1868,14 @@ void _writeFunctionDeclaration(
17461868
if (inlineBody != null) 'inline',
17471869
if (isStatic) 'static',
17481870
if (isVirtual) 'virtual',
1749-
if (isConstructor && parameters.isNotEmpty) 'explicit'
1871+
if (isConstructor && parameters.isNotEmpty && !isCopy) 'explicit'
17501872
],
17511873
trailingAnnotations: <String>[
17521874
if (isConst) 'const',
1875+
if (noexcept) 'noexcept',
17531876
if (isOverride) 'override',
17541877
if (deleted) '= delete',
1878+
if (defaultImpl) '= default',
17551879
if (isPureVirtual) '= 0',
17561880
],
17571881
body: inlineBody,

packages/pigeon/lib/generator_tools.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import 'ast.dart';
1313
/// The current version of pigeon.
1414
///
1515
/// This must match the version in pubspec.yaml.
16-
const String pigeonVersion = '17.2.0';
16+
const String pigeonVersion = '17.3.0';
1717

1818
/// Read all the content from [stdin] to a String.
1919
String readStdin() {

packages/pigeon/lib/pigeon_lib.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,12 @@ class SwiftFunction {
177177
final String value;
178178
}
179179

180+
/// Metadata to annotate data classes to be defined as class in Swift output.
181+
class SwiftClass {
182+
/// Constructor.
183+
const SwiftClass();
184+
}
185+
180186
/// Type of TaskQueue which determines how handlers are dispatched for
181187
/// HostApi's.
182188
enum TaskQueueType {
@@ -1550,6 +1556,7 @@ class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
15501556
_currentClass = Class(
15511557
name: node.name.lexeme,
15521558
fields: <NamedType>[],
1559+
isSwiftClass: _hasMetadata(node.metadata, 'SwiftClass'),
15531560
documentationComments:
15541561
_documentationCommentsParser(node.documentationComment?.tokens),
15551562
);

packages/pigeon/lib/swift_generator.dart

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,26 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
125125
indent, classDefinition.documentationComments, _docCommentSpec,
126126
generatorComments: generatedComments);
127127

128-
indent.write('struct ${classDefinition.name} ');
128+
if (classDefinition.isSwiftClass) {
129+
indent.write('class ${classDefinition.name} ');
130+
} else {
131+
indent.write('struct ${classDefinition.name} ');
132+
}
129133
indent.addScoped('{', '}', () {
130-
getFieldsInSerializationOrder(classDefinition).forEach((NamedType field) {
131-
_writeClassField(indent, field);
132-
});
134+
final Iterable<NamedType> fields =
135+
getFieldsInSerializationOrder(classDefinition);
136+
137+
if (classDefinition.isSwiftClass) {
138+
_writeClassInit(indent, fields.toList());
139+
}
140+
141+
for (final NamedType field in fields) {
142+
addDocumentationComments(
143+
indent, field.documentationComments, _docCommentSpec);
144+
indent.write('var ');
145+
_writeClassField(indent, field, addNil: !classDefinition.isSwiftClass);
146+
indent.newln();
147+
}
133148

134149
indent.newln();
135150
writeClassDecode(
@@ -149,6 +164,35 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
149164
});
150165
}
151166

167+
void _writeClassInit(Indent indent, List<NamedType> fields) {
168+
indent.writeScoped('init(', ')', () {
169+
for (int i = 0; i < fields.length; i++) {
170+
indent.write('');
171+
_writeClassField(indent, fields[i]);
172+
if (i == fields.length - 1) {
173+
indent.newln();
174+
} else {
175+
indent.addln(',');
176+
}
177+
}
178+
}, addTrailingNewline: false);
179+
indent.addScoped(' {', '}', () {
180+
for (final NamedType field in fields) {
181+
_writeClassFieldInit(indent, field);
182+
}
183+
});
184+
}
185+
186+
void _writeClassField(Indent indent, NamedType field, {bool addNil = true}) {
187+
indent.add('${field.name}: ${_nullsafeSwiftTypeForDartType(field.type)}');
188+
final String defaultNil = field.type.isNullable && addNil ? ' = nil' : '';
189+
indent.add(defaultNil);
190+
}
191+
192+
void _writeClassFieldInit(Indent indent, NamedType field) {
193+
indent.writeln('self.${field.name} = ${field.name}');
194+
}
195+
152196
@override
153197
void writeClassEncode(
154198
SwiftOptions generatorOptions,
@@ -222,16 +266,6 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
222266
});
223267
}
224268

225-
void _writeClassField(Indent indent, NamedType field) {
226-
addDocumentationComments(
227-
indent, field.documentationComments, _docCommentSpec);
228-
229-
indent.write(
230-
'var ${field.name}: ${_nullsafeSwiftTypeForDartType(field.type)}');
231-
final String defaultNil = field.type.isNullable ? ' = nil' : '';
232-
indent.addln(defaultNil);
233-
}
234-
235269
@override
236270
void writeApis(
237271
SwiftOptions generatorOptions,

0 commit comments

Comments
 (0)