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

feat, wip: support for optional #246

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ set(CMAKE_POSITION_INDEPENDENT_CODE ${${QT_VERSIONED_PREFIX}_POSITION_INDEPENDEN
if(UNIX)
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
# using Clang
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wno-pessimizing-move -Wno-mismatched-tags -Wno-unused-private-field -Wno-self-assign-overloaded")
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please move this fix to a separate patch set. I also suggest introducing something like 'QTPROTOBUF_DEVELOPER_BUILD' or 'QTPROTOBUF_WARNINGS_AS_ERROR' cache variable to opt-in this functionality.

Copy link
Author

Choose a reason for hiding this comment

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

oops, didn't mean to commit this. was a local workaround for the wall werror failing to build on my machine.

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-pessimizing-move -Wno-mismatched-tags -Wno-unused-private-field -Wno-self-assign-overloaded")
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
# using GCC
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wno-error=deprecated-declarations")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=deprecated-declarations")
endif()
elseif(WIN32)
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
Expand Down
3 changes: 2 additions & 1 deletion cmake/QtProtobufGen.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ function(qtprotobuf_link_target TARGET GENERATED_TARGET)
endfunction()

function(qtprotobuf_generate)
set(options MULTI QML COMMENTS FOLDER FIELDENUM)
set(options MULTI QML COMMENTS FOLDER FIELDENUM ALLOW_PROTO3_OPTIONAL)
set(oneValueArgs OUT_DIR TARGET GENERATED_TARGET EXTRA_NAMESPACE)
set(multiValueArgs GENERATED_HEADERS EXCLUDE_HEADERS PROTO_FILES PROTO_INCLUDES)
cmake_parse_arguments(qtprotobuf_generate "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
Expand Down Expand Up @@ -143,6 +143,7 @@ function(qtprotobuf_generate)
OUTPUT ${GENERATED_SOURCES_FULL} ${GENERATED_HEADERS_FULL}
COMMAND ${PROTOC_COMMAND}
--plugin=protoc-gen-qtprotobufgen=${QT_PROTOBUF_EXECUTABLE}
$<IF:$<BOOL:${qtprotobuf_generate_ALLOW_PROTO3_OPTIONAL}>,--experimental_allow_proto3_optional,>
--qtprotobufgen_out=${OUT_DIR}
${PROTO_INCLUDES}
${qtprotobuf_generate_PROTO_FILES}
Expand Down
1 change: 1 addition & 0 deletions cmake/QtProtobufInternalHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ function(qt_protobuf_internal_add_test)
PROTO_FILES ${proto_files}
GENERATED_HEADERS ${add_test_target_GENERATED_HEADERS}
EXCLUDE_HEADERS ${add_test_target_EXCLUDE_HEADERS}
ALLOW_PROTO3_OPTIONAL
${EXTRA_OPTIONS}
PROTO_INCLUDES ${add_test_target_PROTO_INCLUDES})

Expand Down
2 changes: 2 additions & 0 deletions src/generator/generatorbase.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class GeneratorBase: public ::google::protobuf::compiler::CodeGenerator
GeneratorBase(Mode mode);
virtual ~GeneratorBase() = default;

::google::protobuf::uint64 GetSupportedFeatures() const override { return FEATURE_PROTO3_OPTIONAL; }

virtual bool GenerateAll(const std::vector<const ::google::protobuf::FileDescriptor *> &files,
const std::string &parameter,
::google::protobuf::compiler::GeneratorContext *generatorContext,
Expand Down
16 changes: 16 additions & 0 deletions src/generator/generatorcommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,22 @@ PropertyMap common::producePropertyMap(const FieldDescriptor *field, const Descr
return propertyMap;
}

PropertyMap common::produceOptionalPropertyMap(const FieldDescriptor *field, const Descriptor *scope)
{
PropertyMap map = producePropertyMap(field, scope);

map["property_type"] = "bool";
map["getter_type"] = "bool";
map["setter_type"] = "bool";
map["scope_type"] = "bool";
map["initializer"] = "true";
map["scriptable"] = "true";
map["property_name"] = "has_" + map["property_name"];
map["property_name_cap"] = utils::upperCaseName(map["property_name"]);

return map;
}

std::string common::qualifiedName(const std::string &name)
{
std::string fieldName(name);
Expand Down
1 change: 1 addition & 0 deletions src/generator/generatorcommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ struct common {
static TypeMap produceSimpleTypeMap(::google::protobuf::FieldDescriptor::Type type);
static TypeMap produceTypeMap(const ::google::protobuf::FieldDescriptor *field, const ::google::protobuf::Descriptor *scope);
static PropertyMap producePropertyMap(const ::google::protobuf::FieldDescriptor *field, const ::google::protobuf::Descriptor *scope);
static PropertyMap produceOptionalPropertyMap(const ::google::protobuf::FieldDescriptor *field, const ::google::protobuf::Descriptor *scope);
static std::string qualifiedName(const std::string &name);
static bool isLocalEnum(const ::google::protobuf::EnumDescriptor *type, const google::protobuf::Descriptor *scope);
static EnumVisibility enumVisibility(const ::google::protobuf::EnumDescriptor *type, const ::google::protobuf::Descriptor *scope);
Expand Down
36 changes: 35 additions & 1 deletion src/generator/messagedeclarationprinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,16 @@ void MessageDeclarationPrinter::printProperties()
}
}

for (int i = 0; i < mDescriptor->field_count(); i++) {
const FieldDescriptor *field = mDescriptor->field(i);

if (!field->is_optional()) {
continue;
}

mPrinter->Print(common::produceOptionalPropertyMap(field, mDescriptor), Templates::ReadOnlyPropertyTemplate);
}

Outdent();
}

Expand All @@ -263,6 +273,12 @@ void MessageDeclarationPrinter::printGetters()
mPrinter->Print(propertyMap, Templates::GetterTemplate);
}

