From 32e57bd1ae5e3d9944e5cfb0bae6095022b32b08 Mon Sep 17 00:00:00 2001 From: Alexey Edelev Date: Mon, 31 Jul 2023 18:39:26 +0200 Subject: [PATCH] Fix detecting of unused nested messages Previously if nested message was never used by the nesting class it was ignored to be generated. It was the result if invalid filtering of map messages as nested types. Use the explicit map_key() based check instead of weird and tricky one based on nesting class fields. Keep the fallback solution that uses MessageOptions when building with libprotoc versions less than 3.12. Fixes: QTBUG-115175 Change-Id: I4aa238dac3375bdcdf85e41ba9daef74474ed0fb Reviewed-by: Tatiana Borisova (cherry picked from commit 03ccff3aa79af257ab1672bdc96d3c237d80aafc) --- src/tools/qtprotoccommon/CMakeLists.txt | 1 + src/tools/qtprotoccommon/generatorcommon.cpp | 40 +++++++++---------- src/tools/qtprotoccommon/qtprotocdefs.h | 14 +++++++ .../nested/proto/nestedmessages.proto | 3 ++ .../nested/tst_protobuf_nestedtypes.cpp | 8 ++++ 5 files changed, 44 insertions(+), 22 deletions(-) create mode 100644 src/tools/qtprotoccommon/qtprotocdefs.h diff --git a/src/tools/qtprotoccommon/CMakeLists.txt b/src/tools/qtprotoccommon/CMakeLists.txt index 323f7930..86335a8e 100644 --- a/src/tools/qtprotoccommon/CMakeLists.txt +++ b/src/tools/qtprotoccommon/CMakeLists.txt @@ -2,6 +2,7 @@ # SPDX-License-Identifier: BSD-3-Clause qt_add_library(QtProtocCommon STATIC + qtprotocdefs.h baseprinter.cpp baseprinter.h descriptorprinterbase.h generatorbase.cpp generatorbase.h diff --git a/src/tools/qtprotoccommon/generatorcommon.cpp b/src/tools/qtprotoccommon/generatorcommon.cpp index e5ab51fc..f7986e7d 100644 --- a/src/tools/qtprotoccommon/generatorcommon.cpp +++ b/src/tools/qtprotoccommon/generatorcommon.cpp @@ -7,6 +7,12 @@ #include "utils.h" #include "commontemplates.h" +#include "qtprotocdefs.h" + +#ifndef HAVE_PROTOBUF_SYNC_PIPER +# include +#endif + #include #include @@ -477,41 +483,31 @@ void common::iterateNestedMessages(const Descriptor *message, int numNestedTypes = message->nested_type_count(); for (int i = 0; i < numNestedTypes; ++i) { const Descriptor *nestedMessage = message->nested_type(i); - if (message->field_count() <= 0) { +#ifdef HAVE_PROTOBUF_SYNC_PIPER + if (nestedMessage->map_key() == nullptr) { +#else + if (!nestedMessage->options().map_entry()) { +#endif callback(nestedMessage); continue; } - int numFields = message->field_count(); - for (int j = 0; j < numFields; ++j) { - const FieldDescriptor *field = message->field(j); - // Probably there is more correct way to detect map in - // nested messages. - // TODO: Have idea to make maps nested classes instead of typedefs. - if (!field->is_map() && field->message_type() == nestedMessage) { - callback(nestedMessage); - break; - } - } } } bool common::hasNestedMessages(const Descriptor *message) { int numNestedTypes = message->nested_type_count(); - int numFields = message->field_count(); - if (numNestedTypes > 0 && numFields <= 0) + if (numNestedTypes > 0 && message->field_count() == 0) return true; for (int i = 0; i < numNestedTypes; ++i) { const Descriptor *nestedMessage = message->nested_type(i); - for (int j = 0; j < numFields; ++j) { - const FieldDescriptor *field = message->field(j); - // Probably there is more correct way to detect map in - // nested messages. - // TODO: Have idea to make maps nested classes instead of typedefs. - if (!field->is_map() && field->message_type() == nestedMessage) - return true; - } +#ifdef HAVE_PROTOBUF_SYNC_PIPER + if (nestedMessage->map_key() == nullptr) +#else + if (!nestedMessage->options().map_entry()) +#endif + return true; } return false; diff --git a/src/tools/qtprotoccommon/qtprotocdefs.h b/src/tools/qtprotoccommon/qtprotocdefs.h new file mode 100644 index 00000000..c80b3776 --- /dev/null +++ b/src/tools/qtprotoccommon/qtprotocdefs.h @@ -0,0 +1,14 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#ifndef QTPROTOCDEFS_H +#define QTPROTOCDEFS_H + +#include +#if PROTOBUF_VERSION >= 3012000 +// Idicates the major libprotoc API update that happened in version 3.12 +# define HAVE_PROTOBUF_SYNC_PIPER +#endif +#include + +#endif // QTPROTOCDEFS_H diff --git a/tests/auto/protobuf/nested/proto/nestedmessages.proto b/tests/auto/protobuf/nested/proto/nestedmessages.proto index 1294f81e..9c3d24bb 100644 --- a/tests/auto/protobuf/nested/proto/nestedmessages.proto +++ b/tests/auto/protobuf/nested/proto/nestedmessages.proto @@ -10,6 +10,9 @@ message NestedFieldMessage { message NestedMessage { sint32 testFieldInt = 1; } + message UnusedNestedMessage { + sint32 testFieldInt = 1; + } NestedMessage nested = 2; } diff --git a/tests/auto/protobuf/nested/tst_protobuf_nestedtypes.cpp b/tests/auto/protobuf/nested/tst_protobuf_nestedtypes.cpp index 8d940a0d..55ee4240 100644 --- a/tests/auto/protobuf/nested/tst_protobuf_nestedtypes.cpp +++ b/tests/auto/protobuf/nested/tst_protobuf_nestedtypes.cpp @@ -26,6 +26,7 @@ private slots: void NeighborTest(); void NestedNoFieldsTest(); void NestedCyclingTest(); + void UnusedNestedMessageTest(); private: std::unique_ptr m_serializer; }; @@ -254,5 +255,12 @@ void QtProtobufNestedTest::NestedCyclingTest() test2.setTestField(test); } +void QtProtobufNestedTest::UnusedNestedMessageTest() +{ + qProtobufAssertMessagePropertyRegistered(1, "QtProtobuf::sint32", + "testFieldInt"); +} + QTEST_MAIN(QtProtobufNestedTest) #include "tst_protobuf_nestedtypes.moc"