[pigeon] Adds @SwiftClass annotation (#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].  [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 <stuartmorgan@google.com>
flutter · Mar 26, 2024 · ab1630b · ab1630b
1 parent 28d126c
commit ab1630b
Show file tree

Hide file tree

Showing 31 changed files with 5,190 additions and 159 deletions.
diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 17.3.0
+
+* [swift] Adds `@SwiftClass` annotation to allow choice between `struct` and `class` for data classes.
+* [cpp] Adds support for recursive data class definitions.
+
 ## 17.2.0
 
 * [dart] Adds implementation for `@ProxyApi`.

diff --git a/packages/pigeon/README.md b/packages/pigeon/README.md
@@ -27,6 +27,11 @@ Custom classes, nested datatypes, and enums are also supported.
 
 Nullable enums in Objective-C generated code will be wrapped in a class to allow for nullability.
 
+By default, custom classes in Swift are defined as structs. 
+Structs don't support some features - recursive data, or Objective-C interop.
+Use the @SwiftClass annotation when defining the class to generate the data
+as a Swift class instead.
+
 ### Synchronous and Asynchronous methods
 
 While all calls across platform channel APIs (such as pigeon methods) are asynchronous,

diff --git a/packages/pigeon/lib/ast.dart b/packages/pigeon/lib/ast.dart
@@ -639,6 +639,7 @@ class Class extends Node {
   Class({
     required this.name,
     required this.fields,
+    this.isSwiftClass = false,
     this.documentationComments = const <String>[],
   });
 
@@ -648,6 +649,12 @@ class Class extends Node {
   /// All the fields contained in the class.
   List<NamedType> fields;
 
+  /// Determines whether the defined class should be represented as a struct or
+  /// a class in Swift generation.
+  ///
+  /// Defaults to false, which would represent a struct.
+  bool isSwiftClass;
+
   /// List of documentation comments, separated by line.
   ///
   /// Lines should not include the comment marker itself, but should include any

diff --git a/packages/pigeon/lib/cpp_generator.dart b/packages/pigeon/lib/cpp_generator.dart
@@ -255,6 +255,35 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
         _writeClassConstructor(root, indent, classDefinition, orderedFields,
             'Constructs an object setting all fields.');
 
+        // If any fields are pointer type, then the class requires a custom
+        // copy constructor, so declare the rule-of-five group of functions.
+        if (orderedFields.any((NamedType field) => _isPointerField(
+            getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType)))) {
+          final String className = classDefinition.name;
+          // Add the default destructor, since unique_ptr destroys itself.
+          _writeFunctionDeclaration(indent, '~$className', defaultImpl: true);
+          // Declare custom copy/assign to deep-copy the pointer.
+          _writeFunctionDeclaration(indent, className,
+              isConstructor: true,
+              isCopy: true,
+              parameters: <String>['const $className& other']);
+          _writeFunctionDeclaration(indent, 'operator=',
+              returnType: '$className&',
+              parameters: <String>['const $className& other']);
+          // Re-add the default move operations, since they work fine with
+          // unique_ptr.
+          _writeFunctionDeclaration(indent, className,
+              isConstructor: true,
+              isCopy: true,
+              parameters: <String>['$className&& other'],
+              defaultImpl: true);
+          _writeFunctionDeclaration(indent, 'operator=',
+              returnType: '$className&',
+              parameters: <String>['$className&& other'],
+              defaultImpl: true,
+              noexcept: true);
+        }
+
         for (final NamedType field in orderedFields) {
           addDocumentationComments(
               indent, field.documentationComments, _docCommentSpec);
@@ -313,7 +342,7 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
           final HostDatatype hostDatatype =
               getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType);
           indent.writeln(
-              '${_valueType(hostDatatype)} ${_makeInstanceVariableName(field)};');
+              '${_fieldType(hostDatatype)} ${_makeInstanceVariableName(field)};');
         }
       });
     }, nestCount: 0);
@@ -693,6 +722,13 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
     // All-field constructor.
     _writeClassConstructor(root, indent, classDefinition, orderedFields);
 
