Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unnecessary toList/fromList calls during encode/decode process #6426

Merged
merged 20 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 18.0.1

* Fixes unnecessary calls of `toList` and `fromList` when encoding/decoding data classes.
* [kotlin] Changes to some code to make it more idiomatic.
* Removes collisions with the word `list`.

## 18.0.0

* Adds message channel suffix option to all APIs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,15 @@ ArrayList<Object> toList() {
return toListResult;
}

static @NonNull MessageData fromList(@NonNull ArrayList<Object> list) {
static @NonNull MessageData fromList(@NonNull ArrayList<Object> __pigeon_list) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to be in this PR, but we should consider just inlining all of the value expressions into the setter calls so we don't need variables, and thus don't have a collision concern.

MessageData pigeonResult = new MessageData();
Object name = list.get(0);
Object name = __pigeon_list.get(0);
pigeonResult.setName((String) name);
Object description = list.get(1);
Object description = __pigeon_list.get(1);
pigeonResult.setDescription((String) description);
Object code = list.get(2);
Object code = __pigeon_list.get(2);
pigeonResult.setCode(Code.values()[(int) code]);
Object data = list.get(3);
Object data = __pigeon_list.get(3);
pigeonResult.setData((Map<String, String>) data);
return pigeonResult;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.
// Autogenerated from Pigeon, do not edit directly.
// See also: https://pub.dev/packages/pigeon
@file:Suppress("UNCHECKED_CAST", "ArrayInDataClass")

import android.util.Log
import io.flutter.plugin.common.BasicMessageChannel
Expand All @@ -17,10 +18,10 @@ private fun wrapResult(result: Any?): List<Any?> {
}

private fun wrapError(exception: Throwable): List<Any?> {
if (exception is FlutterError) {
return listOf(exception.code, exception.message, exception.details)
return if (exception is FlutterError) {
listOf(exception.code, exception.message, exception.details)
} else {
return listOf(
listOf(
exception.javaClass.simpleName,
exception.toString(),
"Cause: " + exception.cause + ", Stacktrace: " + Log.getStackTraceString(exception))
Expand Down Expand Up @@ -64,12 +65,12 @@ data class MessageData(
val data: Map<String?, String?>
) {
companion object {
@Suppress("UNCHECKED_CAST")
fun fromList(list: List<Any?>): MessageData {
val name = list[0] as String?
val description = list[1] as String?
val code = Code.ofRaw(list[2] as Int)!!
val data = list[3] as Map<String?, String?>
@Suppress("LocalVariableName")
fun fromList(__pigeon_list: List<Any?>): MessageData {
val name = __pigeon_list[0] as String?
val description = __pigeon_list[1] as String?
val code = Code.ofRaw(__pigeon_list[2] as Int)!!
val data = __pigeon_list[3] as Map<String?, String?>
return MessageData(name, description, code, data)
}
}
Expand All @@ -84,7 +85,6 @@ data class MessageData(
}
}

@Suppress("UNCHECKED_CAST")
private object ExampleHostApiCodec : StandardMessageCodec() {
override fun readValueOfType(type: Byte, buffer: ByteBuffer): Any? {
return when (type) {
Expand Down Expand Up @@ -118,7 +118,6 @@ interface ExampleHostApi {
/** The codec used by ExampleHostApi. */
val codec: MessageCodec<Any?> by lazy { ExampleHostApiCodec }
/** Sets up an instance of `ExampleHostApi` to handle messages through the `binaryMessenger`. */
@Suppress("UNCHECKED_CAST")
fun setUp(
binaryMessenger: BinaryMessenger,
api: ExampleHostApi?,
Expand All @@ -134,12 +133,12 @@ interface ExampleHostApi {
codec)
if (api != null) {
channel.setMessageHandler { _, reply ->
var wrapped: List<Any?>
try {
wrapped = listOf<Any?>(api.getHostLanguage())
} catch (exception: Throwable) {
wrapped = wrapError(exception)
}
val wrapped: List<Any?> =
try {
listOf<Any?>(api.getHostLanguage())
} catch (exception: Throwable) {
wrapError(exception)
}
reply.reply(wrapped)
}
} else {
Expand All @@ -155,14 +154,14 @@ interface ExampleHostApi {
if (api != null) {
channel.setMessageHandler { message, reply ->
val args = message as List<Any?>
val aArg = args[0].let { if (it is Int) it.toLong() else it as Long }
val bArg = args[1].let { if (it is Int) it.toLong() else it as Long }
var wrapped: List<Any?>
try {
wrapped = listOf<Any?>(api.add(aArg, bArg))
} catch (exception: Throwable) {
wrapped = wrapError(exception)
}
val aArg = args[0].let { num -> if (num is Int) num.toLong() else num as Long }
val bArg = args[1].let { num -> if (num is Int) num.toLong() else num as Long }
val wrapped: List<Any?> =
try {
listOf<Any?>(api.add(aArg, bArg))
} catch (exception: Throwable) {
wrapError(exception)
}
reply.reply(wrapped)
}
} else {
Expand Down Expand Up @@ -197,7 +196,6 @@ interface ExampleHostApi {
}
}
/** Generated class from Pigeon that represents Flutter messages that can be called from Kotlin. */
@Suppress("UNCHECKED_CAST")
class MessageFlutterApi(
private val binaryMessenger: BinaryMessenger,
private val messageChannelSuffix: String = ""
Expand Down
11 changes: 6 additions & 5 deletions packages/pigeon/example/app/ios/Runner/Messages.g.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@ struct MessageData {
var code: Code
var data: [String?: String?]

static func fromList(_ list: [Any?]) -> MessageData? {
let name: String? = nilOrValue(list[0])
let description: String? = nilOrValue(list[1])
let code = Code(rawValue: list[2] as! Int)!
let data = list[3] as! [String?: String?]
// swift-format-ignore: AlwaysUseLowerCamelCase
static func fromList(_ __pigeon_list: [Any?]) -> MessageData? {
let name: String? = nilOrValue(__pigeon_list[0])
let description: String? = nilOrValue(__pigeon_list[1])
let code = Code(rawValue: __pigeon_list[2] as! Int)!
let data = __pigeon_list[3] as! [String?: String?]

return MessageData(
name: name,
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ class TypeDeclaration {
@override
String toString() {
final String typeArgumentsStr =
typeArguments.isEmpty ? '' : 'typeArguments:$typeArguments';
typeArguments.isEmpty ? '' : ' typeArguments:$typeArguments';
return '(TypeDeclaration baseName:$baseName isNullable:$isNullable$typeArgumentsStr isEnum:$isEnum isClass:$isClass isProxyApi:$isProxyApi)';
}
}
Expand Down
63 changes: 34 additions & 29 deletions packages/pigeon/lib/cpp_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -787,8 +787,12 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
final HostDatatype hostDatatype =
getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
final String encodableValue = _wrappedHostApiArgumentExpression(
root, _makeInstanceVariableName(field), field.type, hostDatatype,
preSerializeClasses: true);
root,
_makeInstanceVariableName(field),
field.type,
hostDatatype,
true,
);
indent.writeln('list.push_back($encodableValue);');
}
indent.writeln('return list;');
Expand All @@ -815,11 +819,8 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
} else {
final HostDatatype hostDatatype =
getFieldHostDatatype(field, _shortBaseCppTypeForBuiltinDartType);
if (!hostDatatype.isBuiltin &&
root.classes
.map((Class x) => x.name)
.contains(field.type.baseName)) {
return '${hostDatatype.datatype}::FromEncodableList(std::get<EncodableList>($encodable))';
if (field.type.isClass) {
return _classReferenceFromEncodableValue(hostDatatype, encodable);
} else {
return 'std::get<${hostDatatype.datatype}>($encodable)';
}
Expand Down Expand Up @@ -960,8 +961,12 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
indent.addScoped('EncodableValue(EncodableList{', '});', () {
for (final _HostNamedType param in hostParameters) {
final String encodedArgument = _wrappedHostApiArgumentExpression(
root, param.name, param.originalType, param.hostType,
preSerializeClasses: false);
root,
param.name,
param.originalType,
param.hostType,
false,
);
indent.writeln('$encodedArgument,');
}
});
Expand Down Expand Up @@ -1452,27 +1457,20 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';

/// Returns the expression to create an EncodableValue from a host API argument
/// with the given [variableName] and types.
///
/// If [preSerializeClasses] is true, custom classes will be returned as
/// encodable lists rather than CustomEncodableValues; see
/// https://github.com/flutter/flutter/issues/119351 for why this is currently
/// needed.
String _wrappedHostApiArgumentExpression(Root root, String variableName,
TypeDeclaration dartType, HostDatatype hostType,
{required bool preSerializeClasses}) {
String _wrappedHostApiArgumentExpression(
Root root,
String variableName,
TypeDeclaration dartType,
HostDatatype hostType,
bool isNestedClass,
) {
final String encodableValue;
if (!hostType.isBuiltin &&
root.classes.any((Class c) => c.name == dartType.baseName)) {
if (preSerializeClasses) {
final String operator =
hostType.isNullable || _isPointerField(hostType) ? '->' : '.';
encodableValue =
'EncodableValue($variableName${operator}ToEncodableList())';
} else {
final String nonNullValue =
hostType.isNullable ? '*$variableName' : variableName;
encodableValue = 'CustomEncodableValue($nonNullValue)';
}
final String nonNullValue = hostType.isNullable || isNestedClass
? '*$variableName'
: variableName;
encodableValue = 'CustomEncodableValue($nonNullValue)';
} else if (!hostType.isBuiltin &&
root.enums.any((Enum e) => e.name == dartType.baseName)) {
final String nonNullValue =
Expand Down Expand Up @@ -1537,7 +1535,7 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
}
} else {
indent.writeln(
'const auto* $argName = &(std::any_cast<const ${hostType.datatype}&>(std::get<CustomEncodableValue>($encodableArgName)));');
'const auto* $argName = &(${_classReferenceFromEncodableValue(hostType, encodableArgName)});');
}
} else {
// Non-nullable arguments are either passed by value or reference, but the
Expand All @@ -1562,7 +1560,7 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
'const ${hostType.datatype}& $argName = (${hostType.datatype})$encodableArgName.LongValue();');
} else {
indent.writeln(
'const auto& $argName = std::any_cast<const ${hostType.datatype}&>(std::get<CustomEncodableValue>($encodableArgName));');
'const auto& $argName = ${_classReferenceFromEncodableValue(hostType, encodableArgName)};');
}
}
}
Expand All @@ -1573,6 +1571,13 @@ ${prefix}reply(EncodableValue(std::move(wrapped)));''';
String? _shortBaseCppTypeForBuiltinDartType(TypeDeclaration type) {
return _baseCppTypeForBuiltinDartType(type, includeFlutterNamespace: false);
}

/// Returns the code to extract a `const {type.datatype}&` from an EncodableValue
/// variable [variableName] that contains an instance of [type].
String _classReferenceFromEncodableValue(
HostDatatype type, String variableName) {
return 'std::any_cast<const ${type.datatype}&>(std::get<CustomEncodableValue>($variableName))';
}
}

/// Contains information about a host function argument.
Expand Down