Skip to content

Commit

Permalink
[pigeon] Improve C++ data class constructors (#3585)
Browse files Browse the repository at this point in the history 
[pigeon] Improve C++ data class constructors
  • Loading branch information
@stuartmorgan
stuartmorgan committed Mar 30, 2023
1 parent 199fc68 commit ee37f1c
Show file tree
Hide file tree
Showing 26 changed files with 466 additions and 257 deletions.
9 changes: 9 additions & 0 deletions packages/pigeon/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 9.1.3

* [cpp] Requires passing any non-nullable fields of generated data classes as
constructor arguments, similar to what is done in other languages. This may
require updates to existing code that creates data class instances on the
native side.
* [cpp] Adds a convenience constructor to generated data classes to set all
fields during construction.

## 9.1.2

* [cpp] Fixes class parameters to Flutter APIs.
Expand Down
228 changes: 169 additions & 59 deletions packages/pigeon/lib/cpp_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,24 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
indent, klass.documentationComments, _docCommentSpec,
generatorComments: generatedMessages);

final Iterable<NamedType> orderedFields =
getFieldsInSerializationOrder(klass);

indent.write('class ${klass.name} ');
indent.addScoped('{', '};', () {
indent.addScoped(' public:', '', () {
indent.writeln('${klass.name}();');
for (final NamedType field in getFieldsInSerializationOrder(klass)) {
final Iterable<NamedType> requiredFields =
orderedFields.where((NamedType type) => !type.type.isNullable);
// Minimal constructor, if needed.
if (requiredFields.length != orderedFields.length) {
_writeClassConstructor(root, indent, klass, requiredFields,
'Constructs an object setting all non-nullable fields.');
}
// All-field constructor.
_writeClassConstructor(root, indent, klass, orderedFields,
'Constructs an object setting all fields.');

for (final NamedType field in orderedFields) {
addDocumentationComments(
indent, field.documentationComments, _docCommentSpec);
final HostDatatype baseDatatype = getFieldHostDatatype(
Expand All @@ -209,7 +222,8 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
});

indent.addScoped(' private:', '', () {
indent.writeln('${klass.name}(const flutter::EncodableList& list);');
indent.writeln(
'static ${klass.name} FromEncodableList(const flutter::EncodableList& list);');
indent.writeln('flutter::EncodableList ToEncodableList() const;');
for (final Class friend in root.classes) {
if (friend != klass &&
Expand All @@ -228,7 +242,7 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
indent.writeln('friend class $testFixtureClass;');
}

for (final NamedType field in getFieldsInSerializationOrder(klass)) {
for (final NamedType field in orderedFields) {
final HostDatatype hostDatatype = getFieldHostDatatype(
field, root.classes, root.enums, _baseCppTypeForBuiltinDartType);
indent.writeln(
Expand Down Expand Up @@ -366,6 +380,23 @@ class CppHeaderGenerator extends StructuredGenerator<CppOptions> {
}, nestCount: 0);
}

void _writeClassConstructor(Root root, Indent indent, Class klass,
Iterable<NamedType> params, String docComment) {
final String explicit = params.isEmpty ? '' : 'explicit ';
String paramString = params.map((NamedType param) {
final HostDatatype hostDatatype = getFieldHostDatatype(
param, root.classes, root.enums, _baseCppTypeForBuiltinDartType);
return '\t${_hostApiArgumentType(hostDatatype)} ${_makeVariableName(param)}';
}).join(',\n');
if (paramString.isNotEmpty) {
paramString = '\n$paramString';
}
indent.format('''
$_commentPrefix $docComment
$explicit${klass.name}($paramString);
''');
}

void _writeCodec(
CppOptions generatorOptions, Root root, Indent indent, Api api) {
assert(getCodecClasses(api, root).isNotEmpty);
Expand Down Expand Up @@ -423,12 +454,10 @@ class FlutterError {
template<class T> class ErrorOr {
public:
\tErrorOr(const T& rhs) { new(&v_) T(rhs); }
\tErrorOr(const T&& rhs) { v_ = std::move(rhs); }
\tErrorOr(const FlutterError& rhs) {
\t\tnew(&v_) FlutterError(rhs);
\t}
\tErrorOr(const FlutterError&& rhs) { v_ = std::move(rhs); }
\tErrorOr(const T& rhs) : v_(rhs) {}
\tErrorOr(const T&& rhs) : v_(std::move(rhs)) {}
\tErrorOr(const FlutterError& rhs) : v_(rhs) {}
\tErrorOr(const FlutterError&& rhs) : v_(std::move(rhs)) {}
\tbool has_error() const { return std::holds_alternative<FlutterError>(v_); }
\tconst T& value() const { return std::get<T>(v_); };
Expand Down Expand Up @@ -528,19 +557,26 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
indent.writeln('$_commentPrefix ${klass.name}');
indent.newln();

final Iterable<NamedType> orderedFields =
getFieldsInSerializationOrder(klass);
final Iterable<NamedType> requiredFields =
orderedFields.where((NamedType type) => !type.type.isNullable);
// Minimal constructor, if needed.
if (requiredFields.length != orderedFields.length) {
_writeClassConstructor(root, indent, klass, requiredFields);
}
// All-field constructor.
_writeClassConstructor(root, indent, klass, orderedFields);

// Getters and setters.
for (final NamedType field in getFieldsInSerializationOrder(klass)) {
for (final NamedType field in orderedFields) {
_writeCppSourceClassField(generatorOptions, root, indent, klass, field);
}

// Serialization.
writeClassEncode(generatorOptions, root, indent, klass, customClassNames,
customEnumNames);

// Default constructor.
indent.writeln('${klass.name}::${klass.name}() {}');
indent.newln();

// Deserialization.
writeClassDecode(generatorOptions, root, indent, klass, customClassNames,
customEnumNames);
Expand Down Expand Up @@ -581,47 +617,70 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
Set<String> customClassNames,
Set<String> customEnumNames,
) {
indent.write('${klass.name}::${klass.name}(const EncodableList& list) ');
// Returns the expression to convert the given EncodableValue to a field
// value.
String getValueExpression(NamedType field, String encodable) {
if (customEnumNames.contains(field.type.baseName)) {
return '(${field.type.baseName})(std::get<int32_t>($encodable))';
} else if (field.type.baseName == 'int') {
return '$encodable.LongValue()';
} else {
final HostDatatype hostDatatype = getFieldHostDatatype(field,
root.classes, root.enums, _shortBaseCppTypeForBuiltinDartType);
if (!hostDatatype.isBuiltin &&
root.classes
.map((Class x) => x.name)
.contains(field.type.baseName)) {
return '${hostDatatype.datatype}::FromEncodableList(std::get<EncodableList>($encodable))';
} else {
return 'std::get<${hostDatatype.datatype}>($encodable)';
}
}
}

indent.write(
'${klass.name} ${klass.name}::FromEncodableList(const EncodableList& list) ');
indent.addScoped('{', '}', () {
enumerate(getFieldsInSerializationOrder(klass),
(int index, final NamedType field) {
final String instanceVariableName = _makeInstanceVariableName(field);
final String pointerFieldName =
'${_pointerPrefix}_${_makeVariableName(field)}';
const String instanceVariable = 'decoded';
final Iterable<_IndexedField> indexedFields = indexMap(
getFieldsInSerializationOrder(klass),
(int index, NamedType field) => _IndexedField(index, field));
final Iterable<_IndexedField> nullableFields = indexedFields
.where((_IndexedField field) => field.field.type.isNullable);
final Iterable<_IndexedField> nonNullableFields = indexedFields
.where((_IndexedField field) => !field.field.type.isNullable);

// Non-nullable fields must be set via the constructor.
String constructorArgs = nonNullableFields
.map((_IndexedField param) =>
getValueExpression(param.field, 'list[${param.index}]'))
.join(',\n\t');
if (constructorArgs.isNotEmpty) {
constructorArgs = '(\n\t$constructorArgs)';
}
indent.format('${klass.name} $instanceVariable$constructorArgs;');

// Add the nullable fields via setters, since converting the encodable
// values to the pointer types that the convenience constructor uses for
// nullable fields is non-trivial.
for (final _IndexedField entry in nullableFields) {
final NamedType field = entry.field;
final String setterName = _makeSetterName(field);
final String encodableFieldName =
'${_encodablePrefix}_${_makeVariableName(field)}';
indent.writeln('auto& $encodableFieldName = list[$index];');
if (customEnumNames.contains(field.type.baseName)) {
indent.writeln(
'if (const int32_t* $pointerFieldName = std::get_if<int32_t>(&$encodableFieldName))\t$instanceVariableName = (${field.type.baseName})*$pointerFieldName;');
} else {
final HostDatatype hostDatatype = getFieldHostDatatype(field,
root.classes, root.enums, _shortBaseCppTypeForBuiltinDartType);
if (field.type.baseName == 'int') {
indent.format('''
if (const int32_t* $pointerFieldName = std::get_if<int32_t>(&$encodableFieldName))
\t$instanceVariableName = *$pointerFieldName;
else if (const int64_t* ${pointerFieldName}_64 = std::get_if<int64_t>(&$encodableFieldName))
\t$instanceVariableName = *${pointerFieldName}_64;''');
} else if (!hostDatatype.isBuiltin &&
root.classes
.map((Class x) => x.name)
.contains(field.type.baseName)) {
indent.write(
'if (const EncodableList* $pointerFieldName = std::get_if<EncodableList>(&$encodableFieldName)) ');
indent.addScoped('{', '}', () {
indent.writeln(
'$instanceVariableName = ${hostDatatype.datatype}(*$pointerFieldName);');
});
} else {
indent.write(
'if (const ${hostDatatype.datatype}* $pointerFieldName = std::get_if<${hostDatatype.datatype}>(&$encodableFieldName)) ');
indent.addScoped('{', '}', () {
indent.writeln('$instanceVariableName = *$pointerFieldName;');
});
}
}
});
indent.writeln('auto& $encodableFieldName = list[${entry.index}];');

final String valueExpression =
getValueExpression(field, encodableFieldName);
indent.writeScoped('if (!$encodableFieldName.IsNull()) {', '}', () {
indent.writeln('$instanceVariable.$setterName($valueExpression);');
});
}

// This returns by value, relying on copy elision, since it makes the
// usage more convenient during deserialization than it would be with
// explicit transfer via unique_ptr.
indent.writeln('return $instanceVariable;');
});
}

Expand Down Expand Up @@ -868,7 +927,7 @@ EncodableValue ${api.name}::WrapError(const FlutterError& error) {
indent.writeln('case ${customClass.enumeration}:');
indent.nest(1, () {
indent.writeln(
'return CustomEncodableValue(${customClass.name}(std::get<EncodableList>(ReadValue(stream))));');
'return CustomEncodableValue(${customClass.name}::FromEncodableList(std::get<EncodableList>(ReadValue(stream))));');
});
}
indent.writeln('default:');
Expand Down Expand Up @@ -901,6 +960,42 @@ EncodableValue ${api.name}::WrapError(const FlutterError& error) {
indent.newln();
}

void _writeClassConstructor(
Root root, Indent indent, Class klass, Iterable<NamedType> params) {
final Iterable<_HostNamedType> hostParams = params.map((NamedType param) {
return _HostNamedType(
_makeVariableName(param),
getFieldHostDatatype(
param,
root.classes,
root.enums,
_shortBaseCppTypeForBuiltinDartType,
),
param.type,
);
});

String paramString = hostParams
.map((_HostNamedType param) =>
'\t${_hostApiArgumentType(param.hostType)} ${param.name}')
.join(',\n');
if (paramString.isNotEmpty) {
paramString = '\n$paramString\n';
}

String initializerString = hostParams
.map((_HostNamedType param) =>
'${param.name}_(${_fieldValueExpression(param.hostType, param.name)})')
.join(',\n\t\t');
if (initializerString.isNotEmpty) {
initializerString = ' : $initializerString';
}

indent.format('''
${klass.name}::${klass.name}($paramString)$initializerString {}
''');
}

void _writeCppSourceClassField(CppOptions generatorOptions, Root root,
Indent indent, Class klass, NamedType field) {
final HostDatatype hostDatatype = getFieldHostDatatype(
Expand All @@ -918,11 +1013,8 @@ EncodableValue ${api.name}::WrapError(const FlutterError& error) {
// generating multiple setter variants.
String makeSetter(HostDatatype type) {
const String setterArgumentName = 'value_arg';
final String valueExpression = type.isNullable
? '$setterArgumentName ? ${_valueType(type)}(*$setterArgumentName) : std::nullopt'
: setterArgumentName;
return 'void $qualifiedSetterName(${_unownedArgumentType(type)} $setterArgumentName) '
'{ $instanceVariableName = $valueExpression; }';
'{ $instanceVariableName = ${_fieldValueExpression(type, setterArgumentName)}; }';
}

indent.writeln(
Expand All @@ -938,6 +1030,18 @@ EncodableValue ${api.name}::WrapError(const FlutterError& error) {
indent.newln();
}

/// 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) {
return type.isNullable
? '$variable ? ${_valueType(type)}(*$variable) : std::nullopt'
: variable;
}

String _wrapResponse(Indent indent, Root root, TypeDeclaration returnType,
{String prefix = ''}) {
final String nonErrorPath;
Expand Down Expand Up @@ -1109,9 +1213,15 @@ class _HostNamedType {
final TypeDeclaration originalType;
}

/// Contains a class field and its serialization index.
class _IndexedField {
const _IndexedField(this.index, this.field);
final int index;
final NamedType field;
}

String _getCodecSerializerName(Api api) => '${api.name}CodecSerializer';

const String _pointerPrefix = 'pointer';
const String _encodablePrefix = 'encodable';

String _getArgumentName(int count, NamedType argument) =>
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'ast.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '9.1.2';
const String pigeonVersion = '9.1.3';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/mock_handler_tester/test/message.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v9.1.2), do not edit directly.
// Autogenerated from Pigeon (v9.1.3), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import

Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/mock_handler_tester/test/test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v9.1.2), do not edit directly.
// Autogenerated from Pigeon (v9.1.3), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, unnecessary_import
// ignore_for_file: avoid_relative_lib_imports
Expand Down
2 changes: 1 addition & 1 deletion ...st_plugin/android/src/main/java/com/example/alternate_language_test_plugin/CoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v9.1.2), do not edit directly.
// Autogenerated from Pigeon (v9.1.3), do not edit directly.
// See also: https://pub.dev/packages/pigeon

package com.example.alternate_language_test_plugin;
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/CoreTests.gen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v9.1.2), do not edit directly.
// Autogenerated from Pigeon (v9.1.3), do not edit directly.
// See also: https://pub.dev/packages/pigeon

#import <Foundation/Foundation.h>
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/CoreTests.gen.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// Autogenerated from Pigeon (v9.1.2), do not edit directly.
// Autogenerated from Pigeon (v9.1.3), do not edit directly.
// See also: https://pub.dev/packages/pigeon

#import "CoreTests.gen.h"
Expand Down
Loading

0 comments on commit ee37f1c

Please sign in to comment.