+    // Custom copy/assign to handle pointer fields, if necessary.
+    if (orderedFields.any((NamedType field) => _isPointerField(
+        getFieldHostDatatype(field, _baseCppTypeForBuiltinDartType)))) {
+      _writeCopyConstructor(root, indent, classDefinition, orderedFields);
+      _writeAssignmentOperator(root, indent, classDefinition, orderedFields);
+    }
+
     // Getters and setters.
     for (final NamedType field in orderedFields) {
       _writeCppSourceClassField(
@@ -1169,15 +1205,69 @@ return EncodableValue(EncodableList{
         initializers: initializerStrings);
   }
 
+  void _writeCopyConstructor(Root root, Indent indent, Class classDefinition,
+      Iterable<NamedType> fields) {
+    final List<String> initializerStrings = fields.map((NamedType param) {
+      final String fieldName = _makeInstanceVariableName(param);
+      final HostDatatype hostType = getFieldHostDatatype(
+        param,
+        _shortBaseCppTypeForBuiltinDartType,
+      );
+      return '$fieldName(${_fieldValueExpression(hostType, 'other.$fieldName', sourceIsField: true)})';
+    }).toList();
+    _writeFunctionDefinition(indent, classDefinition.name,
+        scope: classDefinition.name,
+        parameters: <String>['const ${classDefinition.name}& other'],
+        initializers: initializerStrings);
+  }
+
+  void _writeAssignmentOperator(Root root, Indent indent, Class classDefinition,
+      Iterable<NamedType> fields) {
+    _writeFunctionDefinition(indent, 'operator=',
+        scope: classDefinition.name,
+        returnType: '${classDefinition.name}&',
+        parameters: <String>['const ${classDefinition.name}& other'], body: () {
+      for (final NamedType field in fields) {
+        final HostDatatype hostDatatype =
+            getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
+
+        final String ivarName = _makeInstanceVariableName(field);
+        final String otherIvar = 'other.$ivarName';
+        final String valueExpression;
+        if (_isPointerField(hostDatatype)) {
+          final String constructor =
+              'std::make_unique<${hostDatatype.datatype}>(*$otherIvar)';
+          valueExpression = hostDatatype.isNullable
+              ? '$otherIvar ? $constructor : nullptr'
+              : constructor;
+        } else {
+          valueExpression = otherIvar;
+        }
+        indent.writeln('$ivarName = $valueExpression;');
+      }
+      indent.writeln('return *this;');
+    });
+  }
+
   void _writeCppSourceClassField(CppOptions generatorOptions, Root root,
       Indent indent, Class classDefinition, NamedType field) {
     final HostDatatype hostDatatype =
         getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
     final String instanceVariableName = _makeInstanceVariableName(field);
     final String setterName = _makeSetterName(field);
-    final String returnExpression = hostDatatype.isNullable
-        ? '$instanceVariableName ? &(*$instanceVariableName) : nullptr'
-        : instanceVariableName;
+    final String returnExpression;
+    if (_isPointerField(hostDatatype)) {
+      // Convert std::unique_ptr<T> to either T* or const T&.
+      returnExpression = hostDatatype.isNullable
+          ? '$instanceVariableName.get()'
+          : '*$instanceVariableName';
+    } else if (hostDatatype.isNullable) {
+      // Convert std::optional<T> to T*.
+      returnExpression =
+          '$instanceVariableName ? &(*$instanceVariableName) : nullptr';
+    } else {
+      returnExpression = instanceVariableName;
+    }
 
     // Writes a setter treating the type as [type], to allow generating multiple
     // setter variants.
@@ -1220,10 +1310,20 @@ return EncodableValue(EncodableList{
   /// Returns the value to use when setting a field of the given type from
   /// an argument of that type.
   ///
-  /// For non-nullable values this is just the variable itself, but for nullable
-  /// values this handles the conversion between an argument type (a pointer)
-  /// and the field type (a std::optional).
-  String _fieldValueExpression(HostDatatype type, String variable) {
+  /// For non-nullable and non-custom-class values this is just the variable
+  /// itself, but for other values this handles the conversion between an
+  /// argument type (a pointer or value/reference) and the field type
+  /// (a std::optional or std::unique_ptr).
+  String _fieldValueExpression(HostDatatype type, String variable,
+      {bool sourceIsField = false}) {
+    if (_isPointerField(type)) {
+      final String constructor = 'std::make_unique<${type.datatype}>';
+      // If the source is a pointer field, it always needs dereferencing.
+      final String maybeDereference = sourceIsField ? '*' : '';
+      return type.isNullable
+          ? '$variable ? $constructor(*$variable) : nullptr'
+          : '$constructor($maybeDereference$variable)';
+    }
     return type.isNullable
         ? '$variable ? ${_valueType(type)}(*$variable) : std::nullopt'
         : variable;
@@ -1309,7 +1409,8 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
     if (!hostType.isBuiltin &&
         root.classes.any((Class c) => c.name == dartType.baseName)) {
       if (preSerializeClasses) {
-        final String operator = hostType.isNullable ? '->' : '.';
+        final String operator =
+            hostType.isNullable || _isPointerField(hostType) ? '->' : '.';
         encodableValue =
             'EncodableValue($variableName${operator}ToEncodableList())';
       } else {
@@ -1547,6 +1648,23 @@ String _valueType(HostDatatype type) {
   return type.isNullable ? 'std::optional<$baseType>' : baseType;
 }
 
+/// Returns the C++ type to use when declaring a data class field for the
+/// given type.
+String _fieldType(HostDatatype type) {
+  return _isPointerField(type)
+      ? 'std::unique_ptr<${type.datatype}>'
+      : _valueType(type);
+}
+
+/// Returns true if [type] should be stored as a pointer, rather than a
+/// value type, in a data class.
+bool _isPointerField(HostDatatype type) {
+  // Custom class types are stored as `unique_ptr`s since they can have
+  // arbitrary size, and can also be arbitrarily (including recursively)
+  // nested, so must be stored as pointers.
+  return !type.isBuiltin && !type.isEnum;
+}
+
 /// Returns the C++ type to use in an argument context without ownership
 /// transfer for the given base type.
 String _unownedArgumentType(HostDatatype type) {
@@ -1723,17 +1841,21 @@ void _writeFunctionDeclaration(
   bool isStatic = false,
   bool isVirtual = false,
   bool isConstructor = false,
+  bool isCopy = false,
   bool isPureVirtual = false,
   bool isConst = false,
   bool isOverride = false,
   bool deleted = false,
+  bool defaultImpl = false,
   bool inlineNoop = false,
+  bool noexcept = false,
   void Function()? inlineBody,
 }) {
   assert(!(isVirtual && isOverride), 'virtual is redundant with override');
   assert(isVirtual || !isPureVirtual, 'pure virtual methods must be virtual');
   assert(returnType == null || !isConstructor,
       'constructors cannot have return types');
+  assert(!(deleted && defaultImpl), 'a function cannot be deleted and default');
   _writeFunction(
     indent,
     inlineNoop || (inlineBody != null)
@@ -1746,12 +1868,14 @@ void _writeFunctionDeclaration(
       if (inlineBody != null) 'inline',
       if (isStatic) 'static',
       if (isVirtual) 'virtual',
-      if (isConstructor && parameters.isNotEmpty) 'explicit'
+      if (isConstructor && parameters.isNotEmpty && !isCopy) 'explicit'
     ],
     trailingAnnotations: <String>[
       if (isConst) 'const',
+      if (noexcept) 'noexcept',
       if (isOverride) 'override',
       if (deleted) '= delete',
+      if (defaultImpl) '= default',
       if (isPureVirtual) '= 0',
     ],
     body: inlineBody,

diff --git a/packages/pigeon/lib/generator_tools.dart b/packages/pigeon/lib/generator_tools.dart
@@ -13,7 +13,7 @@ import 'ast.dart';
 /// The current version of pigeon.
 ///
 /// This must match the version in pubspec.yaml.
-const String pigeonVersion = '17.2.0';
+const String pigeonVersion = '17.3.0';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

diff --git a/packages/pigeon/lib/pigeon_lib.dart b/packages/pigeon/lib/pigeon_lib.dart
@@ -177,6 +177,12 @@ class SwiftFunction {
   final String value;
 }
 
+/// Metadata to annotate data classes to be defined as class in Swift output.
+class SwiftClass {
+  /// Constructor.
+  const SwiftClass();
+}
+
 /// Type of TaskQueue which determines how handlers are dispatched for
 /// HostApi's.
 enum TaskQueueType {
@@ -1550,6 +1556,7 @@ class _RootBuilder extends dart_ast_visitor.RecursiveAstVisitor<Object?> {
       _currentClass = Class(
         name: node.name.lexeme,
         fields: <NamedType>[],
+        isSwiftClass: _hasMetadata(node.metadata, 'SwiftClass'),
         documentationComments:
             _documentationCommentsParser(node.documentationComment?.tokens),
       );

diff --git a/packages/pigeon/lib/swift_generator.dart b/packages/pigeon/lib/swift_generator.dart
@@ -125,11 +125,26 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
         indent, classDefinition.documentationComments, _docCommentSpec,
         generatorComments: generatedComments);
 
-    indent.write('struct ${classDefinition.name} ');
+    if (classDefinition.isSwiftClass) {
+      indent.write('class ${classDefinition.name} ');
+    } else {
+      indent.write('struct ${classDefinition.name} ');
+    }
     indent.addScoped('{', '}', () {
-      getFieldsInSerializationOrder(classDefinition).forEach((NamedType field) {
-        _writeClassField(indent, field);
-      });
+      final Iterable<NamedType> fields =
+          getFieldsInSerializationOrder(classDefinition);
+
+      if (classDefinition.isSwiftClass) {
+        _writeClassInit(indent, fields.toList());
+      }
+
+      for (final NamedType field in fields) {
+        addDocumentationComments(
+            indent, field.documentationComments, _docCommentSpec);
+        indent.write('var ');
+        _writeClassField(indent, field, addNil: !classDefinition.isSwiftClass);
+        indent.newln();
+      }
 
       indent.newln();
       writeClassDecode(
@@ -149,6 +164,35 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
     });
   }
 
+  void _writeClassInit(Indent indent, List<NamedType> fields) {
+    indent.writeScoped('init(', ')', () {
+      for (int i = 0; i < fields.length; i++) {
+        indent.write('');
+        _writeClassField(indent, fields[i]);
+        if (i == fields.length - 1) {
+          indent.newln();
+        } else {
+          indent.addln(',');
+        }
+      }
+    }, addTrailingNewline: false);
+    indent.addScoped(' {', '}', () {
+      for (final NamedType field in fields) {
+        _writeClassFieldInit(indent, field);
+      }
+    });
+  }
+
+  void _writeClassField(Indent indent, NamedType field, {bool addNil = true}) {
+    indent.add('${field.name}: ${_nullsafeSwiftTypeForDartType(field.type)}');
+    final String defaultNil = field.type.isNullable && addNil ? ' = nil' : '';
+    indent.add(defaultNil);
+  }
+
+  void _writeClassFieldInit(Indent indent, NamedType field) {
+    indent.writeln('self.${field.name} = ${field.name}');
+  }
+
   @override
   void writeClassEncode(
     SwiftOptions generatorOptions,
@@ -222,16 +266,6 @@ class SwiftGenerator extends StructuredGenerator<SwiftOptions> {
     });
   }
 
-  void _writeClassField(Indent indent, NamedType field) {
-    addDocumentationComments(
-        indent, field.documentationComments, _docCommentSpec);
-
-    indent.write(
-        'var ${field.name}: ${_nullsafeSwiftTypeForDartType(field.type)}');
-    final String defaultNil = field.type.isNullable ? ' = nil' : '';
-    indent.addln(defaultNil);
-  }
-
   @override
   void writeApis(
     SwiftOptions generatorOptions,