Skip to content

Commit

Permalink
[pigeon] Fix handling of null class args in C++ (flutter#6881)
Browse files Browse the repository at this point in the history 
The code generated for Windows (C++) crashes when `null` is passed for a nullable, class-type parameter to a Pigeon method. This fixes that generation, and adds an integration test covering this case since it was missing from the list of types where we echo a `null` in integration tests to catch this kind of issue.

Fixes flutter/flutter#149174
  • Loading branch information
@stuartmorgan @arc-yong
stuartmorgan authored and arc-yong committed Jun 14, 2024
1 parent 4400fcb commit f1a69eb
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 110 deletions.
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 20.0.1

* [cpp] Fixes handling of null class arguments.

## 20.0.0

* Moves all codec logic to single custom codec per file.
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/cpp_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,7 @@ if (!$encodableArgName.IsNull()) {
}''');
} else {
indent.writeln(
'const auto* $argName = &(${_classReferenceFromEncodableValue(hostType, encodableArgName)});');
'const auto* $argName = $encodableArgName.IsNull() ? nullptr : &(${_classReferenceFromEncodableValue(hostType, encodableArgName)});');
}
} else {
// Non-nullable arguments are either passed by value or reference, but the
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 @@ -13,7 +13,7 @@ import 'ast.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '20.0.0';
const String pigeonVersion = '20.0.1';

/// Prefix for all local variables in methods.
///
Expand Down
9 changes: 9 additions & 0 deletions packages/pigeon/platform_tests/shared_test_plugin_code/lib/integration_tests.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,15 @@ void runPigeonIntegrationTests(TargetGenerator targetGenerator) {
expect(echoEnum, sentEnum);
});

testWidgets('null classes serialize and deserialize correctly',
(WidgetTester _) async {
final HostIntegrationCoreApi api = HostIntegrationCoreApi();

final AllNullableTypes? echoObject = await api.echoAllNullableTypes(null);

expect(echoObject, isNull);
});

testWidgets('optional nullable parameter', (WidgetTester _) async {
final HostIntegrationCoreApi api = HostIntegrationCoreApi();

Expand Down
232 changes: 126 additions & 106 deletions packages/pigeon/platform_tests/test_plugin/windows/pigeon/core_tests.gen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2261,9 +2261,11 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
ErrorOr<std::optional<AllNullableTypes>> output =
api->EchoAllNullableTypes(everything_arg);
if (output.has_error()) {
Expand Down Expand Up @@ -2295,35 +2297,38 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
prepended_suffix,
&GetCodec());
if (api != nullptr) {
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
ErrorOr<std::optional<AllNullableTypesWithoutRecursion>> output =
api->EchoAllNullableTypesWithoutRecursion(everything_arg);
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(
CustomEncodableValue(std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
channel.SetMessageHandler([api](
const EncodableValue& message,
const flutter::MessageReply<EncodableValue>&
reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
ErrorOr<std::optional<AllNullableTypesWithoutRecursion>> output =
api->EchoAllNullableTypesWithoutRecursion(everything_arg);
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(
CustomEncodableValue(std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
} else {
channel.SetMessageHandler(nullptr);
}
Expand Down Expand Up @@ -3459,9 +3464,11 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->EchoAsyncNullableAllNullableTypes(
everything_arg,
[reply](ErrorOr<std::optional<AllNullableTypes>>&& output) {
Expand Down Expand Up @@ -3495,39 +3502,41 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
prepended_suffix,
&GetCodec());
if (api != nullptr) {
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->EchoAsyncNullableAllNullableTypesWithoutRecursion(
everything_arg,
[reply](
ErrorOr<std::optional<AllNullableTypesWithoutRecursion>>&&
channel.SetMessageHandler([api](
const EncodableValue& message,
const flutter::MessageReply<EncodableValue>&
reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->EchoAsyncNullableAllNullableTypesWithoutRecursion(
everything_arg,
[reply](ErrorOr<std::optional<AllNullableTypesWithoutRecursion>>&&
output) {
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(CustomEncodableValue(
std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
});
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(
CustomEncodableValue(std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
});
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
} else {
channel.SetMessageHandler(nullptr);
}
Expand Down Expand Up @@ -4057,9 +4066,11 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->CallFlutterEchoAllNullableTypes(
everything_arg,
[reply](ErrorOr<std::optional<AllNullableTypes>>&& output) {
Expand Down Expand Up @@ -4142,39 +4153,41 @@ void HostIntegrationCoreApi::SetUp(flutter::BinaryMessenger* binary_messenger,
prepended_suffix,
&GetCodec());
if (api != nullptr) {
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
&(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->CallFlutterEchoAllNullableTypesWithoutRecursion(
everything_arg,
[reply](
ErrorOr<std::optional<AllNullableTypesWithoutRecursion>>&&
channel.SetMessageHandler([api](
const EncodableValue& message,
const flutter::MessageReply<EncodableValue>&
reply) {
try {
const auto& args = std::get<EncodableList>(message);
const auto& encodable_everything_arg = args.at(0);
const auto* everything_arg =
encodable_everything_arg.IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
encodable_everything_arg)));
api->CallFlutterEchoAllNullableTypesWithoutRecursion(
everything_arg,
[reply](ErrorOr<std::optional<AllNullableTypesWithoutRecursion>>&&
output) {
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(CustomEncodableValue(
std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
});
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
if (output.has_error()) {
reply(WrapError(output.error()));
return;
}
EncodableList wrapped;
auto output_optional = std::move(output).TakeValue();
if (output_optional) {
wrapped.push_back(
CustomEncodableValue(std::move(output_optional).value()));
} else {
wrapped.push_back(EncodableValue());
}
reply(EncodableValue(std::move(wrapped)));
});
} catch (const std::exception& exception) {
reply(WrapError(exception.what()));
}
});
} else {
channel.SetMessageHandler(nullptr);
}
Expand Down Expand Up @@ -5108,8 +5121,12 @@ void FlutterIntegrationCoreApi::EchoAllNullableTypes(
std::get<std::string>(list_return_value->at(1)),
list_return_value->at(2)));
} else {
const auto* return_value = &(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(list_return_value->at(0))));
const auto* return_value =
list_return_value->at(0).IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypes&>(
std::get<CustomEncodableValue>(
list_return_value->at(0))));
on_success(return_value);
}
} else {
Expand Down Expand Up @@ -5191,8 +5208,11 @@ void FlutterIntegrationCoreApi::EchoAllNullableTypesWithoutRecursion(
list_return_value->at(2)));
} else {
const auto* return_value =
&(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(list_return_value->at(0))));
list_return_value->at(0).IsNull()
? nullptr
: &(std::any_cast<const AllNullableTypesWithoutRecursion&>(
std::get<CustomEncodableValue>(
list_return_value->at(0))));
on_success(return_value);
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: pigeon
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
repository: https://github.com/flutter/packages/tree/main/packages/pigeon
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+pigeon%22
version: 20.0.0 # This must match the version in lib/generator_tools.dart
version: 20.0.1 # This must match the version in lib/generator_tools.dart

environment:
sdk: ^3.2.0
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/test/cpp_generator_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ void main() {
expect(
code,
contains(
'const auto* an_object_arg = &(std::any_cast<const ParameterObject&>(std::get<CustomEncodableValue>(encodable_an_object_arg)));'));
'const auto* an_object_arg = encodable_an_object_arg.IsNull() ? nullptr : &(std::any_cast<const ParameterObject&>(std::get<CustomEncodableValue>(encodable_an_object_arg)));'));
// "Object" requires no extraction at all since it has to use
// EncodableValue directly.
expect(
Expand Down

0 comments on commit f1a69eb

Please sign in to comment.