From db2c50176634fc2d9cf32a6250bba7793c5536cb Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Thu, 28 Mar 2024 17:53:49 -0500 Subject: [PATCH 01/19] vwrite for struct --- dds/DCPS/ValueDispatcher.h | 8 ++- dds/idl/value_writer_generator.cpp | 103 ++++++++++++++++++----------- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/dds/DCPS/ValueDispatcher.h b/dds/DCPS/ValueDispatcher.h index f09587576bb..cffd4011c46 100644 --- a/dds/DCPS/ValueDispatcher.h +++ b/dds/DCPS/ValueDispatcher.h @@ -22,7 +22,7 @@ struct OpenDDS_Dcps_Export ValueDispatcher { virtual void delete_value(void* data) const = 0; virtual bool read(ValueReader& value_reader, void* data) const = 0; - virtual bool write(ValueWriter& value_writer, const void* data) const = 0; + virtual bool write(ValueWriter& value_writer, const void* data, bool key_only) const = 0; virtual DDS::InstanceHandle_t register_instance_helper(DDS::DataWriter* dw, const void* data) const = 0; virtual DDS::ReturnCode_t write_helper(DDS::DataWriter* dw, const void* data, DDS::InstanceHandle_t inst) const = 0; @@ -50,8 +50,12 @@ struct ValueDispatcher_T : public virtual ValueDispatcher { return vread(value_reader, *static_cast(data)); } - virtual bool write(ValueWriter& value_writer, const void* data) const + // Don't have to worry about NestedKeyOnly here since this is only called for topic type. + virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const { + if (key_only) { + return vwrite(value_writer, *static_cast*>(data)); + } return vwrite(value_writer, *static_cast(data)); } diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index 62e31306f8c..96a0c5d0a7b 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -18,7 +18,8 @@ using namespace AstTypeClassification; namespace { - void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, int level = 1); +void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, + int level = 1, FieldFilter field_filter = FieldFilter_All); std::string primitive_type(AST_PredefinedType::PredefinedType pt) { @@ -175,7 +176,7 @@ namespace { indent << "}\n"; } - void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, int level) + void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, int level, FieldFilter field_filter) { AST_Type* const actual = resolveActualType(type); @@ -214,8 +215,22 @@ namespace { "}\n"; } else { + const std::string type_name = scoped(type->name()); + std::string value_expr = expression; + switch (field_filter) { + case FieldFilter_NestedKeyOnly: + value_expr = "nested_key_only_value(" + expression + ")"; + be_global->impl_ << + "const NestedKeyOnly " << value_expr << ";\n"; + break; + case FieldFilter_KeyOnly: + value_expr = "key_only_value(" << expression << ")"; + be_global->impl_ << + "const KeyOnly " << value_expr << ";\n"; + break; + } be_global->impl_ << - "if (!vwrite(value_writer, " << expression << ")) {\n" + "if (!vwrite(value_writer, " << value_expr << ")) {\n" " return false;\n" "}\n"; } @@ -275,6 +290,45 @@ bool value_writer_generator::gen_typedef(AST_Typedef*, return true; } +bool value_writer_generator::gen_struct_i(AST_Structure* node, + const std::string type_name, + bool use_cxx11, + ExtensibilityKind ek, + FieldFilter field_filter) +{ + Function write("vwrite", "bool"); + write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); + write.addArg("value", "const NestedKeyOnly&"); + write.endArgs(); + + const Fields fields(node, field_filter); + be_global->impl_ << + " if (!value_writer.begin_struct(" << extensibility_kind(ek) << ")) {\n" + " return false;\n" + " }\n"; + for (Fields::Iterator i = fields.begin(); i != fields.end(); ++i) { + AST_Field* const field = *i; + const std::string field_name = field->local_name()->get_string(); + const std::string idl_name = canonical_name(field); + const OpenDDS::XTypes::MemberId id = be_global->get_id(field); + const bool must_understand = be_global->is_effective_must_understand(field); + // TODO: Update the arguments for MemberParam when @optional is available. + be_global->impl_ << + " if (!value_writer.begin_struct_member(MemberParam(" << id << ", " << + (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" + " return false;\n" + " }\n"; + generate_write("value." + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i"); + be_global->impl_ << + " if (!value_writer.end_struct_member()) {\n" + " return false;\n" + " }\n"; + } + be_global->impl_ << + " return value_writer.end_struct();\n"; + return true; +} + bool value_writer_generator::gen_struct(AST_Structure* node, UTL_ScopedName* name, const std::vector& fields, @@ -285,44 +339,19 @@ bool value_writer_generator::gen_struct(AST_Structure* node, const std::string type_name = scoped(name); const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; - const std::string accessor_suffix = use_cxx11 ? "()" : ""; - - { - NamespaceGuard guard; + const ExtensibilityKind ek = be_global->extensibility(node); - Function write("vwrite", "bool"); - write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const " + type_name + "&"); - write.endArgs(); + NamespaceGuard guard; + if (!gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_All) || + !gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_NestedKeyOnly)) { + return false; + } - const ExtensibilityKind ek = be_global->extensibility(node); - be_global->impl_ << - " if (!value_writer.begin_struct(" << extensibility_kind(ek) << ")) {\n" - " return false;\n" - " }\n"; - for (std::vector::const_iterator pos = fields.begin(), limit = fields.end(); - pos != limit; ++pos) { - AST_Field* const field = *pos; - const std::string field_name = field->local_name()->get_string(); - const std::string idl_name = canonical_name(field); - // TODO: Update the arguments when @optional is available. - const OpenDDS::XTypes::MemberId id = be_global->get_id(field); - const bool must_understand = be_global->is_effectively_must_understand(field); - be_global->impl_ << - " if (!value_writer.begin_struct_member(MemberParam(" << id << ", " << - (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" - " return false;\n" - " }\n"; - generate_write("value." + field_name + accessor_suffix, field->field_type(), "i"); - be_global->impl_ << - " if (!value_writer.end_struct_member()) {\n" - " return false;\n" - " }\n"; + if (be_global->is_topic_type(node)) { + if (!gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_KeyOnly)) { + return false; } - be_global->impl_ << - " return value_writer.end_struct();\n"; } - return true; } From da01538f81a9c9ae7798720a856a2b918898ac2b Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Fri, 29 Mar 2024 12:48:53 -0500 Subject: [PATCH 02/19] vwrite for keyonly union --- dds/idl/value_writer_generator.cpp | 158 ++++++++++++++++------------- 1 file changed, 88 insertions(+), 70 deletions(-) diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index 96a0c5d0a7b..d6e1e0c18f0 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -260,6 +260,84 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st " }\n"; return ""; } + + bool gen_struct_i(AST_Structure* node, const std::string type_name, + bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) + { + Function write("vwrite", "bool"); + write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); + write.addArg("value", "const NestedKeyOnly&"); + write.endArgs(); + + const Fields fields(node, field_filter); + be_global->impl_ << + " if (!value_writer.begin_struct(" << extensibility_kind(ek) << ")) {\n" + " return false;\n" + " }\n"; + for (Fields::Iterator i = fields.begin(); i != fields.end(); ++i) { + AST_Field* const field = *i; + const std::string field_name = field->local_name()->get_string(); + const std::string idl_name = canonical_name(field); + const OpenDDS::XTypes::MemberId id = be_global->get_id(field); + const bool must_understand = be_global->is_effective_must_understand(field); + // TODO: Update the arguments for MemberParam when @optional is available. + be_global->impl_ << + " if (!value_writer.begin_struct_member(MemberParam(" << id << ", " << + (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" + " return false;\n" + " }\n"; + generate_write("value." + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i"); + be_global->impl_ << + " if (!value_writer.end_struct_member()) {\n" + " return false;\n" + " }\n"; + } + be_global->impl_ << + " return value_writer.end_struct();\n"; + return true; + } + + bool gen_union_i(AST_Union* u, const std::vector& branches, + AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) + { + Function write("vwrite", "bool"); + write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); + write.addArg("value", "const " + type_name + "&"); + write.endArgs(); + + be_global->impl_ << + " if (!value_writer.begin_union(" << extensibility_kind(ek) << ")) {\n" + " return false;\n" + " }\n"; + + const bool write_disc = be_global->union_discriminator_is_key(u) + || filter_kind == FieldFilter_NestedKeyOnly + || filter_kind == FieldFilter_All; + + if (write_disc) { + const bool must_understand = be_global->is_effectively_must_understand(discriminator); + be_global->impl_ << + " if (!value_writer.begin_discriminator(MemberParam(0, " << + (must_understand ? "true" : "false") << "))) {\n" + " return false;\n" + " }\n"; + generate_write("value._d()" , discriminator, "i", 1, filter_kind); + be_global->impl_ << + " if (!value_writer.end_discriminator()) {\n" + " return false;\n" + " }\n"; + } + + if (filter_kind == FieldFilter_All) { + generateSwitchForUnion(u, "value._d()", branch_helper, branches, + discriminator, "", "", type_name.c_str(), + false, false); + } + + be_global->impl_ << + " return value_writer.end_union();\n"; + return true; + } } bool value_writer_generator::gen_enum(AST_Enum*, @@ -290,45 +368,6 @@ bool value_writer_generator::gen_typedef(AST_Typedef*, return true; } -bool value_writer_generator::gen_struct_i(AST_Structure* node, - const std::string type_name, - bool use_cxx11, - ExtensibilityKind ek, - FieldFilter field_filter) -{ - Function write("vwrite", "bool"); - write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const NestedKeyOnly&"); - write.endArgs(); - - const Fields fields(node, field_filter); - be_global->impl_ << - " if (!value_writer.begin_struct(" << extensibility_kind(ek) << ")) {\n" - " return false;\n" - " }\n"; - for (Fields::Iterator i = fields.begin(); i != fields.end(); ++i) { - AST_Field* const field = *i; - const std::string field_name = field->local_name()->get_string(); - const std::string idl_name = canonical_name(field); - const OpenDDS::XTypes::MemberId id = be_global->get_id(field); - const bool must_understand = be_global->is_effective_must_understand(field); - // TODO: Update the arguments for MemberParam when @optional is available. - be_global->impl_ << - " if (!value_writer.begin_struct_member(MemberParam(" << id << ", " << - (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" - " return false;\n" - " }\n"; - generate_write("value." + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i"); - be_global->impl_ << - " if (!value_writer.end_struct_member()) {\n" - " return false;\n" - " }\n"; - } - be_global->impl_ << - " return value_writer.end_struct();\n"; - return true; -} - bool value_writer_generator::gen_struct(AST_Structure* node, UTL_ScopedName* name, const std::vector& fields, @@ -355,7 +394,6 @@ bool value_writer_generator::gen_struct(AST_Structure* node, return true; } - bool value_writer_generator::gen_union(AST_Union* u, UTL_ScopedName* name, const std::vector& branches, @@ -365,38 +403,18 @@ bool value_writer_generator::gen_union(AST_Union* u, be_global->add_include("dds/DCPS/ValueWriter.h", BE_GlobalData::STREAM_H); const std::string type_name = scoped(name); + const ExtensibilityKind ek = be_global->extensibility(u); - { - NamespaceGuard guard; - - Function write("vwrite", "bool"); - write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const " + type_name + "&"); - write.endArgs(); - - const ExtensibilityKind ek = be_global->extensibility(u); - be_global->impl_ << - " if (!value_writer.begin_union(" << extensibility_kind(ek) << ")) {\n" - " return false;\n" - " }\n"; - const bool must_understand = be_global->is_effectively_must_understand(discriminator); - be_global->impl_ << - " if (!value_writer.begin_discriminator(MemberParam(0, " << - (must_understand ? "true" : "false") << "))) {\n" - " return false;\n" - " }\n"; - generate_write("value._d()" , discriminator, "i"); - be_global->impl_ << - " if (!value_writer.end_discriminator()) {\n" - " return false;\n" - " }\n"; - - generateSwitchForUnion(u, "value._d()", branch_helper, branches, - discriminator, "", "", type_name.c_str(), - false, false); - be_global->impl_ << - " return value_writer.end_union();\n"; + NamespaceGuard guard; + if (!gen_union_i(u, branches, discriminator, ek, FieldFilter_All) || + !gen_union_i(u, branches, discriminator, ek, FiedlFilter_NestedKeyOnly)) { + return false; } + if (be_global->is_topic_type(u)) { + if (!gen_union_i(u, branches, discriminator, ek, FieldFilter_KeyOnly)) { + return false; + } + } return true; } From 01735e70c3724b9069e11ad08412bb68cc5fa002 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Mon, 1 Apr 2024 18:20:55 -0500 Subject: [PATCH 03/19] Correct NestedKeyOnly or KeyOnly wrapper for nested fields --- dds/DCPS/ValueDispatcher.h | 2 +- dds/idl/value_writer_generator.cpp | 107 +++++++++++++++++++---------- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/dds/DCPS/ValueDispatcher.h b/dds/DCPS/ValueDispatcher.h index cffd4011c46..6a90db5956e 100644 --- a/dds/DCPS/ValueDispatcher.h +++ b/dds/DCPS/ValueDispatcher.h @@ -22,7 +22,7 @@ struct OpenDDS_Dcps_Export ValueDispatcher { virtual void delete_value(void* data) const = 0; virtual bool read(ValueReader& value_reader, void* data) const = 0; - virtual bool write(ValueWriter& value_writer, const void* data, bool key_only) const = 0; + virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const = 0; virtual DDS::InstanceHandle_t register_instance_helper(DDS::DataWriter* dw, const void* data) const = 0; virtual DDS::ReturnCode_t write_helper(DDS::DataWriter* dw, const void* data, DDS::InstanceHandle_t inst) const = 0; diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index d6e1e0c18f0..a586f53f1d3 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -192,47 +192,52 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st return; } - be_global->impl_ << std::string(level * 2, ' '); + const std::string indent(level * 2, ' '); if (c & CL_FIXED) { be_global->impl_ << - "if (!value_writer.write_fixed(" << expression << ".to_ace_fixed())) {\n" - " return false;\n" - "}\n"; + indent << "if (!value_writer.write_fixed(" << expression << ".to_ace_fixed())) {\n" << + indent << " return false;\n" << + indent << "}\n"; } else if (c & CL_STRING) { be_global->impl_ << - "if (!value_writer.write_" << ((c & CL_WIDE) ? "w" : "") << "string(" << expression << ")) {\n" - " return false;\n" - "}\n"; + indent << "if (!value_writer.write_" << ((c & CL_WIDE) ? "w" : "") << "string(" << expression << ")) {\n" << + indent << " return false;\n" << + indent << "}\n"; } else if (c & CL_PRIMITIVE) { const AST_PredefinedType::PredefinedType pt = dynamic_cast(actual)->pt(); be_global->impl_ << - "if (!value_writer.write_" << primitive_type(pt) << '(' << expression << ")) {\n" - " return false;\n" - "}\n"; + indent << "if (!value_writer.write_" << primitive_type(pt) << '(' << expression << ")) {\n" << + indent << " return false;\n" << + indent << "}\n"; } else { - const std::string type_name = scoped(type->name()); std::string value_expr = expression; - switch (field_filter) { - case FieldFilter_NestedKeyOnly: - value_expr = "nested_key_only_value(" + expression + ")"; - be_global->impl_ << - "const NestedKeyOnly " << value_expr << ";\n"; - break; - case FieldFilter_KeyOnly: - value_expr = "key_only_value(" << expression << ")"; - be_global->impl_ << - "const KeyOnly " << value_expr << ";\n"; - break; + if (!(c & CL_ENUM)) { + const std::string type_name = scoped(type->name()); + switch (field_filter) { + case FieldFilter_NestedKeyOnly: + value_expr = "nested_key_only_value"; + be_global->impl_ << + indent << "const NestedKeyOnly " << value_expr << "(" << expression << ");\n"; + break; + case FieldFilter_KeyOnly: + value_expr = "key_only_value"; + be_global->impl_ << + indent << "const KeyOnly " << value_expr << "(" << expression << ");\n"; + break; + default: + break; + } } + be_global->impl_ << - "if (!vwrite(value_writer, " << value_expr << ")) {\n" - " return false;\n" - "}\n"; + indent << "if (!vwrite(value_writer, " << value_expr << ")) {\n" << + indent << " return false;\n" << + indent << "}\n"; } } @@ -261,15 +266,41 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st return ""; } - bool gen_struct_i(AST_Structure* node, const std::string type_name, + std::string wrapped_type_name(const std::string& type_name, FieldFilter field_filter) + { + std::string wrapped_type_name; + switch (field_filter) { + case FieldFilter_All: + wrapped_type_name = type_name; + break; + case FieldFilter_NestedKeyOnly: + wrapped_type_name = "NestedKeyOnly"; + break; + case FieldFilter_KeyOnly: + wrapped_type_name = "KeyOnly"; + break; + } + return wrapped_type_name; + } + + FieldFilter nested(FieldFilter filter_kind) + { + return filter_kind == FieldFilter_KeyOnly ? FieldFilter_NestedKeyOnly : filter_kind; + } + + bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { + const std::string wrapped_name = wrapped_type_name(type_name, field_filter); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const NestedKeyOnly&"); + write.addArg("value", "const " + wrapped_name + "&"); write.endArgs(); + const std::string value_prefix = field_filter == FieldFilter_All ? "value." : "value.value."; const Fields fields(node, field_filter); + const FieldFilter nested_field_filter = nested(field_filter); + be_global->impl_ << " if (!value_writer.begin_struct(" << extensibility_kind(ek) << ")) {\n" " return false;\n" @@ -279,14 +310,15 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st const std::string field_name = field->local_name()->get_string(); const std::string idl_name = canonical_name(field); const OpenDDS::XTypes::MemberId id = be_global->get_id(field); - const bool must_understand = be_global->is_effective_must_understand(field); + const bool must_understand = be_global->is_effectively_must_understand(field); // TODO: Update the arguments for MemberParam when @optional is available. be_global->impl_ << " if (!value_writer.begin_struct_member(MemberParam(" << id << ", " << (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" " return false;\n" " }\n"; - generate_write("value." + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i"); + generate_write(value_prefix + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i", + 1, nested_field_filter); be_global->impl_ << " if (!value_writer.end_struct_member()) {\n" " return false;\n" @@ -297,14 +329,17 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st return true; } - bool gen_union_i(AST_Union* u, const std::vector& branches, + bool gen_union_i(AST_Union* u, const std::string& type_name, + const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { + const std::string wrapped_name = wrapped_type_name(type_name, filter_kind); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const " + type_name + "&"); + write.addArg("value", "const " + wrapped_name + "&"); write.endArgs(); + const std::string value_prefix = filter_kind == FieldFilter_All ? "value." : "value.value."; be_global->impl_ << " if (!value_writer.begin_union(" << extensibility_kind(ek) << ")) {\n" " return false;\n" @@ -321,7 +356,7 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st (must_understand ? "true" : "false") << "))) {\n" " return false;\n" " }\n"; - generate_write("value._d()" , discriminator, "i", 1, filter_kind); + generate_write(value_prefix + "_d()" , discriminator, "i", 1, nested(filter_kind)); be_global->impl_ << " if (!value_writer.end_discriminator()) {\n" " return false;\n" @@ -370,7 +405,7 @@ bool value_writer_generator::gen_typedef(AST_Typedef*, bool value_writer_generator::gen_struct(AST_Structure* node, UTL_ScopedName* name, - const std::vector& fields, + const std::vector& /*fields*/, AST_Type::SIZE_TYPE, const char*) { @@ -406,13 +441,13 @@ bool value_writer_generator::gen_union(AST_Union* u, const ExtensibilityKind ek = be_global->extensibility(u); NamespaceGuard guard; - if (!gen_union_i(u, branches, discriminator, ek, FieldFilter_All) || - !gen_union_i(u, branches, discriminator, ek, FiedlFilter_NestedKeyOnly)) { + if (!gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_All) || + !gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_NestedKeyOnly)) { return false; } if (be_global->is_topic_type(u)) { - if (!gen_union_i(u, branches, discriminator, ek, FieldFilter_KeyOnly)) { + if (!gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_KeyOnly)) { return false; } } From e5418021a8dd9605c627e5cc9456134bcd745991 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Tue, 2 Apr 2024 12:03:02 -0500 Subject: [PATCH 04/19] Working vwrite --- dds/idl/value_writer_generator.cpp | 46 ++++++++++++++++-------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index a586f53f1d3..a6a1028eb42 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -18,8 +18,14 @@ using namespace AstTypeClassification; namespace { -void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, - int level = 1, FieldFilter field_filter = FieldFilter_All); +void generate_write(const std::string& expression, const std::string& field_name, + AST_Type* type, const std::string& idx, int level = 1, + FieldFilter field_filter = FieldFilter_All); + + FieldFilter nested(FieldFilter filter_kind) + { + return filter_kind == FieldFilter_KeyOnly ? FieldFilter_NestedKeyOnly : filter_kind; + } std::string primitive_type(AST_PredefinedType::PredefinedType pt) { @@ -61,8 +67,8 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st } } - void array_helper(const std::string& expression, AST_Array* array, - size_t dim_idx, const std::string& idx, int level) + void array_helper(const std::string& expression, AST_Array* array, size_t dim_idx, + const std::string& idx, int level, FieldFilter filter_kind) { const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; const std::string indent(level * 2, ' '); @@ -84,7 +90,7 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st indent << " if (!value_writer.begin_element(static_cast(" << idx << "))) {\n" << indent << " return false;\n" << indent << " }\n"; - array_helper(expression + "[" + idx + "]", array, dim_idx + 1, idx + "i", level + 1); + array_helper(expression + "[" + idx + "]", array, dim_idx + 1, idx + "i", level + 1, filter_kind); be_global->impl_ << indent << " if (!value_writer.end_element()) {\n" << indent << " return false;\n" << @@ -115,13 +121,13 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st indent << "}\n"; } else { - generate_write(expression, array->base_type(), idx + "i", level); + generate_write(expression, "elem", array->base_type(), idx + "i", level, nested(filter_kind)); } } } void sequence_helper(const std::string& expression, AST_Sequence* sequence, - const std::string& idx, int level) + const std::string& idx, int level, FieldFilter filter_kind) { const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; const char* const length_func = use_cxx11 ? "size" : "length"; @@ -162,7 +168,7 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st indent << " if (!value_writer.begin_element(static_cast(" << idx << "))) {\n" << indent << " return false;\n" << indent << " }\n"; - generate_write(expression + "[" + idx + "]", base_type, idx + "i", level + 1); + generate_write(expression + "[" + idx + "]", "elem", base_type, idx + "i", level + 1, nested(filter_kind)); be_global->impl_ << indent << " if (!value_writer.end_element()) {\n" << indent << " return false;\n" << @@ -176,19 +182,20 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st indent << "}\n"; } - void generate_write(const std::string& expression, AST_Type* type, const std::string& idx, int level, FieldFilter field_filter) +void generate_write(const std::string& expression, const std::string& field_name, + AST_Type* type, const std::string& idx, int level, FieldFilter field_filter) { AST_Type* const actual = resolveActualType(type); const Classification c = classify(actual); if (c & CL_SEQUENCE) { AST_Sequence* const sequence = dynamic_cast(actual); - sequence_helper(expression, sequence, idx, level); + sequence_helper(expression, sequence, idx, level, field_filter); return; } else if (c & CL_ARRAY) { AST_Array* const array = dynamic_cast(actual); - array_helper(expression, array, 0, idx, level); + array_helper(expression, array, 0, idx, level, field_filter); return; } @@ -220,12 +227,12 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st const std::string type_name = scoped(type->name()); switch (field_filter) { case FieldFilter_NestedKeyOnly: - value_expr = "nested_key_only_value"; + value_expr = field_name + "_nested_key_only"; be_global->impl_ << indent << "const NestedKeyOnly " << value_expr << "(" << expression << ");\n"; break; case FieldFilter_KeyOnly: - value_expr = "key_only_value"; + value_expr = field_name + "_key_only"; be_global->impl_ << indent << "const KeyOnly " << value_expr << "(" << expression << ");\n"; break; @@ -258,7 +265,7 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st (must_understand ? "true" : "false") << ", \"" << canonical_name(branch) << "\", false, true))) {\n" " return false;\n" " }\n"; - generate_write("value." + field_name + "()", type, "i", 2); + generate_write("value." + field_name + "()", field_name, type, "i", 2); be_global->impl_ << " if (!value_writer.end_union_member()) {\n" " return false;\n" @@ -283,11 +290,6 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st return wrapped_type_name; } - FieldFilter nested(FieldFilter filter_kind) - { - return filter_kind == FieldFilter_KeyOnly ? FieldFilter_NestedKeyOnly : filter_kind; - } - bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { @@ -317,8 +319,8 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st (must_understand ? "true" : "false") << ", \"" << idl_name << "\", false, true))) {\n" " return false;\n" " }\n"; - generate_write(value_prefix + field_name + (use_cxx11 ? "()" : ""), field->field_type(), "i", - 1, nested_field_filter); + generate_write(value_prefix + field_name + (use_cxx11 ? "()" : ""), field_name, + field->field_type(), "i", 1, nested_field_filter); be_global->impl_ << " if (!value_writer.end_struct_member()) {\n" " return false;\n" @@ -356,7 +358,7 @@ void generate_write(const std::string& expression, AST_Type* type, const std::st (must_understand ? "true" : "false") << "))) {\n" " return false;\n" " }\n"; - generate_write(value_prefix + "_d()" , discriminator, "i", 1, nested(filter_kind)); + generate_write(value_prefix + "_d()", "disc", discriminator, "i", 1, nested(filter_kind)); be_global->impl_ << " if (!value_writer.end_discriminator()) {\n" " return false;\n" From d25ad6a38b3b1ab123d863e901e5e394d9b80c35 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Tue, 2 Apr 2024 16:59:47 -0500 Subject: [PATCH 05/19] Add vread --- dds/idl/dds_generator.h | 32 ++++ dds/idl/value_reader_generator.cpp | 251 ++++++++++++++++++----------- dds/idl/value_writer_generator.cpp | 32 +--- 3 files changed, 196 insertions(+), 119 deletions(-) diff --git a/dds/idl/dds_generator.h b/dds/idl/dds_generator.h index e0d9225c240..8efa9b064d3 100644 --- a/dds/idl/dds_generator.h +++ b/dds/idl/dds_generator.h @@ -1443,6 +1443,38 @@ inline std::string type_kind(AST_Type* type) } } +inline +std::string key_only_type_name(const std::string& type_name, FieldFilter field_filter) +{ + std::string wrapped_type_name; + switch (field_filter) { + case FieldFilter_All: + wrapped_type_name = type_name; + break; + case FieldFilter_NestedKeyOnly: + wrapped_type_name = "NestedKeyOnly"; + break; + case FieldFilter_KeyOnly: + wrapped_type_name = "KeyOnly"; + break; + } + return wrapped_type_name; +} + +inline +FieldFilter nested(FieldFilter filter_kind) +{ + return filter_kind == FieldFilter_KeyOnly ? FieldFilter_NestedKeyOnly : filter_kind; +} + +inline +bool has_discriminator(AST_Union* u, FieldFilter filter_kind) +{ + return be_global->union_discriminator_is_key(u) + || filter_kind == FieldFilter_NestedKeyOnly + || filter_kind == FieldFilter_All; +} + /// Handling wrapping and unwrapping references in the wrapper types: /// NestedKeyOnly, IDL::DistinctType, and *_forany. struct RefWrapper { diff --git a/dds/idl/value_reader_generator.cpp b/dds/idl/value_reader_generator.cpp index 511f02ae9cd..456d34ac503 100644 --- a/dds/idl/value_reader_generator.cpp +++ b/dds/idl/value_reader_generator.cpp @@ -19,8 +19,9 @@ using namespace AstTypeClassification; namespace { - void generate_read(const std::string& expression, const std::string& accessor, - AST_Type* type, const std::string& idx, int level = 1); +void generate_read(const std::string& expression, const std::string& accessor, + const std::string& field_name, AST_Type* type, const std::string& idx, + int level = 1, FieldFilter field_filter = FieldFilter_All); std::string primitive_type(AST_PredefinedType::PredefinedType pt) { @@ -62,8 +63,8 @@ namespace { } } - void array_helper(const std::string& expression, AST_Array* array, - size_t dim_idx, const std::string& idx, int level) + void array_helper(const std::string& expression, AST_Array* array, size_t dim_idx, + const std::string& idx, int level, FieldFilter filter_kind) { const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; const std::string indent(level * 2, ' '); @@ -80,7 +81,7 @@ namespace { indent << "for (" << (use_cxx11 ? "size_t " : "unsigned int ") << idx << " = 0; " << idx << " != " << dim << "; ++" << idx << ") {\n" << indent << " if (!value_reader.begin_element()) return false;\n"; - array_helper(expression + "[" + idx + "]", array, dim_idx + 1, idx + "i", level + 1); + array_helper(expression + "[" + idx + "]", array, dim_idx + 1, idx + "i", level + 1, filter_kind); be_global->impl_ << indent << " if (!value_reader.end_element()) return false;\n" << indent << "}\n" << @@ -99,13 +100,13 @@ namespace { indent << "if (!value_reader.end_array()) return false;\n"; } else { - generate_read(expression, "", array->base_type(), idx + "i", level); + generate_read(expression, "", "elem", array->base_type(), idx + "i", level, nested(filter_kind)); } } } void sequence_helper(const std::string& expression, AST_Sequence* sequence, - const std::string& idx, int level) + const std::string& idx, int level, FieldFilter filter_kind) { // TODO: Take advantage of the size. const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; @@ -128,7 +129,8 @@ namespace { } be_global->impl_ << indent << " if (!value_reader.begin_element()) return false;\n"; - generate_read(expression + "[" + idx + "]", "", sequence->base_type(), idx + "i", level + 1); + generate_read(expression + "[" + idx + "]", "", "elem", sequence->base_type(), + idx + "i", level + 1, nested(filter_kind)); be_global->impl_ << indent << " if (!value_reader.end_element()) return false;\n" << indent << "}\n" << @@ -136,19 +138,20 @@ namespace { } void generate_read(const std::string& expression, const std::string& accessor, - AST_Type* type, const std::string& idx, int level) + const std::string& field_name, AST_Type* type, const std::string& idx, + int level, FieldFilter filter_kind) { AST_Type* const actual = resolveActualType(type); const Classification c = classify(actual); if (c & CL_SEQUENCE) { AST_Sequence* const sequence = dynamic_cast(actual); - sequence_helper(expression + accessor, sequence, idx, level); + sequence_helper(expression + accessor, sequence, idx, level, filter_kind); return; } else if (c & CL_ARRAY) { AST_Array* const array = dynamic_cast(actual); - array_helper(expression + accessor, array, 0, idx, level); + array_helper(expression + accessor, array, 0, idx, level, filter_kind); return; } @@ -186,9 +189,29 @@ namespace { indent << "if (!value_reader.read_" << primitive_type(pt) << '(' << expression << accessor << ")) return false;\n"; } } else { + std::string value_expr = expression + accessor; + if (!(c & CL_ENUM)) { + const std::string type_name = scoped(type->name()); + switch (filter_kind) { + case FieldFilter_NestedKeyOnly: + value_expr = field_name + "_nested_key_only"; + be_global->impl_ << + indent << "const NestedKeyOnly<" << type_name << "> " << + value_expr << "(" << expression << accessor << ");\n"; + break; + case FieldFilter_KeyOnly: + value_expr = field_name + "_key_only"; + be_global->impl_ << + indent << "const KeyOnly<" << type_name << "> " << + value_expr << "(" << expression << accessor << ");\n"; + break; + default: + break; + } + } + be_global->impl_ << - indent << "if (!vread(value_reader, " << expression << accessor << - ")) return false;\n"; + indent << "if (!vread(value_reader, " << value_expr << ")) return false;\n"; } } @@ -211,12 +234,116 @@ namespace { be_global->impl_ << " if (!value_reader.begin_union_member()) return false;\n" " " << decl << " bv;\n"; - generate_read("bv", "", type, "i", 2); + generate_read("bv", "", field_name, type, "i", 2); be_global->impl_ << " value." << field_name << "(bv" << ((c & CL_STRING) ? ".c_str()" : "") << ");\n" << " if (!value_reader.end_union_member()) return false;\n"; return ""; } + + bool gen_struct_i(AST_Structure* node, const std::string& type_name, + bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) + { + const std::string wrapped_name = key_only_type_name(type_name, field_filter); + Function read("vread", "bool"); + read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); + read.addArg("value", wrapped_name + "&"); + read.endArgs(); + + be_global->impl_ << + " static const ListMemberHelper::Pair pairs[] = {"; + + const std::string value_prefix = field_filter == FieldFilter_All ? "value." : "value.value."; + const Fields fields(node, field_filter); + + bool first = true; + for (Fields::Iterator it = fields.begin(); it != fields.end(); ++it) { + AST_Field* const field = *it; + const std::string idl_name = canonical_name(field); + if (first) { + first = false; + } else { + be_global->impl_ << ','; + } + be_global->impl_ << + '{' << '"' << idl_name << '"' << ',' << be_global->get_id(field) << '}'; + } + + be_global->impl_ << + ",{0,0}};\n" + " ListMemberHelper helper(pairs);\n"; + + be_global->impl_ << + " if (!value_reader.begin_struct(" << extensibility_kind(ek) << ")) return false;\n" + " XTypes::MemberId member_id;\n" + " while (value_reader.members_remaining()) {\n" + " if (!value_reader.begin_struct_member(member_id, helper)) return false;\n" + " switch (member_id) {\n"; + + for (Fields::Iterator it = fields.begin(); it != fields.end(); ++it) { + AST_Field* const field = *it; + const std::string field_name = field->local_name()->get_string(); + be_global->impl_ << + " case " << be_global->get_id(field) << ": {\n"; + generate_read(value_prefix + field_name, use_cxx11 ? "()" : "", field_name, + field->field_type(), "i", 3, nested(field_filter)); + be_global->impl_ << + " break;\n" + " }\n"; + } + + be_global->impl_ << + " }\n" + " if (!value_reader.end_struct_member()) return false;\n" + " }\n" + " if (!value_reader.end_struct()) return false;\n" + " return true;\n"; + return true; + } + + bool gen_union_i(AST_Union* u, const std::string& type_name, + const std::vector& branches, + AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) + { + const std::string wrapped_name = key_only_type_name(type_name, filter_kind); + Function read("vread", "bool"); + read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); + read.addArg("value", wrapped_name + "&"); + read.endArgs(); + + const std::string value_prefix = filter_kind == FieldFilter_All ? "value." : "value.value."; + be_global->impl_ << + " if (!value_reader.begin_union(" << extensibility_kind(ek) << ")) return false;\n"; + + const bool has_disc = has_discriminator(u, filter_kind); + if (has_disc) { + be_global->impl_ << + " if (!value_reader.begin_discriminator()) return false;\n" + " " << scoped(discriminator->name()) << " d;\n"; + generate_read("d", "", "disc", discriminator, "i", 1, nested(filter_kind)); + be_global->impl_ << + " if (!value_reader.end_discriminator()) return false;\n"; + } + + if (filter_kind == FieldFilter_All) { + generateSwitchForUnion(u, "d", branch_helper, branches, + discriminator, "", "", type_name.c_str(), + false, false); + be_global->impl_ << + " value._d(d);\n"; + } else if (has_disc) { + // Assigning the discriminator directly before a branch is set doesn't strictly + // conform to the IDL-to-C++ mapping. However, this case cares only about the + // discriminator, i.e. KeyOnly or NestedKeyOnly samples, so it's probably fine. + be_global->impl_ << + " value.value._d(d);\n"; + } + + be_global->impl_ << + " if (!value_reader.end_union()) return false;\n" + " return true;\n"; + return true; + } } bool value_reader_generator::gen_enum(AST_Enum*, @@ -249,7 +376,7 @@ bool value_reader_generator::gen_typedef(AST_Typedef*, bool value_reader_generator::gen_struct(AST_Structure* node, UTL_ScopedName* name, - const std::vector& fields, + const std::vector& /*fields*/, AST_Type::SIZE_TYPE, const char*) { @@ -258,64 +385,22 @@ bool value_reader_generator::gen_struct(AST_Structure* node, const std::string type_name = scoped(name); const bool use_cxx11 = be_global->language_mapping() == BE_GlobalData::LANGMAP_CXX11; - const std::string accessor = use_cxx11 ? "()" : ""; - - { - NamespaceGuard guard; - - Function read("vread", "bool"); - read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); - read.addArg("value", type_name + "&"); - read.endArgs(); - - be_global->impl_ << - " static const ListMemberHelper::Pair pairs[] = {"; - - for (size_t i = 0; i != fields.size(); ++i) { - if (i) { - be_global->impl_ << ','; - } - const std::string idl_name = canonical_name(fields[i]); - be_global->impl_ << - '{' << '"' << idl_name << '"' << ',' << be_global->get_id(fields[i]) << '}'; - } - - be_global->impl_ << - ",{0,0}};\n" - " ListMemberHelper helper(pairs);\n"; + const ExtensibilityKind ek = be_global->extensibility(node); - const ExtensibilityKind ek = be_global->extensibility(node); - be_global->impl_ << - " if (!value_reader.begin_struct(" << extensibility_kind(ek) << ")) return false;\n" - " XTypes::MemberId member_id;\n" - " while (value_reader.members_remaining()) {\n" - " if (!value_reader.begin_struct_member(member_id, helper)) return false;\n" - " switch (member_id) {\n"; + NamespaceGuard guard; + if (!gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_All) || + !gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_NestedKeyOnly)) { + return false; + } - for (std::vector::const_iterator pos = fields.begin(), limit = fields.end(); - pos != limit; ++pos) { - AST_Field* const field = *pos; - const std::string field_name = field->local_name()->get_string(); - be_global->impl_ << - " case " << be_global->get_id(field) << ": {\n"; - generate_read("value." + field_name, accessor, field->field_type(), "i", 3); - be_global->impl_ << - " break;\n" - " }\n"; + if (be_global->is_topic_type(node)) { + if (!gen_struct_i(node, type_name, use_cxx11, ek, FieldFilter_KeyOnly)) { + return false; } - - be_global->impl_ << - " }\n" - " if (!value_reader.end_struct_member()) return false;\n" - " }\n" - " if (!value_reader.end_struct()) return false;\n" - " return true;\n"; } - return true; } - bool value_reader_generator::gen_union(AST_Union* u, UTL_ScopedName* name, const std::vector& branches, @@ -326,32 +411,18 @@ bool value_reader_generator::gen_union(AST_Union* u, be_global->add_include("dds/DCPS/ValueReader.h", BE_GlobalData::STREAM_H); const std::string type_name = scoped(name); + const ExtensibilityKind ek = be_global->extensibility(u); - { - NamespaceGuard guard; - - Function read("vread", "bool"); - read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); - read.addArg("value", type_name + "&"); - read.endArgs(); - - const ExtensibilityKind ek = be_global->extensibility(u); - be_global->impl_ << - " if (!value_reader.begin_union(" << extensibility_kind(ek) << ")) return false;\n" - " if (!value_reader.begin_discriminator()) return false;\n" - " " << scoped(discriminator->name()) << " d;\n"; - generate_read("d", "", discriminator, "i", 2); - be_global->impl_ << - " if (!value_reader.end_discriminator()) return false;\n"; - - generateSwitchForUnion(u, "d", branch_helper, branches, - discriminator, "", "", type_name.c_str(), - false, false); - be_global->impl_ << - " value._d(d);\n" - " if (!value_reader.end_union()) return false;\n" - " return true;\n"; + NamespaceGuard guard; + if (!gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_All) || + !gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_NestedKeyOnly)) { + return false; } + if (be_global->is_topic_type(u)) { + if (!gen_union_i(u, type_name, branches, discriminator, ek, FieldFilter_KeyOnly)) { + return false; + } + } return true; } diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index a6a1028eb42..120d9e46831 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -22,11 +22,6 @@ void generate_write(const std::string& expression, const std::string& field_name AST_Type* type, const std::string& idx, int level = 1, FieldFilter field_filter = FieldFilter_All); - FieldFilter nested(FieldFilter filter_kind) - { - return filter_kind == FieldFilter_KeyOnly ? FieldFilter_NestedKeyOnly : filter_kind; - } - std::string primitive_type(AST_PredefinedType::PredefinedType pt) { switch (pt) { @@ -273,27 +268,10 @@ void generate_write(const std::string& expression, const std::string& field_name return ""; } - std::string wrapped_type_name(const std::string& type_name, FieldFilter field_filter) - { - std::string wrapped_type_name; - switch (field_filter) { - case FieldFilter_All: - wrapped_type_name = type_name; - break; - case FieldFilter_NestedKeyOnly: - wrapped_type_name = "NestedKeyOnly"; - break; - case FieldFilter_KeyOnly: - wrapped_type_name = "KeyOnly"; - break; - } - return wrapped_type_name; - } - bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { - const std::string wrapped_name = wrapped_type_name(type_name, field_filter); + const std::string wrapped_name = key_only_type_name(type_name, field_filter); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); write.addArg("value", "const " + wrapped_name + "&"); @@ -335,7 +313,7 @@ void generate_write(const std::string& expression, const std::string& field_name const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { - const std::string wrapped_name = wrapped_type_name(type_name, filter_kind); + const std::string wrapped_name = key_only_type_name(type_name, filter_kind); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); write.addArg("value", "const " + wrapped_name + "&"); @@ -347,11 +325,7 @@ void generate_write(const std::string& expression, const std::string& field_name " return false;\n" " }\n"; - const bool write_disc = be_global->union_discriminator_is_key(u) - || filter_kind == FieldFilter_NestedKeyOnly - || filter_kind == FieldFilter_All; - - if (write_disc) { + if (has_discriminator(u, filter_kind)) { const bool must_understand = be_global->is_effectively_must_understand(discriminator); be_global->impl_ << " if (!value_writer.begin_discriminator(MemberParam(0, " << From 1e6349dd6f93b3606e553afc1d90ccbdc1177474 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Wed, 3 Apr 2024 13:05:35 -0500 Subject: [PATCH 06/19] Fix compile errors --- dds/idl/dds_generator.h | 20 ++++++++++++++++---- dds/idl/value_reader_generator.cpp | 4 ++-- dds/idl/value_writer_generator.cpp | 8 ++++---- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/dds/idl/dds_generator.h b/dds/idl/dds_generator.h index 8efa9b064d3..8eb4e6c2e84 100644 --- a/dds/idl/dds_generator.h +++ b/dds/idl/dds_generator.h @@ -1444,18 +1444,30 @@ inline std::string type_kind(AST_Type* type) } inline -std::string key_only_type_name(const std::string& type_name, FieldFilter field_filter) +std::string key_only_type_name(const std::string& type_name, FieldFilter field_filter, bool writing) { std::string wrapped_type_name; switch (field_filter) { case FieldFilter_All: - wrapped_type_name = type_name; + if (writing) { + wrapped_type_name = "const" + type_name; + } else { + wrapped_type_name = type_name; + } break; case FieldFilter_NestedKeyOnly: - wrapped_type_name = "NestedKeyOnly"; + if (writing) { + wrapped_type_name = "const NestedKeyOnly"; + } else { + wrapped_type_name = "const NestedKeyOnly<" + type_name + ">"; + } break; case FieldFilter_KeyOnly: - wrapped_type_name = "KeyOnly"; + if (writing) { + wrapped_type_name = "const KeyOnly"; + } else { + wrapped_type_name = "const KeyOnly<" + type_name + ">"; + } break; } return wrapped_type_name; diff --git a/dds/idl/value_reader_generator.cpp b/dds/idl/value_reader_generator.cpp index 456d34ac503..d55d98ced46 100644 --- a/dds/idl/value_reader_generator.cpp +++ b/dds/idl/value_reader_generator.cpp @@ -244,7 +244,7 @@ void generate_read(const std::string& expression, const std::string& accessor, bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { - const std::string wrapped_name = key_only_type_name(type_name, field_filter); + const std::string wrapped_name = key_only_type_name(type_name, field_filter, false); Function read("vread", "bool"); read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); read.addArg("value", wrapped_name + "&"); @@ -305,7 +305,7 @@ void generate_read(const std::string& expression, const std::string& accessor, const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { - const std::string wrapped_name = key_only_type_name(type_name, filter_kind); + const std::string wrapped_name = key_only_type_name(type_name, filter_kind, false); Function read("vread", "bool"); read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); read.addArg("value", wrapped_name + "&"); diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index 120d9e46831..252ce892f03 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -271,10 +271,10 @@ void generate_write(const std::string& expression, const std::string& field_name bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { - const std::string wrapped_name = key_only_type_name(type_name, field_filter); + const std::string wrapped_name = key_only_type_name(type_name, field_filter, true); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const " + wrapped_name + "&"); + write.addArg("value", wrapped_name + "&"); write.endArgs(); const std::string value_prefix = field_filter == FieldFilter_All ? "value." : "value.value."; @@ -313,10 +313,10 @@ void generate_write(const std::string& expression, const std::string& field_name const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { - const std::string wrapped_name = key_only_type_name(type_name, filter_kind); + const std::string wrapped_name = key_only_type_name(type_name, filter_kind, true); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", "const " + wrapped_name + "&"); + write.addArg("value", wrapped_name + "&"); write.endArgs(); const std::string value_prefix = filter_kind == FieldFilter_All ? "value." : "value.value."; From 38f36f953c33082dbf1dc2c6ee353b1d18ded7a9 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Wed, 3 Apr 2024 17:01:22 -0500 Subject: [PATCH 07/19] Fix compile errors --- dds/DCPS/ValueDispatcher.h | 10 ++++++---- dds/idl/value_reader_generator.cpp | 11 ++--------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/dds/DCPS/ValueDispatcher.h b/dds/DCPS/ValueDispatcher.h index 6a90db5956e..ccb810bf74a 100644 --- a/dds/DCPS/ValueDispatcher.h +++ b/dds/DCPS/ValueDispatcher.h @@ -21,7 +21,7 @@ struct OpenDDS_Dcps_Export ValueDispatcher { virtual void* new_value() const = 0; virtual void delete_value(void* data) const = 0; - virtual bool read(ValueReader& value_reader, void* data) const = 0; + virtual bool read(ValueReader& value_reader, void* data, bool key_only = false) const = 0; virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const = 0; virtual DDS::InstanceHandle_t register_instance_helper(DDS::DataWriter* dw, const void* data) const = 0; @@ -45,16 +45,18 @@ struct ValueDispatcher_T : public virtual ValueDispatcher { delete tbd; } - virtual bool read(ValueReader& value_reader, void* data) const + virtual bool read(ValueReader& value_reader, void* data, bool key_only = false) const { + if (key_only) { + return vread(value_reader, *static_cast*>(data)); + } return vread(value_reader, *static_cast(data)); } - // Don't have to worry about NestedKeyOnly here since this is only called for topic type. virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const { if (key_only) { - return vwrite(value_writer, *static_cast*>(data)); + return vwrite(value_writer, *static_cast*>(data)); } return vwrite(value_writer, *static_cast(data)); } diff --git a/dds/idl/value_reader_generator.cpp b/dds/idl/value_reader_generator.cpp index d55d98ced46..74c8ceb5fb9 100644 --- a/dds/idl/value_reader_generator.cpp +++ b/dds/idl/value_reader_generator.cpp @@ -256,21 +256,14 @@ void generate_read(const std::string& expression, const std::string& accessor, const std::string value_prefix = field_filter == FieldFilter_All ? "value." : "value.value."; const Fields fields(node, field_filter); - bool first = true; for (Fields::Iterator it = fields.begin(); it != fields.end(); ++it) { AST_Field* const field = *it; - const std::string idl_name = canonical_name(field); - if (first) { - first = false; - } else { - be_global->impl_ << ','; - } be_global->impl_ << - '{' << '"' << idl_name << '"' << ',' << be_global->get_id(field) << '}'; + "{\"" << canonical_name(field) << "\"," << be_global->get_id(field) << "},"; } be_global->impl_ << - ",{0,0}};\n" + "{0,0}};\n" " ListMemberHelper helper(pairs);\n"; be_global->impl_ << From 308972cf8a2beba2f88a0c4b72c0c2cd0f184151 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Thu, 4 Apr 2024 18:14:07 -0500 Subject: [PATCH 08/19] Add KeyOnly test cases to the vread_vwrite test --- .../Compiler/vread_vwrite/VreadVwriteTest.cpp | 175 ++++++++++++++++++ .../Compiler/vread_vwrite/VreadVwriteTest.idl | 48 +++++ 2 files changed, 223 insertions(+) diff --git a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.cpp b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.cpp index d6b11d4b7dc..df1c1631ece 100644 --- a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.cpp +++ b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.cpp @@ -294,6 +294,181 @@ TEST(VreadVwriteTest, StaticSerializeTest) //std::cout << output << std::endl; } +template +void write_helper(const T& sample, CORBA::String_out out) +{ + OpenDDS::DCPS::KeyOnly keyonly(sample); + + rapidjson::StringBuffer buffer; + rapidjson::Writer writer(buffer); + OpenDDS::DCPS::JsonValueWriter > jvw(writer); + + ASSERT_TRUE(vwrite(jvw, keyonly)); + out = buffer.GetString(); +} + +TEST(VreadVwriteTest, KeyOnly_StructWithNoKeys) +{ + Mod::NoExplicitKeysStruct sample; + OpenDDS::DCPS::set_default(sample); + + // Write the KeyOnly sample to JSON. Then parse the result and compare with the original sample. + // In this case no fields are serialized. + CORBA::String_var result; + write_helper(sample, result); + rapidjson::Document document; + document.Parse(result.in()); + rapidjson::Value& val = document; + + ASSERT_TRUE(val.IsObject()); + ASSERT_TRUE(val.ObjectEmpty()); + + // Read from the JSON result with vread + rapidjson::StringStream buffer(result.in()); + OpenDDS::DCPS::JsonValueReader<> jvr(buffer); + Mod::NoExplicitKeysStruct sample_out; + OpenDDS::DCPS::set_default(sample_out); + const OpenDDS::DCPS::KeyOnly keyonly_out(sample_out); + ASSERT_TRUE(vread(jvr, keyonly_out)); + ASSERT_EQ(sample.b, sample_out.b); + ASSERT_EQ(sample.c, sample_out.c); + ASSERT_EQ(sample.l, sample_out.l); +} + +TEST(VreadVwriteTest, KeyOnly_StructWithKeys) +{ + Mod::KeyOnlyStruct sample; + OpenDDS::DCPS::set_default(sample); + sample.id = 1; + sample.eks.s = 10; + sample.eks.str = "eks.str"; + sample.neks.b = true; + sample.neks.c = 'a'; + sample.neks.l = 12l; + sample.eksarr[0][0].s = 1; + sample.eksarr[0][0].str = "eksarr[0][0].str"; + sample.eksarr[0][1].s = 2; + sample.eksarr[0][1].str = "eksarr[0][1].str"; + sample.eksarr[1][0].s = 3; + sample.eksarr[1][0].str = "eksarr[1][0].str"; + sample.eksarr[1][1].s = 4; + sample.eksarr[1][1].str = "eksarr[1][1].str"; + sample.eku.c('d'); + sample.neku.b(false); + sample.str = "hello"; + + // Write the KeyOnly sample to JSON. Then parse and check the result against the input. + CORBA::String_var result; + write_helper(sample, result); + rapidjson::Document document; + document.Parse(result.in()); + rapidjson::Value& val = document; + + ASSERT_TRUE(val.IsObject()); + ASSERT_EQ(7ul, val.MemberCount()); + ASSERT_EQ(val["id"], 1); + ASSERT_EQ(2ul, val["eks"].MemberCount()); + ASSERT_EQ(val["eks"]["s"], 10); + ASSERT_EQ(val["eks"]["str"], "eks.str"); + ASSERT_EQ(val["neks"]["b"], true); + ASSERT_EQ(val["neks"]["c"], "a"); + ASSERT_EQ(val["neks"]["l"], 12); + ASSERT_EQ(2ul, val["eksarr"].Size()); + ASSERT_EQ(2ul, val["eksarr"][0].Size()); + ASSERT_EQ(2ul, val["eksarr"][1].Size()); + ASSERT_EQ(2ul, val["eksarr"][0][0].MemberCount()); + ASSERT_EQ(val["eksarr"][0][0]["s"], 1); + ASSERT_EQ(val["eksarr"][0][0]["str"], "eksarr[0][0].str"); + ASSERT_EQ(2ul, val["eksarr"][0][1].MemberCount()); + ASSERT_EQ(val["eksarr"][0][1]["s"], 2); + ASSERT_EQ(val["eksarr"][0][1]["str"], "eksarr[0][1].str"); + ASSERT_EQ(2ul, val["eksarr"][1][0].MemberCount()); + ASSERT_EQ(val["eksarr"][1][0]["s"], 3); + ASSERT_EQ(val["eksarr"][1][0]["str"], "eksarr[1][0].str"); + ASSERT_EQ(2ul, val["eksarr"][1][1].MemberCount()); + ASSERT_EQ(val["eksarr"][1][1]["s"], 4); + ASSERT_EQ(val["eksarr"][1][1]["str"], "eksarr[1][1].str"); + ASSERT_EQ(1ul, val["eku"].MemberCount()); + ASSERT_EQ(val["eku"]["$discriminator"], "two"); + ASSERT_EQ(1ul, val["neku"].MemberCount()); + ASSERT_EQ(val["neku"]["$discriminator"], "one"); + ASSERT_EQ(val["str"], "hello"); + + // Read from the JSON result into a KeyOnly sample and compare with the original sample. + rapidjson::StringStream buffer(result.in()); + OpenDDS::DCPS::JsonValueReader<> jvr(buffer); + Mod::KeyOnlyStruct sample_out; + OpenDDS::DCPS::set_default(sample_out); + const OpenDDS::DCPS::KeyOnly keyonly_out(sample_out); + + ASSERT_TRUE(vread(jvr, keyonly_out)); + ASSERT_EQ(sample.id, sample_out.id); + ASSERT_EQ(sample.eks.s, sample_out.eks.s); + ASSERT_STREQ(sample.eks.str, sample_out.eks.str); + ASSERT_EQ(sample.neks.b, sample_out.neks.b); + ASSERT_EQ(sample.neks.c, sample_out.neks.c); + ASSERT_EQ(sample.neks.l, sample_out.neks.l); + ASSERT_EQ(sample.eksarr[0][0].s, sample_out.eksarr[0][0].s); + ASSERT_STREQ(sample.eksarr[0][0].str, sample_out.eksarr[0][0].str); + ASSERT_EQ(sample.eksarr[0][1].s, sample_out.eksarr[0][1].s); + ASSERT_STREQ(sample.eksarr[0][1].str, sample_out.eksarr[0][1].str); + ASSERT_EQ(sample.eksarr[1][0].s, sample_out.eksarr[1][0].s); + ASSERT_STREQ(sample.eksarr[1][0].str, sample_out.eksarr[1][0].str); + ASSERT_EQ(sample.eksarr[1][1].s, sample_out.eksarr[1][1].s); + ASSERT_STREQ(sample.eksarr[1][1].str, sample_out.eksarr[1][1].str); + ASSERT_EQ(sample.eku._d(), sample_out.eku._d()); + ASSERT_EQ(sample.neku._d(), sample_out.neku._d()); + ASSERT_STREQ(sample.str, sample_out.str); +} + +TEST(VreadVwriteTest, KeyOnly_UnionWithNoKey) +{ + Mod::NoExplicitKeyUnion sample; + OpenDDS::DCPS::set_default(sample); + + CORBA::String_var result; + write_helper(sample, result); + rapidjson::Document document; + document.Parse(result.in()); + rapidjson::Value& val = document; + + ASSERT_TRUE(val.IsObject()); + ASSERT_TRUE(val.ObjectEmpty()); + + rapidjson::StringStream buffer(result.in()); + OpenDDS::DCPS::JsonValueReader<> jvr(buffer); + Mod::NoExplicitKeyUnion sample_out; + OpenDDS::DCPS::set_default(sample_out); + const OpenDDS::DCPS::KeyOnly keyonly_out(sample_out); + ASSERT_TRUE(vread(jvr, keyonly_out)); + ASSERT_EQ(sample._d(), sample_out._d()); +} + +TEST(VreadVwriteTest, KeyOnly_UnionWithKey) +{ + Mod::ExplicitKeyUnion sample; + sample.o(0x22); + sample._d(Mod::three); + + CORBA::String_var result; + write_helper(sample, result); + rapidjson::Document document; + document.Parse(result.in()); + rapidjson::Value& val = document; + + ASSERT_TRUE(val.IsObject()); + ASSERT_EQ(1ul, val.MemberCount()); + ASSERT_EQ(val["$discriminator"], "three"); + + rapidjson::StringStream buffer(result.in()); + OpenDDS::DCPS::JsonValueReader<> jvr(buffer); + Mod::ExplicitKeyUnion sample_out; + OpenDDS::DCPS::set_default(sample_out); + const OpenDDS::DCPS::KeyOnly keyonly_out(sample_out); + ASSERT_TRUE(vread(jvr, keyonly_out)); + ASSERT_EQ(sample._d(), sample_out._d()); +} + #ifndef OPENDDS_SAFETY_PROFILE DDS::DynamicType_var get_dynamic_type(OpenDDS::XTypes::TypeLookupService& tls) diff --git a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.idl b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.idl index 7eb2dd5fd71..56f647d0549 100644 --- a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.idl +++ b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.idl @@ -95,4 +95,52 @@ module Mod { MaskType mask; }; + /////// For KeyOnly tests + struct ExplicitKeysStruct { + @key short s; + @key string str; + unsigned long long ull; + }; + + typedef ExplicitKeysStruct ExplicitKeysStructArr[2][2]; + + @topic + struct NoExplicitKeysStruct { + boolean b; + char c; + long l; + }; + +#define COMMON_BRANCHES \ + case one: \ + boolean b; \ + case two: \ + char c; \ + default: \ + octet o; \ + + @topic + union ExplicitKeyUnion switch (@key MyEnum) { + COMMON_BRANCHES + }; + + @topic + union NoExplicitKeyUnion switch (MyEnum) { + COMMON_BRANCHES + }; + + @topic + struct KeyOnlyStruct { + @key long id; + @key ExplicitKeysStruct eks; + @key NoExplicitKeysStruct neks; + @key ExplicitKeysStructArr eksarr; + @key ExplicitKeyUnion eku; + @key NoExplicitKeyUnion neku; + @key string str; + octet o; + MaskedJunk mj; + long long ll; + }; + }; From c8b6e514c4e6277e841a6eae559255a91d4fac0d Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Fri, 5 Apr 2024 11:58:54 -0500 Subject: [PATCH 09/19] Add KeyOnly test cases to the xcdr test --- tests/DCPS/Compiler/xcdr/xcdr.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/DCPS/Compiler/xcdr/xcdr.cpp b/tests/DCPS/Compiler/xcdr/xcdr.cpp index 8a58364977b..68b8443b949 100644 --- a/tests/DCPS/Compiler/xcdr/xcdr.cpp +++ b/tests/DCPS/Compiler/xcdr/xcdr.cpp @@ -309,8 +309,11 @@ template void baseline_checks_vwrite(const Encoding& encoding, const Type& value, const DataView& expected_cdr) { if (encoding.kind() == Encoding::KIND_XCDR2) { + // Compute serialized size Xcdr2ValueWriter value_writer(encoding); - vwrite(value_writer, value); + EXPECT_TRUE(vwrite(value_writer, value)); + + // Serialize ACE_Message_Block buffer(value_writer.get_serialized_size()); Serializer ser(&buffer, encoding); value_writer.set_serializer(&ser); @@ -2000,6 +2003,16 @@ void key_only_test(bool topic_type) amalgam_serializer_test_base( xcdr2, expected, wrapped_value, wrapped_result, field_filter); EXPECT_PRED_FORMAT2(assert_values, wrapped_value, wrapped_result); + + // Test vwrite with KeyOnly or NestedKeyOnly samples + Xcdr2ValueWriter value_writer(xcdr2); + EXPECT_TRUE(vwrite(value_writer, wrapped_value)); + + ACE_Message_Block buffer(value_writer.get_serialized_size()); + Serializer ser(&buffer, xcdr2); + value_writer.set_serializer(&ser); + EXPECT_TRUE(vwrite(value_writer, wrapped_value)); + EXPECT_PRED_FORMAT2(assert_DataView, expected, buffer); } void serialize_u32(DataVec& data_vec, size_t value) From a59e645bae7fdac0c92589824e4fd12469d0f3a7 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Mon, 8 Apr 2024 12:23:19 -0500 Subject: [PATCH 10/19] - Add delimiter for KeyOnly union without key - Generate KeyOnly serialization functions for union only when it is a topic type - Added and fixed xcdr test with Complex(Unkeyed/Keyed)Struct --- dds/idl/marshal_generator.cpp | 44 +++++++++++++------------- tests/DCPS/Compiler/xcdr/xcdr.cpp | 52 +++++++++++++++++++------------ 2 files changed, 55 insertions(+), 41 deletions(-) diff --git a/dds/idl/marshal_generator.cpp b/dds/idl/marshal_generator.cpp index e6c9a62c169..0a8aea3bc5a 100644 --- a/dds/idl/marshal_generator.cpp +++ b/dds/idl/marshal_generator.cpp @@ -3379,9 +3379,9 @@ namespace { serialized_size.addArg("uni", "const " + wrapper + ""); serialized_size.endArgs(); - if (has_key) { - marshal_generator::generate_dheader_code(" serialized_size_delimiter(encoding, size);\n", not_final, false); + marshal_generator::generate_dheader_code(" serialized_size_delimiter(encoding, size);\n", not_final, false); + if (has_key) { if (exten == extensibilitykind_mutable) { be_global->impl_ << " size_t mutable_running_total = 0;\n" @@ -3409,16 +3409,16 @@ namespace { insertion.addArg("uni", wrapper + ""); insertion.endArgs(); - if (has_key) { - be_global->impl_ << - " const Encoding& encoding = strm.encoding();\n" - " ACE_UNUSED_ARG(encoding);\n"; - marshal_generator::generate_dheader_code( - " serialized_size(encoding, total_size, uni);\n" - " if (!strm.write_delimiter(total_size)) {\n" - " return false;\n" - " }\n", not_final); + be_global->impl_ << + " const Encoding& encoding = strm.encoding();\n" + " ACE_UNUSED_ARG(encoding);\n"; + marshal_generator::generate_dheader_code( + " serialized_size(encoding, total_size, uni);\n" + " if (!strm.write_delimiter(total_size)) {\n" + " return false;\n" + " }\n", not_final); + if (has_key) { // EMHEADER for discriminator if (exten == extensibilitykind_mutable) { be_global->impl_ << @@ -3452,16 +3452,16 @@ namespace { extraction.addArg("uni", wrapper + "<" + cxx + ">"); extraction.endArgs(); - if (has_key) { - // DHEADER - be_global->impl_ << - " const Encoding& encoding = strm.encoding();\n" - " ACE_UNUSED_ARG(encoding);\n"; - marshal_generator::generate_dheader_code( - " if (!strm.read_delimiter(total_size)) {\n" - " return false;\n" - " }\n", not_final); + // DHEADER + be_global->impl_ << + " const Encoding& encoding = strm.encoding();\n" + " ACE_UNUSED_ARG(encoding);\n"; + marshal_generator::generate_dheader_code( + " if (!strm.read_delimiter(total_size)) {\n" + " return false;\n" + " }\n", not_final); + if (has_key) { if (exten == extensibilitykind_mutable) { // EMHEADER for discriminator be_global->impl_ << @@ -3739,7 +3739,9 @@ bool marshal_generator::gen_union(AST_Union* node, UTL_ScopedName* name, } gen_union_key_serializers(node, FieldFilter_NestedKeyOnly); - gen_union_key_serializers(node, FieldFilter_KeyOnly); + if (be_global->is_topic_type(node)) { + gen_union_key_serializers(node, FieldFilter_KeyOnly); + } TopicKeys keys(node); return generate_marshal_traits(node, cxx, exten, keys); diff --git a/tests/DCPS/Compiler/xcdr/xcdr.cpp b/tests/DCPS/Compiler/xcdr/xcdr.cpp index 68b8443b949..e40f4753dbd 100644 --- a/tests/DCPS/Compiler/xcdr/xcdr.cpp +++ b/tests/DCPS/Compiler/xcdr/xcdr.cpp @@ -2153,55 +2153,57 @@ void key_only_complex_set_base_values(Type& value, const bool include_possible_keyed = (field_filter != FieldFilter_KeyOnly) || keyed; const bool include_unkeyed = field_filter == FieldFilter_All || (!keyed && field_filter == FieldFilter_NestedKeyOnly); + const FieldFilter nested_field_filter = + field_filter == FieldFilter_All ? FieldFilter_All : FieldFilter_NestedKeyOnly; set_default(value); if (include_possible_keyed) { key_only_set_base_values( - value.unkeyed_struct_value, field_filter, false); + value.unkeyed_struct_value, nested_field_filter, false); key_only_set_base_values( - value.unkeyed_struct_array_value[0], field_filter, false); + value.unkeyed_struct_array_value[0], nested_field_filter, false); key_only_set_base_values( - value.unkeyed_struct_array_value[1], field_filter, false); + value.unkeyed_struct_array_value[1], nested_field_filter, false); /* TODO(iguessthislldo): See IDL Def value.unkeyed_struct_seq_value.length(1); key_only_set_base_values( - value.unkeyed_struct_seq_value[0], field_filter, false); + value.unkeyed_struct_seq_value[0], nested_field_filter, false); */ key_only_set_base_values( - value.keyed_struct_value, field_filter, true); + value.keyed_struct_value, nested_field_filter, true); key_only_set_base_values( - value.keyed_struct_array_value[0], field_filter, true); + value.keyed_struct_array_value[0], nested_field_filter, true); key_only_set_base_values( - value.keyed_struct_array_value[1], field_filter, true); + value.keyed_struct_array_value[1], nested_field_filter, true); /* TODO(iguessthislldo): See IDL Def value.keyed_struct_seq_value.length(1); key_only_set_base_values( - value.keyed_struct_seq_value[0], field_filter, true); + value.keyed_struct_seq_value[0], nested_field_filter, true); */ key_only_union_set_base_values( - value.unkeyed_union_value, field_filter, false); + value.unkeyed_union_value, nested_field_filter, false); key_only_union_set_base_values( - value.unkeyed_union_array_value[0], field_filter, false); + value.unkeyed_union_array_value[0], nested_field_filter, false); key_only_union_set_base_values( - value.unkeyed_union_array_value[1], field_filter, false); + value.unkeyed_union_array_value[1], nested_field_filter, false); /* TODO(iguessthislldo): See IDL Def value.unkeyed_union_seq_value.length(1); key_only_union_set_base_values( - value.unkeyed_union_seq_value[0], field_filter, false); + value.unkeyed_union_seq_value[0], nested_field_filter, false); */ key_only_union_set_base_values( - value.keyed_union_value, field_filter, true); + value.keyed_union_value, nested_field_filter, true); key_only_union_set_base_values( - value.keyed_union_array_value[0], field_filter, true); + value.keyed_union_array_value[0], nested_field_filter, true); key_only_union_set_base_values( - value.keyed_union_array_value[1], field_filter, true); + value.keyed_union_array_value[1], nested_field_filter, true); /* TODO(iguessthislldo): See IDL Def value.keyed_union_seq_value.length(1); key_only_union_set_base_values( - value.keyed_union_seq_value[0], field_filter, true); + value.keyed_union_seq_value[0], nested_field_filter, true); */ } @@ -2546,9 +2548,7 @@ void build_expected_union(DataVec& expected, FieldFilter field_filter, bool keye if (include_unkeyed) { non_keys = DataView(key_only_union_non_keys_expected_base); } - if (include_possible_keyed) { - serialize_u32(expected, keys.size + non_keys.size); - } + serialize_u32(expected, keys.size + non_keys.size); keys.copy_to(expected); non_keys.copy_to(expected); } @@ -2575,7 +2575,7 @@ void build_expected_complex_struct(DataVec& expected, FieldFilter field_filter, field_filter == FieldFilter_All ? FieldFilter_All : FieldFilter_NestedKeyOnly; if (include_possible_keyed) { - build_expected_basic_struct(all_contents, field_filter, false); + build_expected_basic_struct(all_contents, nested_field_filter, false); { DataVec array_contents; build_expected_basic_struct(array_contents, nested_field_filter, false); @@ -2726,6 +2726,18 @@ TEST(KeyTests, KeyOnly_KeyedUnion) KeyOnly, KeyOnly >(true); } +TEST(KeyTests, KeyOnly_ComplexUnkeyedStruct) +{ + key_only_test, KeyOnly >(true); +} + +TEST(KeyTests, KeyOnly_ComplexKeyedStruct) +{ + key_only_test, KeyOnly >(true); +} + // ---------------------------------------------------------------------------- int main(int argc, char* argv[]) From df62cf4c0589f8e85f89c084dcb44c97cab299c1 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Mon, 8 Apr 2024 13:27:22 -0500 Subject: [PATCH 11/19] Add delimiter when serializing KeyOnly of dynamic union with no key --- dds/DCPS/XTypes/DynamicDataImpl.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dds/DCPS/XTypes/DynamicDataImpl.cpp b/dds/DCPS/XTypes/DynamicDataImpl.cpp index 0e1b4cb1cfc..a0f8487b833 100644 --- a/dds/DCPS/XTypes/DynamicDataImpl.cpp +++ b/dds/DCPS/XTypes/DynamicDataImpl.cpp @@ -5042,10 +5042,6 @@ bool serialized_size_dynamic_union(const Encoding& encoding, size_t& size, using namespace OpenDDS::XTypes; const DDS::DynamicType_var type = union_data->type(); const DDS::DynamicType_var base_type = get_base_type(type); - if (ext == Sample::KeyOnly && !has_explicit_keys(base_type)) { - // nothing is serialized (not even a delimiter) for key-only serialization when there is no @key - return true; - } DDS::TypeDescriptor_var td; if (!get_type_descriptor(base_type, td)) { @@ -5058,6 +5054,10 @@ bool serialized_size_dynamic_union(const Encoding& encoding, size_t& size, serialized_size_delimiter(encoding, size); } + if (ext == Sample::KeyOnly && !has_explicit_keys(base_type)) { + return true; + } + // Discriminator size_t mutable_running_total = 0; DDS::DynamicType_var disc_type = get_base_type(td->discriminator_type()); @@ -5704,10 +5704,6 @@ bool serialize_dynamic_union(Serializer& ser, DDS::DynamicData_ptr data, Sample: using namespace OpenDDS::XTypes; const DDS::DynamicType_var type = data->type(); const DDS::DynamicType_var base_type = get_base_type(type); - if (ext == Sample::KeyOnly && !has_explicit_keys(base_type)) { - // nothing is serialized (not even a delimiter) for key-only serialization when there is no @key - return true; - } DDS::TypeDescriptor_var td; if (!get_type_descriptor(base_type, td)) { @@ -5725,6 +5721,10 @@ bool serialize_dynamic_union(Serializer& ser, DDS::DynamicData_ptr data, Sample: } } + if (ext == Sample::KeyOnly && !has_explicit_keys(base_type)) { + return true; + } + // Discriminator DDS::DynamicTypeMember_var dtm; if (base_type->get_member(dtm, DISCRIMINATOR_ID) != DDS::RETCODE_OK) { From 6c6b499ff161aa931691360dea379b83ee1ed103 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Tue, 9 Apr 2024 13:45:19 -0500 Subject: [PATCH 12/19] From review and fix CI --- dds/DCPS/ValueDispatcher.h | 36 ++++++++++++++----- .../Compiler/vread_vwrite/VreadVwriteTest.mpc | 2 +- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/dds/DCPS/ValueDispatcher.h b/dds/DCPS/ValueDispatcher.h index ccb810bf74a..49e1f9abcbe 100644 --- a/dds/DCPS/ValueDispatcher.h +++ b/dds/DCPS/ValueDispatcher.h @@ -9,6 +9,7 @@ #include "TypeSupportImpl.h" #include "ValueReader.h" #include "ValueWriter.h" +#include "Sample.h" OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL @@ -21,8 +22,8 @@ struct OpenDDS_Dcps_Export ValueDispatcher { virtual void* new_value() const = 0; virtual void delete_value(void* data) const = 0; - virtual bool read(ValueReader& value_reader, void* data, bool key_only = false) const = 0; - virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const = 0; + virtual bool read(ValueReader& value_reader, void* data, Sample::Extent ext = Sample::Full) const = 0; + virtual bool write(ValueWriter& value_writer, const void* data, Sample::Extent ext = Sample::Full) const = 0; virtual DDS::InstanceHandle_t register_instance_helper(DDS::DataWriter* dw, const void* data) const = 0; virtual DDS::ReturnCode_t write_helper(DDS::DataWriter* dw, const void* data, DDS::InstanceHandle_t inst) const = 0; @@ -45,23 +46,40 @@ struct ValueDispatcher_T : public virtual ValueDispatcher { delete tbd; } - virtual bool read(ValueReader& value_reader, void* data, bool key_only = false) const + typedef typename OpenDDS::DCPS::DDSTraits TraitsType; + + virtual bool read(ValueReader& value_reader, void* data, Sample::Extent ext = Sample::Full) const { - if (key_only) { + switch (ext) { + case Sample::Full: + return vread(value_reader, *static_cast(data)); + case Sample::KeyOnly: return vread(value_reader, *static_cast*>(data)); + default: + if (log_level >= LogLevel::Notice) { + ACE_ERROR((LM_NOTICE, "(%P|%t) NOTICE: ValueDispatcher_T<%C>::read:" + " Called with Sample::Extent NestedKeyOnly\n", TraitsType::type_name())); + } + return false; } - return vread(value_reader, *static_cast(data)); } - virtual bool write(ValueWriter& value_writer, const void* data, bool key_only = false) const + virtual bool write(ValueWriter& value_writer, const void* data, Sample::Extent ext = Sample::Full) const { - if (key_only) { + switch (ext) { + case Sample::Full: + return vwrite(value_writer, *static_cast(data)); + case Sample::KeyOnly: return vwrite(value_writer, *static_cast*>(data)); + default: + if (log_level >= LogLevel::Notice) { + ACE_ERROR((LM_NOTICE, "(%P|%t) NOTICE: ValueDispatcher_T<%C>::write:" + " Called with Sample::Extent NestedKeyOnly\n", TraitsType::type_name())); + } + return false; } - return vwrite(value_writer, *static_cast(data)); } - typedef typename OpenDDS::DCPS::DDSTraits TraitsType; typedef typename TraitsType::DataWriterType DataWriterType; virtual DDS::InstanceHandle_t register_instance_helper(DDS::DataWriter* dw, const void* data) const diff --git a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.mpc b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.mpc index f76607e36e5..f595046dce5 100644 --- a/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.mpc +++ b/tests/DCPS/Compiler/vread_vwrite/VreadVwriteTest.mpc @@ -1,4 +1,4 @@ -project: dcpsexe, dcps_test, googletest, rapidjson, opendds_uses_cxx11 { +project: dcpsexe, dcps_test, googletest, rapidjson, opendds_uses_cxx11, msvc_bigobj { dcps_ts_flags += -Gxtypes-complete exename = * From c3793d2c4a4dbb39c5f339b4d2bab1a93f771985 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Wed, 10 Apr 2024 11:20:37 -0500 Subject: [PATCH 13/19] Add header --- dds/DCPS/ValueDispatcher.h | 1 + 1 file changed, 1 insertion(+) diff --git a/dds/DCPS/ValueDispatcher.h b/dds/DCPS/ValueDispatcher.h index 49e1f9abcbe..ca723287be2 100644 --- a/dds/DCPS/ValueDispatcher.h +++ b/dds/DCPS/ValueDispatcher.h @@ -10,6 +10,7 @@ #include "ValueReader.h" #include "ValueWriter.h" #include "Sample.h" +#include "debug.h" OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL From 427ce89b0cd750f79bcaa2833fdb7ebbb2ebbd83 Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Thu, 11 Apr 2024 01:20:40 -0500 Subject: [PATCH 14/19] Use RefWrapper. Add a news fragment. --- dds/idl/dds_generator.h | 56 +++++++++++----------------- dds/idl/marshal_generator.cpp | 2 +- dds/idl/value_reader_generator.cpp | 14 +++---- dds/idl/value_writer_generator.cpp | 18 ++++----- docs/news.d/keyonly_vread_vwrite.rst | 5 +++ 5 files changed, 43 insertions(+), 52 deletions(-) create mode 100644 docs/news.d/keyonly_vread_vwrite.rst diff --git a/dds/idl/dds_generator.h b/dds/idl/dds_generator.h index a8b3fd0e3f6..d2e83f2ef98 100644 --- a/dds/idl/dds_generator.h +++ b/dds/idl/dds_generator.h @@ -1445,36 +1445,6 @@ inline std::string type_kind(AST_Type* type) } } -inline -std::string key_only_type_name(const std::string& type_name, FieldFilter field_filter, bool writing) -{ - std::string wrapped_type_name; - switch (field_filter) { - case FieldFilter_All: - if (writing) { - wrapped_type_name = "const" + type_name; - } else { - wrapped_type_name = type_name; - } - break; - case FieldFilter_NestedKeyOnly: - if (writing) { - wrapped_type_name = "const NestedKeyOnly"; - } else { - wrapped_type_name = "const NestedKeyOnly<" + type_name + ">"; - } - break; - case FieldFilter_KeyOnly: - if (writing) { - wrapped_type_name = "const KeyOnly"; - } else { - wrapped_type_name = "const KeyOnly<" + type_name + ">"; - } - break; - } - return wrapped_type_name; -} - inline FieldFilter nested(FieldFilter filter_kind) { @@ -1490,7 +1460,7 @@ bool has_discriminator(AST_Union* u, FieldFilter filter_kind) } /// Handling wrapping and unwrapping references in the wrapper types: -/// NestedKeyOnly, IDL::DistinctType, and *_forany. +/// NestedKeyOnly, KeyOnly, IDL::DistinctType, and *_forany. struct RefWrapper { const bool cpp11_; AST_Type* const type_; @@ -1500,6 +1470,7 @@ struct RefWrapper { const std::string fieldref_; const std::string local_; bool is_const_; + FieldFilter field_filter_; bool nested_key_only_; bool classic_array_copy_; bool dynamic_data_adapter_; @@ -1514,6 +1485,7 @@ struct RefWrapper { , to_wrap_(strip_shift_op(to_wrap)) , shift_op_(get_shift_op(to_wrap)) , is_const_(is_const) + , field_filter_(FieldFilter_All) , nested_key_only_(false) , classic_array_copy_(false) , dynamic_data_adapter_(false) @@ -1532,6 +1504,7 @@ struct RefWrapper { , fieldref_(strip_shift_op(fieldref)) , local_(local) , is_const_(is_const) + , field_filter_(FieldFilter_All) , nested_key_only_(false) , classic_array_copy_(false) , dynamic_data_adapter_(false) @@ -1552,8 +1525,9 @@ struct RefWrapper { const bool forany = classic_array_copy_ || needs_forany(type_); const bool distinct_type = needs_distinct_type(type_); needs_dda_tag_ = dynamic_data_adapter_ && (forany || distinct_type); - nested_key_only_ = nested_key_only_ && - needs_nested_key_only(typedef_node_ ? typedef_node_ : type_); + // If field_filter_ is set, this object is being used for vwrite or vread generator. + nested_key_only_ = field_filter_ == FieldFilter_NestedKeyOnly || + (nested_key_only_ && needs_nested_key_only(typedef_node_ ? typedef_node_ : type_)); wrapped_type_name_ = type_name_; bool by_ref = true; @@ -1590,9 +1564,12 @@ struct RefWrapper { ref_ = var_name; } + if (field_filter_ == FieldFilter_KeyOnly) { + wrapped_type_name_ = std::string("KeyOnly<") + const_str + wrapped_type_name_ + ">"; + } + if (nested_key_only_) { - wrapped_type_name_ = - std::string("NestedKeyOnly<") + const_str + wrapped_type_name_ + ">"; + wrapped_type_name_ = std::string("NestedKeyOnly<") + const_str + wrapped_type_name_ + ">"; value_access_post_ = ".value" + value_access_post_; const std::string nko_arg = "(" + ref_ + ")"; if (is_const_) { @@ -1737,4 +1714,13 @@ struct RefWrapper { } }; +inline +std::string key_only_type_name(AST_Type* type, const std::string& type_name, + FieldFilter field_filter, bool writing) +{ + RefWrapper wrapper(type, type_name, "", writing ? true : false); + wrapper.field_filter_ = field_filter; + return wrapper.done().wrapped_type_name(); +} + #endif diff --git a/dds/idl/marshal_generator.cpp b/dds/idl/marshal_generator.cpp index ec7bf787793..e2fd0c9a93a 100644 --- a/dds/idl/marshal_generator.cpp +++ b/dds/idl/marshal_generator.cpp @@ -1565,7 +1565,7 @@ bool marshal_generator::gen_typedef(AST_Typedef* node, UTL_ScopedName* name, AST namespace { // common to both fields (in structs) and branches (in unions) - string findSizeCommon(const std::string& indent, AST_Decl*field, const string& name, + string findSizeCommon(const std::string& indent, AST_Decl* field, const string& name, AST_Type* type, const string& prefix, bool wrap_nested_key_only, Intro& intro, const string& = "") // same sig as streamCommon { diff --git a/dds/idl/value_reader_generator.cpp b/dds/idl/value_reader_generator.cpp index 74c8ceb5fb9..1085e8c2999 100644 --- a/dds/idl/value_reader_generator.cpp +++ b/dds/idl/value_reader_generator.cpp @@ -19,9 +19,9 @@ using namespace AstTypeClassification; namespace { -void generate_read(const std::string& expression, const std::string& accessor, - const std::string& field_name, AST_Type* type, const std::string& idx, - int level = 1, FieldFilter field_filter = FieldFilter_All); + void generate_read(const std::string& expression, const std::string& accessor, + const std::string& field_name, AST_Type* type, const std::string& idx, + int level = 1, FieldFilter field_filter = FieldFilter_All); std::string primitive_type(AST_PredefinedType::PredefinedType pt) { @@ -244,10 +244,10 @@ void generate_read(const std::string& expression, const std::string& accessor, bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { - const std::string wrapped_name = key_only_type_name(type_name, field_filter, false); + const std::string wrapped_name = key_only_type_name(node, type_name, field_filter, false); Function read("vread", "bool"); read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); - read.addArg("value", wrapped_name + "&"); + read.addArg("value", wrapped_name); read.endArgs(); be_global->impl_ << @@ -298,10 +298,10 @@ void generate_read(const std::string& expression, const std::string& accessor, const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { - const std::string wrapped_name = key_only_type_name(type_name, filter_kind, false); + const std::string wrapped_name = key_only_type_name(u, type_name, filter_kind, false); Function read("vread", "bool"); read.addArg("value_reader", "OpenDDS::DCPS::ValueReader&"); - read.addArg("value", wrapped_name + "&"); + read.addArg("value", wrapped_name); read.endArgs(); const std::string value_prefix = filter_kind == FieldFilter_All ? "value." : "value.value."; diff --git a/dds/idl/value_writer_generator.cpp b/dds/idl/value_writer_generator.cpp index 252ce892f03..4c27ffcf899 100644 --- a/dds/idl/value_writer_generator.cpp +++ b/dds/idl/value_writer_generator.cpp @@ -18,9 +18,9 @@ using namespace AstTypeClassification; namespace { -void generate_write(const std::string& expression, const std::string& field_name, - AST_Type* type, const std::string& idx, int level = 1, - FieldFilter field_filter = FieldFilter_All); + void generate_write(const std::string& expression, const std::string& field_name, + AST_Type* type, const std::string& idx, int level = 1, + FieldFilter field_filter = FieldFilter_All); std::string primitive_type(AST_PredefinedType::PredefinedType pt) { @@ -177,8 +177,8 @@ void generate_write(const std::string& expression, const std::string& field_name indent << "}\n"; } -void generate_write(const std::string& expression, const std::string& field_name, - AST_Type* type, const std::string& idx, int level, FieldFilter field_filter) + void generate_write(const std::string& expression, const std::string& field_name, + AST_Type* type, const std::string& idx, int level, FieldFilter field_filter) { AST_Type* const actual = resolveActualType(type); @@ -271,10 +271,10 @@ void generate_write(const std::string& expression, const std::string& field_name bool gen_struct_i(AST_Structure* node, const std::string& type_name, bool use_cxx11, ExtensibilityKind ek, FieldFilter field_filter) { - const std::string wrapped_name = key_only_type_name(type_name, field_filter, true); + const std::string wrapped_name = key_only_type_name(node, type_name, field_filter, true); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", wrapped_name + "&"); + write.addArg("value", wrapped_name); write.endArgs(); const std::string value_prefix = field_filter == FieldFilter_All ? "value." : "value.value."; @@ -313,10 +313,10 @@ void generate_write(const std::string& expression, const std::string& field_name const std::vector& branches, AST_Type* discriminator, ExtensibilityKind ek, FieldFilter filter_kind) { - const std::string wrapped_name = key_only_type_name(type_name, filter_kind, true); + const std::string wrapped_name = key_only_type_name(u, type_name, filter_kind, true); Function write("vwrite", "bool"); write.addArg("value_writer", "OpenDDS::DCPS::ValueWriter&"); - write.addArg("value", wrapped_name + "&"); + write.addArg("value", wrapped_name); write.endArgs(); const std::string value_prefix = filter_kind == FieldFilter_All ? "value." : "value.value."; diff --git a/docs/news.d/keyonly_vread_vwrite.rst b/docs/news.d/keyonly_vread_vwrite.rst new file mode 100644 index 00000000000..e4c449ccf2a --- /dev/null +++ b/docs/news.d/keyonly_vread_vwrite.rst @@ -0,0 +1,5 @@ +.. news-prs: 4554 + +.. news-start-section: Fixes +- KeyOnly serialization of union that has no key now has a delimiter for appendable and mutable extensibility. +.. news-end-section From 379fe7e16b464c131dfe587679353f7957ef84aa Mon Sep 17 00:00:00 2001 From: Son Dinh Date: Thu, 11 Apr 2024 11:22:42 -0500 Subject: [PATCH 15/19] Fix CI and from review --- dds/idl/dds_generator.h | 8 +++++++- docs/news.d/keyonly_vread_vwrite.rst | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dds/idl/dds_generator.h b/dds/idl/dds_generator.h index d2e83f2ef98..e6ca05b9a09 100644 --- a/dds/idl/dds_generator.h +++ b/dds/idl/dds_generator.h @@ -1459,6 +1459,11 @@ bool has_discriminator(AST_Union* u, FieldFilter filter_kind) || filter_kind == FieldFilter_All; } +// TODO: Add more fine-grained control of "const" string for the wrapper type and wrapped type. +// Currently, there is a single bool to control both; that is, either both are "const" or +// none is "const". But sometimes, we want something like "const KeyOnly&", and +// not "const KeyOnly&" or "KeyOnly&". + /// Handling wrapping and unwrapping references in the wrapper types: /// NestedKeyOnly, KeyOnly, IDL::DistinctType, and *_forany. struct RefWrapper { @@ -1720,7 +1725,8 @@ std::string key_only_type_name(AST_Type* type, const std::string& type_name, { RefWrapper wrapper(type, type_name, "", writing ? true : false); wrapper.field_filter_ = field_filter; - return wrapper.done().wrapped_type_name(); + const bool has_wrapper = field_filter != FieldFilter_All; + return (has_wrapper && !writing ? "const " : "") + wrapper.done().wrapped_type_name(); } #endif diff --git a/docs/news.d/keyonly_vread_vwrite.rst b/docs/news.d/keyonly_vread_vwrite.rst index e4c449ccf2a..774438d6ede 100644 --- a/docs/news.d/keyonly_vread_vwrite.rst +++ b/docs/news.d/keyonly_vread_vwrite.rst @@ -1,5 +1,5 @@ .. news-prs: 4554 .. news-start-section: Fixes -- KeyOnly serialization of union that has no key now has a delimiter for appendable and mutable extensibility. +- XCDR2 KeyOnly serialization of union that has no key now has a delimiter for appendable and mutable extensibility. .. news-end-section From d005c83bec7725d768044c01c8739b1ed064fa11 Mon Sep 17 00:00:00 2001 From: Fred Hornsey Date: Wed, 10 Apr 2024 12:49:46 -0500 Subject: [PATCH 16/19] Pass Xerces Path to ACE/TAO in CMake --- .github/workflows/cmake.yml | 26 ++++++++++++++++++++++++++ cmake/ace_group.cmake | 10 ---------- cmake/build_ace_tao.cmake | 23 ++++++++++++++--------- cmake/init.cmake | 13 +++++++++++++ docs/news.d/cmake-xerces.rst | 7 +++++++ 5 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 docs/news.d/cmake-xerces.rst diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 1ee1638748e..d6e796b7682 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -289,6 +289,8 @@ jobs: -DBUILD_SHARED_LIBS=TRUE^ -DOPENDDS_ACE_TAO_SRC=${{ github.workspace }}\OpenDDS\build\ACE_TAO^ -DOPENDDS_MPC=${{ github.workspace }}\MPC^ + -DOPENDDS_XERCES3=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ + -DOPENDDS_OPENSSL=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ -DOPENDDS_BUILD_TESTS=TRUE^ -DOPENDDS_CMAKE_VERBOSE=all - uses: ammaraskar/msvc-problem-matcher@0.3.0 @@ -308,6 +310,15 @@ jobs: runs-on: windows-2022 steps: + - name: setup for run-vcpkg + shell: bash + run: | + echo '{ "name": "opendds", "version-string": "github-actions", "dependencies": [ "openssl", "xerces-c" ] }' > vcpkg.json + - name: install openssl & xerces-c + uses: lukka/run-vcpkg@v11 + with: + vcpkgGitCommitId: '${{ env.VCPKG_GIT_COMMIT }}' + runVcpkgInstall: true - name: Checkout OpenDDS uses: actions/checkout@v4 with: @@ -334,6 +345,8 @@ jobs: -DBUILD_SHARED_LIBS=FALSE^ -DOPENDDS_ACE_TAO_SRC=${{ github.workspace }}\OpenDDS\build\ACE_TAO^ -DOPENDDS_MPC=${{ github.workspace }}\MPC^ + -DOPENDDS_XERCES3=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ + -DOPENDDS_OPENSSL=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ -DOPENDDS_BUILD_TESTS=TRUE^ -DOPENDDS_CMAKE_VERBOSE=all - uses: ammaraskar/msvc-problem-matcher@0.3.0 @@ -389,6 +402,8 @@ jobs: -DBUILD_SHARED_LIBS=TRUE^ -DOPENDDS_ACE_TAO_SRC=${{ github.workspace }}\OpenDDS\build\ACE_TAO^ -DOPENDDS_MPC=${{ github.workspace }}\MPC^ + -DOPENDDS_XERCES3=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ + -DOPENDDS_OPENSSL=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ -DOPENDDS_BUILD_TESTS=TRUE^ -DOPENDDS_CMAKE_VERBOSE=all - uses: ammaraskar/msvc-problem-matcher@0.3.0 @@ -408,6 +423,15 @@ jobs: runs-on: windows-2022 steps: + - name: setup for run-vcpkg + shell: bash + run: | + echo '{ "name": "opendds", "version-string": "github-actions", "dependencies": [ "openssl", "xerces-c" ] }' > vcpkg.json + - name: install openssl & xerces-c + uses: lukka/run-vcpkg@v11 + with: + vcpkgGitCommitId: '${{ env.VCPKG_GIT_COMMIT }}' + runVcpkgInstall: true - name: Checkout OpenDDS uses: actions/checkout@v4 with: @@ -434,6 +458,8 @@ jobs: -DBUILD_SHARED_LIBS=FALSE^ -DOPENDDS_ACE_TAO_SRC=${{ github.workspace }}\OpenDDS\build\ACE_TAO^ -DOPENDDS_MPC=${{ github.workspace }}\MPC^ + -DOPENDDS_XERCES3=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ + -DOPENDDS_OPENSSL=${{ env.VCPKG_INSTALLED_DIR }}/x64-windows^ -DOPENDDS_BUILD_TESTS=TRUE^ -DOPENDDS_CMAKE_VERBOSE=all - uses: ammaraskar/msvc-problem-matcher@0.3.0 diff --git a/cmake/ace_group.cmake b/cmake/ace_group.cmake index a9d2218368b..6105b1b9733 100644 --- a/cmake/ace_group.cmake +++ b/cmake/ace_group.cmake @@ -38,16 +38,6 @@ _opendds_group_exe(ace_gperf EXTRA_BIN_DIRS "${TAO_BIN_DIR}" ) -if(OPENDDS_XERCES3) - find_package(XercesC PATHS "${OPENDDS_XERCES3}" NO_DEFAULT_PATH QUIET) - if(NOT XercesC_FOUND) - find_package(XercesC QUIET) - endif() - if(NOT XercesC_FOUND) - message(FATAL_ERROR "Could not find XercesC") - endif() -endif() - function(_opendds_vs_force_static) # Make sure the MSVC runtime library, which is similar to libc of other # systems, is the same kind everywhere. Normally we shouldn't make global diff --git a/cmake/build_ace_tao.cmake b/cmake/build_ace_tao.cmake index 843d63935ed..eb2bcedc8e3 100644 --- a/cmake/build_ace_tao.cmake +++ b/cmake/build_ace_tao.cmake @@ -35,6 +35,11 @@ execute_process( COMMAND_ERROR_IS_FATAL ANY ) +set(_build_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") +if(_OPENDDS_XERCES3_FOR_ACE) + list(APPEND _build_cmd "XERCESCROOT=${_OPENDDS_XERCES3_FOR_ACE}") +endif() + if(_OPENDDS_MPC_TYPE STREQUAL gnuace) execute_process( COMMAND @@ -65,20 +70,19 @@ if(_OPENDDS_MPC_TYPE STREQUAL gnuace) list(APPEND byproducts "${file}") endforeach() - set(make_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") if(CMAKE_GENERATOR STREQUAL "Unix Makefiles") - list(APPEND make_cmd "$(MAKE)") + list(APPEND _build_cmd "$(MAKE)") else() find_program(make_exe NAMES gmake make) cmake_host_system_information(RESULT core_count QUERY NUMBER_OF_LOGICAL_CORES) - list(APPEND make_cmd "${make_exe}" "-j${core_count}") + list(APPEND _build_cmd "${make_exe}" "-j${core_count}") endif() ExternalProject_Add(build_ace_tao SOURCE_DIR "${OPENDDS_ACE_TAO_SRC}" CONFIGURE_COMMAND "${CMAKE_COMMAND}" -E echo "Already configured" BUILD_IN_SOURCE TRUE - BUILD_COMMAND ${make_cmd} + BUILD_COMMAND ${_build_cmd} BUILD_BYPRODUCTS ${byproducts} USES_TERMINAL_BUILD TRUE # Needed for Ninja to show the ACE/TAO build INSTALL_COMMAND "${CMAKE_COMMAND}" -E echo "No install step" @@ -123,15 +127,16 @@ elseif(_OPENDDS_MPC_TYPE MATCHES "^vs") endif() endforeach() + list(APPEND _build_cmd + MSBuild "${sln}" "-maxcpucount" + "-property:Configuration=$,Platform=${CMAKE_VS_PLATFORM_TOOLSET_HOST_ARCHITECTURE}" + ) + ExternalProject_Add(build_ace_tao SOURCE_DIR "${OPENDDS_ACE_TAO_SRC}" CONFIGURE_COMMAND "${CMAKE_COMMAND}" -E echo "Already configured" BUILD_IN_SOURCE TRUE - BUILD_COMMAND - "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}" - MSBuild "${sln}" - "-property:Configuration=$,Platform=${CMAKE_VS_PLATFORM_TOOLSET_HOST_ARCHITECTURE}" - "-maxcpucount" + BUILD_COMMAND ${_build_cmd} BUILD_BYPRODUCTS ${byproducts} USES_TERMINAL_BUILD TRUE # Needed for Ninja to show the ACE/TAO build INSTALL_COMMAND "${CMAKE_COMMAND}" -E echo "No install step" diff --git a/cmake/init.cmake b/cmake/init.cmake index d5437793771..87b158aadac 100644 --- a/cmake/init.cmake +++ b/cmake/init.cmake @@ -374,3 +374,16 @@ if(NOT DEFINED OPENDDS_SUPPORTS_SHMEM) set(OPENDDS_SUPPORTS_SHMEM TRUE) endif() endif() + +# This should be in ace_group.cmake, but it's needed by build_ace_tao.cmake. +if(OPENDDS_XERCES3) + find_package(XercesC PATHS "${OPENDDS_XERCES3}" NO_DEFAULT_PATH QUIET) + if(NOT XercesC_FOUND) + find_package(XercesC QUIET) + endif() + if(NOT XercesC_FOUND) + message(FATAL_ERROR "Could not find XercesC") + endif() + get_filename_component(_opendds_xerces3_for_ace "${XercesC_INCLUDE_DIRS}" DIRECTORY) + set(_OPENDDS_XERCES3_FOR_ACE "${_opendds_xerces3_for_ace}" CACHE PATH "" FORCE) +endif() diff --git a/docs/news.d/cmake-xerces.rst b/docs/news.d/cmake-xerces.rst new file mode 100644 index 00000000000..3b2cf84f370 --- /dev/null +++ b/docs/news.d/cmake-xerces.rst @@ -0,0 +1,7 @@ +.. news-prs: 4572 + +.. news-start-section: Platform Support and Dependencies +.. news-start-section: Building with CMake +- Fixed :ghissue:`ACE/TAO build not getting Xerces path <4375>` when using :cmake:var:`OPENDDS_XERCES3`. +.. news-end-section +.. news-end-section From 54362154823d81c3ee68946ad5b3a8cd64aac15b Mon Sep 17 00:00:00 2001 From: Fred Hornsey Date: Wed, 10 Apr 2024 17:00:03 -0500 Subject: [PATCH 17/19] Fix for building with Xerces master branch https://github.com/apache/xerces-c/pull/12 --- dds/DCPS/QOS_XML_Handler/XML_String_Intf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dds/DCPS/QOS_XML_Handler/XML_String_Intf.h b/dds/DCPS/QOS_XML_Handler/XML_String_Intf.h index 1f7789f16a0..e736a17d3e9 100644 --- a/dds/DCPS/QOS_XML_Handler/XML_String_Intf.h +++ b/dds/DCPS/QOS_XML_Handler/XML_String_Intf.h @@ -28,10 +28,10 @@ namespace XML class XML_Error_Handler; } -XERCES_CPP_NAMESPACE_BEGIN +namespace XERCES_CPP_NAMESPACE { class XercesDOMParser; class DOMDocument; -XERCES_CPP_NAMESPACE_END +} OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL From 8b529a326f5891732193e641cda0567e6eb9d38d Mon Sep 17 00:00:00 2001 From: Fred Hornsey Date: Thu, 11 Apr 2024 00:43:59 -0500 Subject: [PATCH 18/19] Workaround Vcpkg, Add Missing BIGOBJ --- cmake/build_ace_tao.cmake | 17 ++++++++++++----- cmake/configure_ace_tao.pl | 9 +++++++++ tools/dds/rtpsrelaylib/CMakeLists.txt | 2 +- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/cmake/build_ace_tao.cmake b/cmake/build_ace_tao.cmake index eb2bcedc8e3..c2f36750ea1 100644 --- a/cmake/build_ace_tao.cmake +++ b/cmake/build_ace_tao.cmake @@ -16,6 +16,18 @@ if(OPENDDS_JUST_BUILD_HOST_TOOLS) list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS "--workspace-file=${ws}") endif() +set(_build_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") +if(_OPENDDS_XERCES3_FOR_ACE) + list(APPEND _build_cmd "XERCESCROOT=${_OPENDDS_XERCES3_FOR_ACE}") + + # Vcpkg debug libraries are in a non-standard place + set(_opendds_debug_vcpkg_xerces3_for_ace "${_OPENDDS_XERCES3_FOR_ACE}/debug/lib") + if(MSVC AND IS_DIRECTORY "${_opendds_debug_vcpkg_xerces3_for_ace}") + list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS + --env "XERCESC_LIBDIR=${_opendds_debug_vcpkg_xerces3_for_ace}") + endif() +endif() + find_package(Perl REQUIRED) if(OPENDDS_STATIC) list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS --static=1) @@ -35,11 +47,6 @@ execute_process( COMMAND_ERROR_IS_FATAL ANY ) -set(_build_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") -if(_OPENDDS_XERCES3_FOR_ACE) - list(APPEND _build_cmd "XERCESCROOT=${_OPENDDS_XERCES3_FOR_ACE}") -endif() - if(_OPENDDS_MPC_TYPE STREQUAL gnuace) execute_process( COMMAND diff --git a/cmake/configure_ace_tao.pl b/cmake/configure_ace_tao.pl index 0add2d4e216..773f7508408 100755 --- a/cmake/configure_ace_tao.pl +++ b/cmake/configure_ace_tao.pl @@ -33,6 +33,8 @@ push(@opts, "$key=s"); } push(@opts, "macro-line=s@"); +my %env; +push(@opts, "env=s%" => %env); if (!GetOptions(\%values, @opts)) { exit(1); } @@ -113,6 +115,13 @@ sub read_file { $ENV{MPC_ROOT} = File::Spec->rel2abs($values{mpc}); $ENV{ACE_ROOT} = File::Spec->rel2abs($values{ace}); $ENV{TAO_ROOT} = File::Spec->rel2abs($values{tao}); +if ($values{'env'}) { + for my $name (keys(%{$values{'env'}})) { + my $value = $values{env}->{$name}; + print("env: $name=$value\n"); + $ENV{$name} = $value; + } +} my $mwc_name = 'ACE_TAO_for_OpenDDS.mwc'; my $mwc_src = $values{'workspace-file'} // "$FindBin::RealBin/../$mwc_name"; my $mwc = "$values{src}/$mwc_name"; diff --git a/tools/dds/rtpsrelaylib/CMakeLists.txt b/tools/dds/rtpsrelaylib/CMakeLists.txt index 9f3a1925bf6..2561ccc9c79 100644 --- a/tools/dds/rtpsrelaylib/CMakeLists.txt +++ b/tools/dds/rtpsrelaylib/CMakeLists.txt @@ -18,7 +18,7 @@ target_sources(OpenDDS_RtpsRelayLib Utility.h export.h ) -_opendds_library(OpenDDS_RtpsRelayLib) +_opendds_library(OpenDDS_RtpsRelayLib MSVC_BIGOBJ) target_link_libraries(OpenDDS_RtpsRelayLib PUBLIC ${deps}) opendds_target_sources(OpenDDS_RtpsRelayLib From 191f25529e8a63791074504b9dae58f30f9c15d5 Mon Sep 17 00:00:00 2001 From: Fred Hornsey Date: Thu, 11 Apr 2024 17:34:12 -0500 Subject: [PATCH 19/19] Workaround doesn't work on Release https://github.com/DOCGroup/MPC/pull/202 --- cmake/build_ace_tao.cmake | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/cmake/build_ace_tao.cmake b/cmake/build_ace_tao.cmake index c2f36750ea1..eb2bcedc8e3 100644 --- a/cmake/build_ace_tao.cmake +++ b/cmake/build_ace_tao.cmake @@ -16,18 +16,6 @@ if(OPENDDS_JUST_BUILD_HOST_TOOLS) list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS "--workspace-file=${ws}") endif() -set(_build_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") -if(_OPENDDS_XERCES3_FOR_ACE) - list(APPEND _build_cmd "XERCESCROOT=${_OPENDDS_XERCES3_FOR_ACE}") - - # Vcpkg debug libraries are in a non-standard place - set(_opendds_debug_vcpkg_xerces3_for_ace "${_OPENDDS_XERCES3_FOR_ACE}/debug/lib") - if(MSVC AND IS_DIRECTORY "${_opendds_debug_vcpkg_xerces3_for_ace}") - list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS - --env "XERCESC_LIBDIR=${_opendds_debug_vcpkg_xerces3_for_ace}") - endif() -endif() - find_package(Perl REQUIRED) if(OPENDDS_STATIC) list(APPEND _OPENDDS_CONFIGURE_ACE_TAO_ARGS --static=1) @@ -47,6 +35,11 @@ execute_process( COMMAND_ERROR_IS_FATAL ANY ) +set(_build_cmd "${CMAKE_COMMAND}" -E env "ACE_ROOT=${OPENDDS_ACE}" "TAO_ROOT=${OPENDDS_TAO}") +if(_OPENDDS_XERCES3_FOR_ACE) + list(APPEND _build_cmd "XERCESCROOT=${_OPENDDS_XERCES3_FOR_ACE}") +endif() + if(_OPENDDS_MPC_TYPE STREQUAL gnuace) execute_process( COMMAND