if (field->is_optional()) {
auto optionalMap = common::produceOptionalPropertyMap(field, mDescriptor);

mPrinter->Print(optionalMap, Templates::GetterTemplate);
}

if (field->is_repeated()) {
mPrinter->Print(propertyMap, Templates::GetterContainerExtraTemplate);
if (field->type() == FieldDescriptor::TYPE_MESSAGE && !field->is_map()
Expand Down Expand Up @@ -292,6 +308,11 @@ void MessageDeclarationPrinter::printSetters()
break;
default:
mPrinter->Print(propertyMap, Templates::SetterTemplate);
if (field->is_optional()) {
mPrinter->Print(propertyMap, Templates::EndOptionalSetterTemplate);
} else {
mPrinter->Print(propertyMap, Templates::EndSetterTemplate);
}
break;
}
});
Expand Down Expand Up @@ -324,7 +345,12 @@ void MessageDeclarationPrinter::printSignals()
{
Indent();
for (int i = 0; i < mDescriptor->field_count(); i++) {
mPrinter->Print(common::producePropertyMap(mDescriptor->field(i), mDescriptor), Templates::SignalTemplate);
const auto field = mDescriptor->field(i);
mPrinter->Print(common::producePropertyMap(field, mDescriptor), Templates::SignalTemplate);

if (field->is_optional()) {
mPrinter->Print(common::produceOptionalPropertyMap(field, mDescriptor), Templates::SignalTemplate);
}
}
Outdent();
}
Expand All @@ -337,6 +363,11 @@ void MessageDeclarationPrinter::printPrivateMethods()
if (common::hasQmlAlias(field)) {
mPrinter->Print(propertyMap, Templates::NonScriptableGetterTemplate);
mPrinter->Print(propertyMap, Templates::NonScriptableSetterTemplate);
if (field->is_optional()) {
mPrinter->Print(propertyMap, Templates::EndOptionalSetterTemplate);
} else {
mPrinter->Print(propertyMap, Templates::EndSetterTemplate);
}
}
});
Outdent();
Expand Down Expand Up @@ -434,6 +465,9 @@ void MessageDeclarationPrinter::printClassMembers()
} else {
mPrinter->Print(propertyMap, Templates::MemberTemplate);
}
if (field->is_optional()) {
mPrinter->Print(common::produceOptionalPropertyMap(field, mDescriptor), Templates::MemberTemplate);
}
});
Outdent();
}
Expand Down
21 changes: 21 additions & 0 deletions src/generator/messagedefinitionprinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ void MessageDefinitionPrinter::printInitializationList(int fieldCount)
}
}
}

if (field->is_optional()) {
mPrinter->Print(common::produceOptionalPropertyMap(field, mDescriptor), Templates::PropertyDefaultInitializerTemplate);
}
}
}

Expand Down Expand Up @@ -372,16 +376,33 @@ void MessageDefinitionPrinter::printGetters()
case FieldDescriptor::TYPE_MESSAGE:
if (!field->is_map() && !field->is_repeated() && !common::isQtType(field)) {
mPrinter->Print(propertyMap, Templates::SetterPrivateTemplateDefinitionMessageType);
if (field->is_optional()) {
mPrinter->Print(propertyMap, Templates::EndOptionalSetterTemplate);
} else {
mPrinter->Print(propertyMap, Templates::EndSetterTemplate);
}

mPrinter->Print(propertyMap, Templates::SetterTemplateDefinitionMessageType);
} else {
mPrinter->Print(propertyMap, Templates::SetterTemplateDefinitionComplexType);
}
if (field->is_optional()) {
mPrinter->Print(propertyMap, Templates::EndOptionalSetterTemplate);
} else {
mPrinter->Print(propertyMap, Templates::EndSetterTemplate);
}
break;
case FieldDescriptor::FieldDescriptor::TYPE_STRING:
case FieldDescriptor::FieldDescriptor::TYPE_BYTES:
mPrinter->Print(propertyMap, Templates::SetterTemplateDefinitionComplexType);
if (field->is_optional()) {
mPrinter->Print(propertyMap, Templates::EndOptionalSetterTemplate);
} else {
mPrinter->Print(propertyMap, Templates::EndSetterTemplate);
}
break;
default:
mPrinter->Print(propertyMap, "/* default ending */");
break;
}
});
Expand Down
24 changes: 14 additions & 10 deletions src/generator/templates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const char *Templates::ProtoClassDeclarationBeginTemplate = "\nclass $classname$
" Q_DECLARE_PROTOBUF_SERIALIZERS($classname$)\n";

const char *Templates::PropertyTemplate = "Q_PROPERTY($property_type$ $property_name$ READ $property_name$ WRITE set$property_name_cap$ NOTIFY $property_name$Changed SCRIPTABLE $scriptable$)\n";
const char *Templates::ReadOnlyPropertyTemplate = "Q_PROPERTY($property_type$ $property_name$ READ $property_name$ NOTIFY $property_name$Changed SCRIPTABLE $scriptable$)\n";
const char *Templates::RepeatedPropertyTemplate = "Q_PROPERTY($property_list_type$ $property_name$ READ $property_name$ WRITE set$property_name_cap$ NOTIFY $property_name$Changed SCRIPTABLE $scriptable$)\n";
const char *Templates::RepeatedMessagePropertyTemplate = "Q_PROPERTY($property_list_type$ $property_name$Data READ $property_name$ WRITE set$property_name_cap$ NOTIFY $property_name$Changed SCRIPTABLE $scriptable$)\n";
const char *Templates::NonScriptablePropertyTemplate = "Q_PROPERTY($property_type$ $property_name$_p READ $property_name$ WRITE set$property_name_cap$ NOTIFY $property_name$Changed SCRIPTABLE false)\n";
Expand Down Expand Up @@ -208,37 +209,40 @@ const char *Templates::SetterPrivateTemplateDefinitionMessageType = "void $class
" if (m_$property_name$.get() != $property_name$) {\n"
" m_$property_name$.reset($property_name$);\n"
" $property_name$Changed();\n"
" }\n"
"}\n\n";
" }\n";

const char *Templates::SetterTemplateDeclarationMessageType = "void set$property_name_cap$(const $setter_type$ &$property_name$);\n";
const char *Templates::SetterTemplateDefinitionMessageType = "void $classname$::set$property_name_cap$(const $setter_type$ &$property_name$)\n{\n"
" if (*m_$property_name$ != $property_name$) {\n"
" *m_$property_name$ = $property_name$;\n"
" $property_name$Changed();\n"
" }\n"
"}\n\n";
" }\n";

const char *Templates::SetterTemplateDeclarationComplexType = "void set$property_name_cap$(const $setter_type$ &$property_name$);\n";
const char *Templates::SetterTemplateDefinitionComplexType = "void $classname$::set$property_name_cap$(const $setter_type$ &$property_name$)\n{\n"
" if (m_$property_name$ != $property_name$) {\n"
" m_$property_name$ = $property_name$;\n"
" $property_name$Changed();\n"
" }\n"
"}\n\n";
" }\n";

const char *Templates::SetterTemplate = "void set$property_name_cap$(const $setter_type$ &$property_name$) {\n"
" if (m_$property_name$ != $property_name$) {\n"
" m_$property_name$ = $property_name$;\n"
" $property_name$Changed();\n"
" }\n"
"}\n\n";
" }\n";
const char *Templates::NonScriptableSetterTemplate = "void set$property_name_cap$_p(const $qml_alias_type$ &$property_name$) {\n"
" if (m_$property_name$ != $property_name$) {\n"
" m_$property_name$ = $property_name$;\n"
" $property_name$Changed();\n"
" }\n"
"}\n\n";
" }\n";

const char *Templates::EndSetterTemplate = "}\n\n";

const char *Templates::EndOptionalSetterTemplate = " if (!m_has_$property_name$) {\n"
" m_has_$property_name$ = true;\n"
" Q_EMIT has_$property_name$Changed();\n"
" }\n"
"}\n\n";

const char *Templates::SignalsBlockTemplate = "\nsignals:\n";
const char *Templates::SignalTemplate = "void $property_name$Changed();\n";
Expand Down
3 changes: 3 additions & 0 deletions src/generator/templates.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class Templates {
static const char *QObjectMacro;

static const char *PropertyTemplate;
static const char *ReadOnlyPropertyTemplate;
static const char *RepeatedPropertyTemplate;
static const char *RepeatedMessagePropertyTemplate;
static const char *NonScriptablePropertyTemplate;
Expand Down Expand Up @@ -147,6 +148,8 @@ class Templates {
static const char *SetterTemplateDefinitionComplexType;
static const char *SetterTemplate;
static const char *NonScriptableSetterTemplate;
static const char *EndSetterTemplate;
static const char *EndOptionalSetterTemplate;
static const char *SignalsBlockTemplate;
static const char *SignalTemplate;
static const char *FieldsOrderingContainerTemplate;
Expand Down
8 changes: 8 additions & 0 deletions tests/test_protobuf/proto/simpletest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -634,3 +634,11 @@ message LowerCaseFieldMessageName {
message NoPackageMessage {
SimpleIntMessageExt testField = 1;
}

message OptionalScalarMessage {
optional int32 testField = 1;
}

message OptionalNonScalarMessage {
optional EmptyMessage testField = 1;
}