From 5ddfdce24ced9b4a085ed7e3d83a62edf0709b75 Mon Sep 17 00:00:00 2001 From: Dave Allison Date: Fri, 28 Aug 2026 15:52:20 -0700 Subject: [PATCH 1/3] Improve ROS compatibility and clean compiler warnings Expand Phaser's payload-backed fields and ROS code generation for source metadata, service-only schemas, narrow wire types, and safer container operations. Keep clean GCC and Clang builds focused on actionable diagnostics. --- .bazelrc | 4 + MODULE.bazel.lock | 53 -- phaser/BUILD.bazel | 17 +- phaser/compiler/BUILD.bazel | 6 +- phaser/compiler/gen.cc | 31 +- phaser/compiler/message_gen.cc | 296 ++++++-- phaser/compiler/message_gen.h | 2 +- phaser/copts.bzl | 36 +- phaser/options.proto | 10 + phaser/phaser_library.bzl | 3 - phaser/ros_compile_test.cc | 150 +++- phaser/ros_metadata_protobuf_frontend_test.cc | 24 +- phaser/ros_metadata_ros_frontend_test.cc | 20 +- phaser/ros_wire_conversion_test.cc | 217 +++--- phaser/runtime/BUILD.bazel | 8 +- phaser/runtime/any.h | 9 + phaser/runtime/arrays.h | 5 +- phaser/runtime/fields.h | 275 ++++++- phaser/runtime/iterators.h | 112 ++- phaser/runtime/message.h | 4 + phaser/runtime/message_test.cc | 6 + phaser/runtime/ros_wireformat_test.cc | 2 +- phaser/runtime/vectors.h | 702 +++++++++++++++--- phaser/runtime/wireformat.h | 4 +- phaser/stress_test.cc | 12 +- phaser/testdata/BUILD | 18 +- phaser/testdata/RosCompile.proto | 30 + phaser/testdata/RosMetadata.proto | 17 + phaser/testdata/ServiceOnly.proto | 9 + 29 files changed, 1660 insertions(+), 422 deletions(-) create mode 100644 phaser/testdata/ServiceOnly.proto diff --git a/.bazelrc b/.bazelrc index 1290f59..6dcb767 100644 --- a/.bazelrc +++ b/.bazelrc @@ -34,6 +34,10 @@ build --enable_platform_specific_config # warning set used for our own code (see phaser/copts.bzl) does not flood the # build with diagnostics from abseil/protobuf/googletest/toolbelt. build --features=external_include_paths +# Dependency sources are outside Phaser's control. Keep clean builds focused on +# diagnostics actionable in this repository for both target and tool builds. +build --per_file_copt=^external/.*@-w +build --host_per_file_copt=^external/.*@-w # For all builds, use C++17 build --cxxopt="-std=c++17" diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 4618626..009208c 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -276,59 +276,6 @@ }, "selectedYankedVersions": {}, "moduleExtensions": { - "@@protobuf+//python/dist:system_python.bzl%system_python_extension": { - "general": { - "bzlTransitiveDigest": "qh0n9IrXU/xS94wxKQrG1J63zrLkA1Wy2Y3BQxptPcI=", - "usagesDigest": "AF5a9lHFrJtHw1GTt3jJOs7ZYl1+N2bkn1LbPFLoguA=", - "recordedInputs": [], - "generatedRepoSpecs": { - "system_python": { - "repoRuleId": "@@protobuf+//python/dist:system_python.bzl%system_python", - "attributes": { - "minimum_python_version": "3.9" - } - } - } - } - }, - "@@pybind11_bazel+//:internal_configure.bzl%internal_configure_extension": { - "general": { - "bzlTransitiveDigest": "Ilz4hu4VWEbx3OM4ZIpgYmYXuPq6ewOVgzv5F0ziWS8=", - "usagesDigest": "tVQNvLoXMWAbiK39am3yovKGpwINdftfn7RpDyN+JZc=", - "recordedInputs": [ - "REPO_MAPPING:pybind11_bazel+,bazel_tools bazel_tools" - ], - "generatedRepoSpecs": { - "pybind11": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "build_file": "@@pybind11_bazel+//:pybind11-BUILD.bazel", - "strip_prefix": "pybind11-2.13.6", - "url": "https://github.com/pybind/pybind11/archive/refs/tags/v2.13.6.tar.gz", - "integrity": "sha256-4Iy4f0dz2pf6e18DXeh2OrxlbYfVdz5i9toFh9Hw7CA=" - } - } - } - } - }, - "@@rules_android+//bzlmod_extensions:apksig.bzl%apksig_extension": { - "general": { - "bzlTransitiveDigest": "IiT2UgJGnHaKiyP2A1yh3U/QWN4W9g/Byolrm78hC/s=", - "usagesDigest": "0FXD4PX+vQ/jVne2oV4v3Cw5Mc9DZQ4yTcoRkAjj/X4=", - "recordedInputs": [ - "REPO_MAPPING:rules_android+,bazel_tools bazel_tools" - ], - "generatedRepoSpecs": { - "apksig": { - "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", - "attributes": { - "url": "https://android.googlesource.com/platform/tools/apksig/+archive/24e3075e68ebe17c0b529bb24bfda819db5e2f3b.tar.gz", - "build_file": "@@rules_android+//bzlmod_extensions:apksig.BUILD" - } - } - } - } - }, "@@rules_android+//rules/android_sdk_repository:rule.bzl%android_sdk_repository_extension": { "general": { "bzlTransitiveDigest": "qHbR00gVzVzkxX+PRtv4UGcUFMtBz7TK9CNYUWH8nIE=", diff --git a/phaser/BUILD.bazel b/phaser/BUILD.bazel index 44ed55f..42c0da7 100644 --- a/phaser/BUILD.bazel +++ b/phaser/BUILD.bazel @@ -1,7 +1,11 @@ load("@com_google_protobuf//bazel:cc_proto_library.bzl", "cc_proto_library") load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library") load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") -load("//phaser:copts.bzl", "PHASER_COPTS") +load( + "//phaser:copts.bzl", + "PHASER_COPTS", + "PHASER_PROTOBUF_GENERATED_COPTS", +) package(default_visibility = ["//visibility:public"]) @@ -38,7 +42,7 @@ cc_test( srcs = [ "phaser_test.cc", ], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, data = ["valgrind.supp"], deps = [ ":test_helpers", @@ -66,12 +70,13 @@ cc_test( cc_test( name = "ros_compile_test", srcs = ["ros_compile_test.cc"], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, data = ["valgrind.supp"], deps = [ "//phaser/runtime:phaser_runtime", "//phaser/testdata:ros_compile_cc_proto", "//phaser/testdata:ros_compile_phaser", + "//phaser/testdata:service_only_phaser", "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest_main", ], @@ -192,7 +197,7 @@ cc_test( cc_test( name = "ros_wire_conversion_test", srcs = ["ros_wire_conversion_test.cc"], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, data = ["valgrind.supp"], deps = [ "//phaser/runtime:phaser_runtime", @@ -208,7 +213,7 @@ cc_test( cc_test( name = "all_types_test", srcs = ["all_types_test.cc"], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, data = ["valgrind.supp"], deps = [ ":test_helpers", @@ -246,7 +251,7 @@ cc_test( # Performance benchmark; excluded from wildcard test runs (e.g. //...). # Run explicitly with `bazel test //phaser:perf_test`. tags = ["manual"], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, deps = [ "//phaser/runtime:phaser_runtime", "//phaser/testdata:test_message_phaser", diff --git a/phaser/compiler/BUILD.bazel b/phaser/compiler/BUILD.bazel index 7410082..4938a89 100644 --- a/phaser/compiler/BUILD.bazel +++ b/phaser/compiler/BUILD.bazel @@ -1,5 +1,5 @@ load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") -load("//phaser:copts.bzl", "PHASER_COPTS") +load("//phaser:copts.bzl", "PHASER_PROTOC_COPTS") package(default_visibility = ["//visibility:public"]) @@ -10,7 +10,7 @@ cc_library( "gen.cc", "message_gen.cc", ], - copts = PHASER_COPTS, + copts = PHASER_PROTOC_COPTS, hdrs = [ "enum_gen.h", "gen.h", @@ -33,7 +33,7 @@ cc_binary( srcs = [ "main.cc", ], - copts = PHASER_COPTS, + copts = PHASER_PROTOC_COPTS, deps = [ ":phaser_lib", "@com_google_absl//absl/flags:flag", diff --git a/phaser/compiler/gen.cc b/phaser/compiler/gen.cc index da26f7c..c46021e 100644 --- a/phaser/compiler/gen.cc +++ b/phaser/compiler/gen.cc @@ -152,14 +152,31 @@ bool CodeGenerator::Generate( generate_ros_metadata_ && file->package().rfind("google.protobuf", 0) != 0; - // Custom option schemas and other message-free protos need no C++ output. - // descriptor.proto is imported for extensions but must not be emitted as a - // Phaser message graph (it is huge and not a runtime payload type here). - if (file->message_type_count() == 0 && file->enum_type_count() == 0) { - return true; - } - if (file->name() == std::string("google/protobuf/descriptor.proto") || + // Custom option schemas, service-only files, and descriptor.proto need no + // generated declarations. Still create their declared outputs so build rules + // can safely include them in larger transitive proto graphs. + if ((file->message_type_count() == 0 && file->enum_type_count() == 0) || + file->name() == std::string("google/protobuf/descriptor.proto") || file->name() == std::string("phaser/options.proto")) { + std::string filename = GeneratedFilename( + package_name_, target_name_, std::string(file->name())); + std::filesystem::path hp(filename); + hp.replace_extension(".phaser.h"); + std::filesystem::path cp(filename); + cp.replace_extension(".phaser.cc"); + auto header_output = std::unique_ptr( + generator_context->Open(hp.string())); + auto source_output = std::unique_ptr( + generator_context->Open(cp.string())); + if (header_output == nullptr || source_output == nullptr) { + *error = absl::StrFormat("Failed to create empty outputs for %s", + file->name()); + return false; + } + WriteToZeroCopyStream("// No Phaser declarations in this schema.\n", + header_output.get()); + WriteToZeroCopyStream("// No Phaser definitions in this schema.\n", + source_output.get()); return true; } diff --git a/phaser/compiler/message_gen.cc b/phaser/compiler/message_gen.cc index a2221fc..923bfee 100644 --- a/phaser/compiler/message_gen.cc +++ b/phaser/compiler/message_gen.cc @@ -123,6 +123,24 @@ static bool IsCppReservedWord(const std::string& s) { return reserved_words.contains(s); } +static void GenerateInvalidOffsetofWarningPush(std::ostream& os) { + os << "#if defined(__clang__)\n"; + os << "#pragma clang diagnostic push\n"; + os << "#pragma clang diagnostic ignored \"-Winvalid-offsetof\"\n"; + os << "#elif defined(__GNUC__)\n"; + os << "#pragma GCC diagnostic push\n"; + os << "#pragma GCC diagnostic ignored \"-Winvalid-offsetof\"\n"; + os << "#endif\n"; +} + +static void GenerateInvalidOffsetofWarningPop(std::ostream& os) { + os << "#if defined(__clang__)\n"; + os << "#pragma clang diagnostic pop\n"; + os << "#elif defined(__GNUC__)\n"; + os << "#pragma GCC diagnostic pop\n"; + os << "#endif\n"; +} + static bool IsFixedWireType(const google::protobuf::FieldDescriptor* field) { using Field = google::protobuf::FieldDescriptor; switch (field->type()) { @@ -258,6 +276,39 @@ static std::string RosFieldName( return std::string(field->name()); } +static bool IsRosConstantOnlyMessageField( + const google::protobuf::FieldDescriptor* field) { + if (field->type() != google::protobuf::FieldDescriptor::TYPE_ENUM || + !field->options().HasExtension(phaser::ros_field)) { + return false; + } + const auto& metadata = field->options().GetExtension(phaser::ros_field); + const std::string& definition = metadata.nested_md5_text(); + if (metadata.nested_data_type().empty() || definition.empty()) { + return false; + } + bool saw_constant = false; + size_t line_start = 0; + while (line_start < definition.size()) { + const size_t line_end = definition.find('\n', line_start); + const std::string_view line( + definition.data() + line_start, + (line_end == std::string::npos ? definition.size() : line_end) - + line_start); + if (!line.empty()) { + if (line.find('=') == std::string_view::npos) { + return false; + } + saw_constant = true; + } + if (line_end == std::string::npos) { + break; + } + line_start = line_end + 1; + } + return saw_constant; +} + static std::vector RosConstantDeclarations( const google::protobuf::Descriptor* message) { const auto& message_metadata = @@ -296,6 +347,11 @@ static std::vector RosConstantDeclarations( static std::string RosSourceDefinition( const google::protobuf::Descriptor* message) { + const auto& message_metadata = + message->options().GetExtension(phaser::ros_message); + if (!message_metadata.source_definition().empty()) { + return message_metadata.source_definition(); + } std::string definition; for (const auto& constant : RosConstantDeclarations(message)) { definition += constant + "\n"; @@ -323,7 +379,9 @@ static void AppendRosDependencies(const google::protobuf::Descriptor* message, const google::protobuf::Descriptor* dependency = nullptr; if (!field_metadata.nested_data_type().empty()) { data_type = field_metadata.nested_data_type(); - source_definition = field_metadata.nested_md5_text() + "\n"; + source_definition = field_metadata.nested_source_definition().empty() + ? field_metadata.nested_md5_text() + "\n" + : field_metadata.nested_source_definition(); } else if (field->type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE) { dependency = field->message_type(); @@ -336,6 +394,11 @@ static void AppendRosDependencies(const google::protobuf::Descriptor* message, if (!seen->insert(data_type).second) { continue; } + if (!definition->empty() && definition->back() == '\n' && + (definition->size() == 1 || + (*definition)[definition->size() - 2] != '\n')) { + definition->push_back('\n'); + } *definition += std::string(80, '=') + "\n"; *definition += "MSG: " + data_type + "\n"; *definition += source_definition; @@ -438,10 +501,10 @@ absl::Status MessageGenerator::ValidateArraySizeOption( if (!field->options().HasExtension(phaser::array_size)) { return absl::OkStatus(); } - const int array_size = GetArraySize(field); + const int fixed_array_size = GetArraySize(field); const std::string context = absl::StrFormat("%s.%s", message_->full_name(), field->name()); - if (array_size <= 0) { + if (fixed_array_size <= 0) { return absl::InvalidArgumentError(absl::StrFormat( "phaser.array_size must be positive on field %s", context)); } @@ -477,9 +540,9 @@ absl::Status MessageGenerator::ValidateFieldOptions() const { } if (IsRosFrontend() && IsRosIntrinsic(field) && field->containing_oneof() != nullptr) { - return absl::InvalidArgumentError(absl::StrFormat( - "ROS intrinsic field %s.%s cannot be in a oneof", - message_->full_name(), field->name())); + return absl::InvalidArgumentError( + absl::StrFormat("ROS intrinsic field %s.%s cannot be in a oneof", + message_->full_name(), field->name())); } // time[] and duration[] are repeated through RosRepeatedMessageField, which // converts per element. A repeated Header would need the same treatment, @@ -779,9 +842,9 @@ std::string MessageGenerator::FieldCType( std::string MessageGenerator::FieldRepeatedCType( const google::protobuf::FieldDescriptor* field) { - const int array_size = GetArraySize(field); - if (IsRosFrontend() && array_size > 0) { - return FieldRepeatedArrayCType(field, array_size); + const int fixed_array_size = GetArraySize(field); + if (IsRosFrontend() && fixed_array_size > 0) { + return FieldRepeatedArrayCType(field, fixed_array_size); } return FieldRepeatedVectorCType(field); } @@ -838,8 +901,8 @@ std::string MessageGenerator::FieldRepeatedVectorCType( } std::string MessageGenerator::FieldRepeatedArrayCType( - const google::protobuf::FieldDescriptor* field, int array_size) { - const std::string extent = std::to_string(array_size); + const google::protobuf::FieldDescriptor* field, int fixed_array_size) { + const std::string extent = std::to_string(fixed_array_size); const std::string packed = field->is_packed() ? ", true>" : ", false>"; switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_INT32: @@ -1400,6 +1463,7 @@ void MessageGenerator::GenerateRosOwnerCopyMove(std::ostream& os, bool decl) { return; } + GenerateInvalidOffsetofWarningPush(os); os << name << "::" << name << "(const " << name << "& other) : Message(other)\n"; GenerateFieldInitializers(os, ", "); @@ -1416,6 +1480,8 @@ void MessageGenerator::GenerateRosOwnerCopyMove(std::ostream& os, bool decl) { } )XXX"; + GenerateInvalidOffsetofWarningPop(os); + os << "\n"; os << name << "& " << name << "::operator=(const " << name << "& other) {\n"; os << " if (this != &other) {\n"; @@ -1441,8 +1507,8 @@ void MessageGenerator::GenerateRosOwnerCopyMove(std::ostream& os, bool decl) { os << name << "& " << name << "::operator=(" << name << "&& other) noexcept " << "{\n"; os << " if (this != &other) {\n"; - os << " (void)CloneFrom(other);\n"; - os << " other.Clear();\n"; + os << " this->~" << name << "();\n"; + os << " new (this) " << name << "(std::move(other));\n"; os << " }\n"; os << " return *this;\n"; os << "}\n\n"; @@ -1586,6 +1652,7 @@ void MessageGenerator::GenerateDefaultConstructor(std::ostream& os, bool decl) { "::phaser::Tuning::kPerformance);\n"; return; } + GenerateInvalidOffsetofWarningPush(os); os << MessageName(message_) << "::" << MessageName(message_) << "(size_t initial_size, ::phaser::Tuning tuning)\n"; // Generate field initializers. @@ -1598,6 +1665,8 @@ void MessageGenerator::GenerateDefaultConstructor(std::ostream& os, bool decl) { } )XXX"; + GenerateInvalidOffsetofWarningPop(os); + os << "\n"; } void MessageGenerator::GenerateInternalDefaultConstructor(std::ostream& os, @@ -1606,11 +1675,14 @@ void MessageGenerator::GenerateInternalDefaultConstructor(std::ostream& os, os << " " << MessageName(message_) << "(::phaser::InternalDefault d);\n"; return; } + GenerateInvalidOffsetofWarningPush(os); os << MessageName(message_) << "::" << MessageName(message_) << "(::phaser::InternalDefault)\n"; // Generate field initializers. GenerateFieldInitializers(os); os << "{}\n\n"; + GenerateInvalidOffsetofWarningPop(os); + os << "\n"; } void MessageGenerator::GenerateMainConstructor(std::ostream& os, bool decl) { @@ -1621,6 +1693,7 @@ void MessageGenerator::GenerateMainConstructor(std::ostream& os, bool decl) { "offset);\n"; return; } + GenerateInvalidOffsetofWarningPush(os); os << MessageName(message_) << "::" << MessageName(message_) << "("; os << "std::shared_ptr<::phaser::MessageRuntime> runtime_ptr, " "::toolbelt::BufferOffset " @@ -1628,6 +1701,8 @@ void MessageGenerator::GenerateMainConstructor(std::ostream& os, bool decl) { // Generate field initializers. GenerateFieldInitializers(os, ", "); os << "{}\n\n"; + GenerateInvalidOffsetofWarningPop(os); + os << "\n"; } void MessageGenerator::GenerateFieldInitializers(std::ostream& os, @@ -1635,8 +1710,6 @@ void MessageGenerator::GenerateFieldInitializers(std::ostream& os, if (fields_.empty() && unions_.empty()) { return; } - os << "#pragma clang diagnostic push\n"; - os << "#pragma clang diagnostic ignored \"-Winvalid-offsetof\"\n"; for (auto& field : fields_) { os << sep << field->member_name << "(offsetof(" << MessageName(message_) << ", " << field->member_name << "), " << field->offset << ", " @@ -1649,7 +1722,6 @@ void MessageGenerator::GenerateFieldInitializers(std::ostream& os, << "absl::MakeConstSpan(" << u->member_name << "_field_numbers))\n"; sep = ", "; } - os << "#pragma clang diagnostic pop\n\n"; } void MessageGenerator::GenerateCreators(std::ostream& os, bool decl) { @@ -2353,7 +2425,71 @@ std::string MessageGenerator::ROSFieldValueExpression( std::to_string(union_index) + ", " + field->c_type + ">()"; } -static std::string ROSBulkPrimitiveType( +static std::string ROSPrimitiveCppType( + const google::protobuf::FieldDescriptor* field) { + switch (field->type()) { + case google::protobuf::FieldDescriptor::TYPE_INT32: + case google::protobuf::FieldDescriptor::TYPE_SINT32: + case google::protobuf::FieldDescriptor::TYPE_SFIXED32: + case google::protobuf::FieldDescriptor::TYPE_INT64: + case google::protobuf::FieldDescriptor::TYPE_SINT64: + case google::protobuf::FieldDescriptor::TYPE_SFIXED64: + case google::protobuf::FieldDescriptor::TYPE_UINT32: + case google::protobuf::FieldDescriptor::TYPE_FIXED32: + case google::protobuf::FieldDescriptor::TYPE_UINT64: + case google::protobuf::FieldDescriptor::TYPE_FIXED64: + case google::protobuf::FieldDescriptor::TYPE_DOUBLE: + case google::protobuf::FieldDescriptor::TYPE_FLOAT: + case google::protobuf::FieldDescriptor::TYPE_BOOL: + break; + case google::protobuf::FieldDescriptor::TYPE_ENUM: + case google::protobuf::FieldDescriptor::TYPE_STRING: + case google::protobuf::FieldDescriptor::TYPE_BYTES: + case google::protobuf::FieldDescriptor::TYPE_MESSAGE: + case google::protobuf::FieldDescriptor::TYPE_GROUP: + return ""; + } + std::string ros_type = RosFieldType(field); + if (const size_t array = ros_type.find('['); array != std::string::npos) { + ros_type.resize(array); + } + if (ros_type == "bool") { + return "bool"; + } + if (ros_type == "byte" || ros_type == "int8") { + return "int8_t"; + } + if (ros_type == "char" || ros_type == "uint8") { + return "uint8_t"; + } + if (ros_type == "int16") { + return "int16_t"; + } + if (ros_type == "uint16") { + return "uint16_t"; + } + if (ros_type == "int32") { + return "int32_t"; + } + if (ros_type == "uint32") { + return "uint32_t"; + } + if (ros_type == "int64") { + return "int64_t"; + } + if (ros_type == "uint64") { + return "uint64_t"; + } + if (ros_type == "float32") { + return "float"; + } + if (ros_type == "float64") { + return "double"; + } + return ""; +} + +static std::string ProtobufPrimitiveCppType( const google::protobuf::FieldDescriptor* field) { switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_INT32: @@ -2386,9 +2522,23 @@ static std::string ROSBulkPrimitiveType( return ""; } +static std::string ROSBulkPrimitiveType( + const google::protobuf::FieldDescriptor* field) { + const std::string protobuf_type = ProtobufPrimitiveCppType(field); + return ROSPrimitiveCppType(field) == protobuf_type ? protobuf_type : ""; +} + void MessageGenerator::GenerateROSFieldSize( std::ostream& os, const google::protobuf::FieldDescriptor* field, const std::string& value_expression, const std::string& indent) { + if (IsRosConstantOnlyMessageField(field)) { + return; + } + if (const std::string ros_type = ROSPrimitiveCppType(field); + !ros_type.empty()) { + os << indent << "_phaser_serialized_size += sizeof(" << ros_type << ");\n"; + return; + } switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_INT32: case google::protobuf::FieldDescriptor::TYPE_SINT32: @@ -2441,10 +2591,18 @@ void MessageGenerator::GenerateROSFieldSize( void MessageGenerator::GenerateROSFieldWrite( std::ostream& os, const google::protobuf::FieldDescriptor* field, const std::string& value_expression, const std::string& indent) { + if (IsRosConstantOnlyMessageField(field)) { + return; + } auto write = [&](const std::string& expression) { os << indent << "if (absl::Status _phaser_status = _phaser_buffer.Write(" << expression << "); !_phaser_status.ok()) return _phaser_status;\n"; }; + if (const std::string ros_type = ROSPrimitiveCppType(field); + !ros_type.empty()) { + write("static_cast<" + ros_type + ">(" + value_expression + ")"); + return; + } switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_INT32: case google::protobuf::FieldDescriptor::TYPE_SINT32: @@ -2571,6 +2729,16 @@ void MessageGenerator::GenerateROSFieldRead( os << indent << "}\n"; }; + if (IsRosConstantOnlyMessageField(field)) { + set_value("static_cast<" + EnumName(field->enum_type()) + ">(0)"); + return; + } + if (const std::string ros_type = ROSPrimitiveCppType(field); + !ros_type.empty()) { + read_value(ros_type); + return; + } + switch (field->type()) { case google::protobuf::FieldDescriptor::TYPE_INT32: case google::protobuf::FieldDescriptor::TYPE_SINT32: @@ -2939,27 +3107,33 @@ void MessageGenerator::GenerateDirectProtobufField( os << indent << " ::phaser::ProtoBuffer ros_values(*ros_packed);\n"; os << indent << " while (!ros_values.Eof()) {\n"; - os << indent << " " << type << " ros_ignored{};\n"; + os << indent << " [[maybe_unused]] " << type + << " _phaser_ignored{};\n"; os << indent << " {\n"; - GenerateDirectProtobufReadValue(os, field, "ros_values", "ros_ignored", + GenerateDirectProtobufReadValue(os, field, "ros_values", + "_phaser_ignored", indent + " "); os << indent << " }\n"; os << indent << " ++ros_count;\n"; os << indent << " }\n"; } os << indent << " } else {\n"; - os << indent << " " << type << " ros_ignored{};\n"; + os << indent << " [[maybe_unused]] " << type + << " _phaser_ignored{};\n"; os << indent << " {\n"; GenerateDirectProtobufReadValue(os, field, "ros_count_scan", - "ros_ignored", indent + " "); + "_phaser_ignored", + indent + " "); os << indent << " }\n"; os << indent << " ++ros_count;\n"; os << indent << " }\n"; } else { - os << indent << " " << type << " ros_ignored{};\n"; + os << indent << " [[maybe_unused]] " << type + << " _phaser_ignored{};\n"; os << indent << " {\n"; GenerateDirectProtobufReadValue(os, field, "ros_count_scan", - "ros_ignored", indent + " "); + "_phaser_ignored", + indent + " "); os << indent << " }\n"; os << indent << " ++ros_count;\n"; } @@ -3411,21 +3585,23 @@ void MessageGenerator::GenerateROSSerialization(std::ostream& os, bool decl) { os << " absl::Status DeserializeFromROS(" "::phaser::ROSReader& _phaser_buffer);\n"; os << " absl::Status ParseFromROS(absl::Span input);\n"; - os << R"XXX( absl::Status SerializeToROSArray(void* data, size_t size) const { - ::phaser::ROSBuffer buffer(data, size); - return SerializeToROS(buffer); + os << R"XXX( absl::Status SerializeToROSArray( + void* _phaser_data, size_t _phaser_size) const { + ::phaser::ROSBuffer _phaser_buffer(_phaser_data, _phaser_size); + return SerializeToROS(_phaser_buffer); } - absl::Status SerializeToROSString(std::string* output) const { - if (output == nullptr) { + absl::Status SerializeToROSString(std::string* _phaser_output) const { + if (_phaser_output == nullptr) { return absl::InvalidArgumentError("ROS output string is null"); } - output->resize(ROSSerializedSize()); - ::phaser::ROSBuffer buffer(output->data(), output->size()); - absl::Status status = SerializeToROS(buffer); - if (!status.ok()) { - output->clear(); + _phaser_output->resize(ROSSerializedSize()); + ::phaser::ROSBuffer _phaser_buffer( + _phaser_output->data(), _phaser_output->size()); + absl::Status _phaser_status = SerializeToROS(_phaser_buffer); + if (!_phaser_status.ok()) { + _phaser_output->clear(); } - return status; + return _phaser_status; } )XXX"; os << " static absl::Status ProtobufToROS(" @@ -3925,33 +4101,35 @@ void MessageGenerator::GenerateProtobufSerialization(std::ostream& os) { return static_cast(ByteSizeLong()); } - bool SerializeToArray(char* array, size_t size) const { - ::phaser::ProtoBuffer buffer(array, size); - if (absl::Status status = Serialize(buffer); !status.ok()) return false; + bool SerializeToArray(char* _phaser_array, size_t _phaser_size) const { + ::phaser::ProtoBuffer _phaser_buffer(_phaser_array, _phaser_size); + if (absl::Status _phaser_status = Serialize(_phaser_buffer); + !_phaser_status.ok()) return false; return true; } - bool ParseFromArray(const char* array, size_t size) { - ::phaser::ProtoBuffer buffer(array, size); - if (absl::Status status = Deserialize(buffer); !status.ok()) return false; + bool ParseFromArray(const char* _phaser_array, size_t _phaser_size) { + ::phaser::ProtoBuffer _phaser_buffer(_phaser_array, _phaser_size); + if (absl::Status _phaser_status = Deserialize(_phaser_buffer); + !_phaser_status.ok()) return false; return true; } // String serialization. - bool SerializeToString(std::string* str) const { - size_t size = SerializedSize(); - str->resize(size); - return SerializeToArray(&(*str)[0], size); + bool SerializeToString(std::string* _phaser_string) const { + size_t _phaser_size = SerializedSize(); + _phaser_string->resize(_phaser_size); + return SerializeToArray(&(*_phaser_string)[0], _phaser_size); } std::string SerializeAsString() const { - std::string str; - SerializeToString(&str); - return str; + std::string _phaser_string; + SerializeToString(&_phaser_string); + return _phaser_string; } - bool ParseFromString(const std::string& str) { - return ParseFromArray(str.data(), str.size()); + bool ParseFromString(const std::string& _phaser_string) { + return ParseFromArray(_phaser_string.data(), _phaser_string.size()); } )XXX"; } @@ -4067,7 +4245,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { if (field->field->is_repeated()) { os << " " << field->member_name << ".Clear();\n"; if (UsesArrayFacade(field->field)) { - const int array_size = GetArraySize(field->field); + const int fixed_array_size = GetArraySize(field->field); // A ROS intrinsic element is a value, not a bound message, so it // clones through Set/Get like a string rather than through CloneFrom. if (field->field->type() == @@ -4075,7 +4253,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { !IsRosIntrinsic(field->field)) { os << " for (size_t _phaser_index = 0; " "_phaser_index < static_cast(" - << array_size << "); ++_phaser_index) {\n"; + << fixed_array_size << "); ++_phaser_index) {\n"; os << " auto _phaser_source = _phaser_other." << field->member_name << ".Get(_phaser_index);\n"; os << " if (_phaser_source.IsBound()) {\n"; @@ -4092,7 +4270,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { google::protobuf::FieldDescriptor::TYPE_BYTES) { os << " for (size_t _phaser_index = 0; " "_phaser_index < static_cast(" - << array_size << "); ++_phaser_index) {\n"; + << fixed_array_size << "); ++_phaser_index) {\n"; os << " " << field->member_name << ".Set(_phaser_index, _phaser_other." << field->member_name << ".Get(_phaser_index));\n"; @@ -4100,7 +4278,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { } else { os << " for (size_t _phaser_index = 0; " "_phaser_index < static_cast(" - << array_size << "); ++_phaser_index) {\n"; + << fixed_array_size << "); ++_phaser_index) {\n"; os << " " << field->member_name << ".Set(_phaser_index, _phaser_other." << field->member_name << ".Get(_phaser_index));\n"; @@ -4221,8 +4399,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { << accessor << "()); !_phaser_status.ok()) return _phaser_status;\n"; } else { - os << " set_" << name << "(_phaser_other." << accessor - << "());\n"; + os << " set_" << name << "(_phaser_other." << accessor << "());\n"; } os << " }\n"; } @@ -4554,8 +4731,13 @@ void MessageGenerator::GenerateMessageInfo(std::ostream& os, bool decl) { os << " return &info;\n"; os << " }\n"; + os << "#if defined(__clang__)\n"; os << "#pragma clang diagnostic push\n"; os << "#pragma clang diagnostic ignored \"-Winvalid-offsetof\"\n"; + os << "#elif defined(__GNUC__)\n"; + os << "#pragma GCC diagnostic push\n"; + os << "#pragma GCC diagnostic ignored \"-Winvalid-offsetof\"\n"; + os << "#endif\n"; // Generate fields_in_order. int index = 0; @@ -4605,7 +4787,11 @@ void MessageGenerator::GenerateMessageInfo(std::ostream& os, bool decl) { os << " return &info;\n"; os << "}\n\n"; + os << "#if defined(__clang__)\n"; os << "#pragma clang diagnostic pop\n"; + os << "#elif defined(__GNUC__)\n"; + os << "#pragma GCC diagnostic pop\n"; + os << "#endif\n"; } } // namespace phaser diff --git a/phaser/compiler/message_gen.h b/phaser/compiler/message_gen.h index 5495fdd..7af6dff 100644 --- a/phaser/compiler/message_gen.h +++ b/phaser/compiler/message_gen.h @@ -188,7 +188,7 @@ class MessageGenerator { std::string FieldRepeatedVectorCType( const google::protobuf::FieldDescriptor* field); std::string FieldRepeatedArrayCType( - const google::protobuf::FieldDescriptor* field, int array_size); + const google::protobuf::FieldDescriptor* field, int fixed_array_size); std::string FieldUnionCType(const google::protobuf::FieldDescriptor* field); uint32_t FieldBinarySize(const google::protobuf::FieldDescriptor* field); std::string FieldInfoType(const google::protobuf::FieldDescriptor* field); diff --git a/phaser/copts.bzl b/phaser/copts.bzl index 307d26f..dadaaa7 100644 --- a/phaser/copts.bzl +++ b/phaser/copts.bzl @@ -76,7 +76,6 @@ _PHASER_GCC_COPTS = [ "-Winit-self", "-Wlogical-op", "-Wmissing-declarations", - "-Wmissing-include-dirs", "-Wnoexcept", "-Wnon-virtual-dtor", "-Wnull-dereference", @@ -101,9 +100,44 @@ _PHASER_GCC_COPTS = [ # shared headers, which GCC reports as unknown pragmas. "-Wno-invalid-offsetof", "-Wno-unknown-pragmas", + + # Bazel emits speculative external and generated include paths that need + # not exist for every compile action. -Wmissing-include-dirs reports those + # toolchain-managed paths rather than problems in Phaser source. + "-Wno-missing-include-dirs", ] PHASER_COPTS = select({ "//phaser:is_clang": _PHASER_CLANG_COPTS, "//conditions:default": _PHASER_GCC_COPTS, }) + +# protoc and Abseil expose template-heavy public headers that trigger several +# aggressive GCC diagnostics at instantiation sites in the Phaser compiler. +# Keep the full warning set elsewhere, and scope these upstream-header +# exemptions to targets that directly consume the protoc compiler API. +PHASER_PROTOC_COPTS = PHASER_COPTS + select({ + "//phaser:is_clang": [], + "//conditions:default": [ + "-Wno-conversion", + "-Wno-ctor-dtor-privacy", + "-Wno-deprecated-declarations", + "-Wno-duplicated-branches", + "-Wno-extra-semi", + "-Wno-redundant-decls", + "-Wno-sign-conversion", + "-Wno-sign-promo", + "-Wno-useless-cast", + "-Wno-zero-as-null-pointer-constant", + ], +}) + +# Protobuf-generated headers are compiled as part of consumer translation +# units, so GCC attributes their implementation details to the consumer. +PHASER_PROTOBUF_GENERATED_COPTS = select({ + "//phaser:is_clang": [], + "//conditions:default": [ + "-Wno-conversion", + "-Wno-useless-cast", + ], +}) diff --git a/phaser/options.proto b/phaser/options.proto index fa4c2e9..8efb76e 100644 --- a/phaser/options.proto +++ b/phaser/options.proto @@ -11,6 +11,11 @@ message RosMessageMetadata { // Canonical constant declarations used by the ROS MD5 algorithm. repeated string constants = 2; + + // Canonical local ROS source text when comments, blank lines, or declaration + // ordering cannot be reconstructed from the protobuf descriptor alone. + // Definitions for nested message types are appended automatically. + string source_definition = 3; } message RosFieldMetadata { @@ -28,6 +33,11 @@ message RosFieldMetadata { // Datatype for the non-protobuf nested ROS message. string nested_data_type = 4; + + // Full ROS source definition for a nested type that is not represented as a + // protobuf message. Omit when it is identical to nested_md5_text plus a + // trailing newline. + string nested_source_definition = 5; } message RosEnumMetadata { diff --git a/phaser/phaser_library.bzl b/phaser/phaser_library.bzl index 464a357..2043035 100644 --- a/phaser/phaser_library.bzl +++ b/phaser/phaser_library.bzl @@ -45,9 +45,6 @@ def _phaser_action( import_paths = {} for s in transitive_sources: for f in s.to_list(): - if f.short_path.startswith("../"): - repository_name = f.short_path.split("/", 2)[1] - import_paths["-Iexternal/" + repository_name] = None if not f.is_source: index = f.path.find("_virtual_imports") if index != -1: diff --git a/phaser/ros_compile_test.cc b/phaser/ros_compile_test.cc index 00d5e37..1252497 100644 --- a/phaser/ros_compile_test.cc +++ b/phaser/ros_compile_test.cc @@ -13,6 +13,32 @@ namespace foo::bar::phaser { namespace { +TEST(RosCompileTest, PreservesExplicitRosSourceDefinition) { + EXPECT_EQ(RosDefinitionMetadata::RosDefinition(), + "# Original comment\n\nint32 value\n"); +} + +TEST(RosCompileTest, ConstantOnlyRosMessageFieldHasNoWirePayload) { + RosConstantOnlyHolder message; + message.marker = RosConstantOnly::ROS_CONSTANT_ONLY_UNSPECIFIED; + message.value = 42; + EXPECT_EQ(message.marker.Get(), + RosConstantOnly::ROS_CONSTANT_ONLY_UNSPECIFIED); + EXPECT_EQ(message.ROSSerializedSize(), sizeof(uint32_t)); + + std::string wire; + ASSERT_TRUE(message.SerializeToROSString(&wire).ok()); + ASSERT_EQ(wire.size(), sizeof(uint32_t)); + + RosConstantOnlyHolder decoded; + ASSERT_TRUE( + decoded.ParseFromROS(absl::Span(wire.data(), wire.size())) + .ok()); + EXPECT_EQ(decoded.marker.Get(), + RosConstantOnly::ROS_CONSTANT_ONLY_UNSPECIFIED); + EXPECT_EQ(decoded.value.Get(), 42u); +} + TEST(RosCompileTest, ScalarConversionAndAssignment) { RosCompileMessage msg; msg.x = 42; @@ -90,6 +116,27 @@ TEST(RosCompileTest, StringConversionAndAssignment) { msg.name = "hello"; EXPECT_TRUE(msg.name.IsPresent()); EXPECT_EQ(std::string_view(msg.name), "hello"); + EXPECT_EQ(msg.name, "hello"); + EXPECT_EQ("hello", msg.name); + EXPECT_TRUE(msg.name.starts_with("hell")); + EXPECT_TRUE(msg.name.ends_with('o')); + EXPECT_TRUE(msg.name.contains("ell")); + EXPECT_EQ(msg.name.find('e'), 1u); + EXPECT_EQ(msg.name.rfind("hell", 0), 0u); + EXPECT_EQ(msg.name.rfind("world", 0), std::string_view::npos); + EXPECT_EQ(msg.name.substr(1, 3), "ell"); + + msg.name.append(" world"); + msg.name += '!'; + EXPECT_EQ(msg.name, "hello world!"); + msg.name.erase(5, 6); + msg.name.insert(5, ","); + msg.name.replace(6, 1, "?"); + EXPECT_EQ(msg.name, "hello,?"); + msg.name.pop_back(); + msg.name.resize(8, '.'); + EXPECT_EQ(msg.name, "hello,.."); + EXPECT_EQ(std::string(msg.name.rbegin(), msg.name.rend()), "..,olleh"); msg.name = std::string("world"); EXPECT_EQ(msg.name.Get(), "world"); @@ -128,8 +175,7 @@ TEST(RosCompileTest, IndirectMessageAccess) { TEST(RosCompileTest, PrimitiveVectorSyntax) { RosCompileMessage msg; - msg.xs.push_back(1); - msg.xs.push_back(2); + msg.xs = {1, 2}; msg.xs.reserve(8); EXPECT_EQ(msg.xs.size(), 2u); EXPECT_EQ(msg.xs[0], 1); @@ -152,24 +198,44 @@ TEST(RosCompileTest, PrimitiveVectorSyntax) { EXPECT_EQ(seen[0], 10); EXPECT_EQ(seen[3], 40); + EXPECT_EQ(msg.xs.at(0), 10); + msg.xs.emplace_back(50); + EXPECT_EQ(msg.xs.back(), 50); + msg.xs.pop_back(); + auto inserted = msg.xs.insert(msg.xs.begin() + 1, 30); + EXPECT_EQ(*inserted, 30); + msg.xs.erase(inserted); + msg.xs.resize(6, 60); + EXPECT_EQ(msg.xs, std::vector({10, 2, 0, 40, 60, 60})); + EXPECT_EQ(*msg.xs.rbegin(), 60); + msg.xs.assign(2, 7); + EXPECT_EQ(std::vector({7, 7}), msg.xs); + msg.xs.clear(); EXPECT_TRUE(msg.xs.empty()); } TEST(RosCompileTest, EnumVectorSyntax) { RosCompileMessage msg; - msg.colors.push_back(RosColor::ROS_COLOR_RED); - msg.colors.push_back(RosColor::ROS_COLOR_BLUE); + msg.colors = {RosColor::ROS_COLOR_RED, RosColor::ROS_COLOR_BLUE}; EXPECT_EQ(msg.colors.size(), 2u); EXPECT_EQ(msg.colors[0], RosColor::ROS_COLOR_RED); msg.colors[1] = RosColor::ROS_COLOR_UNSPECIFIED; EXPECT_EQ(msg.colors[1], RosColor::ROS_COLOR_UNSPECIFIED); + msg.colors.emplace_back(RosColor::ROS_COLOR_BLUE); + msg.colors.pop_back(); + auto inserted = + msg.colors.insert(msg.colors.begin() + 1, RosColor::ROS_COLOR_BLUE); + EXPECT_EQ(*inserted, RosColor::ROS_COLOR_BLUE); + msg.colors.erase(inserted); + EXPECT_EQ(msg.colors, + std::vector( + {RosColor::ROS_COLOR_RED, RosColor::ROS_COLOR_UNSPECIFIED})); } TEST(RosCompileTest, StringVectorSyntax) { RosCompileMessage msg; - msg.names.push_back("a"); - msg.names.push_back("b"); + msg.names = {"a", "b"}; EXPECT_EQ(msg.names.size(), 2u); EXPECT_EQ(std::string_view(msg.names[0]), "a"); @@ -178,18 +244,38 @@ TEST(RosCompileTest, StringVectorSyntax) { msg.names[0] = msg.names[1]; EXPECT_EQ(msg.names[0].Get(), "beta"); + EXPECT_TRUE(msg.names[0].starts_with("be")); + msg.names[0].append("!"); + EXPECT_EQ(msg.names[0], "beta!"); + + msg.names.emplace_back("gamma"); + msg.names.emplace_back() = "delta"; + msg.names.pop_back(); + msg.names.resize(4, "fill"); + EXPECT_EQ(msg.names, + std::vector({"beta!", "beta", "gamma", "fill"})); + msg.names.resize(5); + msg.names[4] = "late"; + EXPECT_EQ(*msg.names.rbegin(), "late"); + auto insert_pos = msg.names.begin(); + ++insert_pos; + auto inserted = msg.names.insert(insert_pos, "inserted"); + EXPECT_EQ(*inserted, "inserted"); + msg.names.erase(inserted); size_t count = 0; for (std::string_view s : msg.names) { EXPECT_FALSE(s.empty()); ++count; } - EXPECT_EQ(count, 2u); + EXPECT_EQ(count, msg.names.size()); + msg.names.clear(); + EXPECT_TRUE(msg.names.empty()); } TEST(RosCompileTest, MessageVectorSyntax) { RosCompileMessage msg; - auto a = msg.inners.Add(); + auto a = msg.inners.emplace_back(); a->id = 1; auto b = msg.inners.Add(); b->id = 2; @@ -200,12 +286,38 @@ TEST(RosCompileTest, MessageVectorSyntax) { msg.inners[0]->id = 11; EXPECT_EQ(msg.inners.front()->id.Get(), 11); + msg.inners.resize(4); + msg.inners[2]->id = 12; + msg.inners.at(3)->id = 13; + msg.inners.pop_back(); + EXPECT_EQ(msg.inners.size(), 3u); + + RosInner source; + source.id = 14; + msg.inners.push_back(source); + source.id = 15; + EXPECT_EQ(msg.inners.back()->id.Get(), 14); + EXPECT_EQ((*msg.inners.rbegin())->id.Get(), 14); + auto insert_pos = msg.inners.begin(); + ++insert_pos; + auto inserted = msg.inners.insert(insert_pos, source); + EXPECT_EQ((*inserted)->id.Get(), 15); + msg.inners.erase(inserted); + size_t count = 0; for (auto elem : msg.inners) { EXPECT_TRUE(elem->id.IsPresent()); ++count; } - EXPECT_EQ(count, 2u); + EXPECT_EQ(count, msg.inners.size()); + msg.inners.clear(); + EXPECT_TRUE(msg.inners.empty()); + + std::vector allocated = msg.inners.Allocate(2); + allocated[0].id = 21; + allocated[1].id = 22; + EXPECT_EQ(msg.inners[0]->id.Get(), 21); + EXPECT_EQ(msg.inners[1]->id.Get(), 22); } TEST(RosCompileTest, ProxyAssignmentDoesNotRebind) { @@ -274,11 +386,12 @@ TEST(RosCompileTest, MessageCopyAssignUsesCloneFrom) { EXPECT_EQ(dst.x.Get(), 11); } -TEST(RosCompileTest, MessageMoveAssignUsesCloneFrom) { +TEST(RosCompileTest, MessageMoveAssignTransfersStorage) { RosCompileMessage src; src.x = 21; src.name = "moved"; src.xs.push_back(9); + const auto* source_runtime = src.runtime.get(); RosCompileMessage dst; dst = std::move(src); @@ -287,9 +400,20 @@ TEST(RosCompileTest, MessageMoveAssignUsesCloneFrom) { EXPECT_EQ(dst.name.Get(), "moved"); ASSERT_EQ(dst.xs.size(), 1u); EXPECT_EQ(dst.xs[0], 9); - EXPECT_FALSE(src.x.IsPresent()); - EXPECT_FALSE(src.name.IsPresent()); - EXPECT_TRUE(src.xs.empty()); + EXPECT_EQ(dst.runtime.get(), source_runtime); + EXPECT_FALSE(src.IsBound()); +} + +TEST(RosCompileTest, MessageMoveAssignRebindsMovedFromDestination) { + RosCompileMessage accumulator; + accumulator.x = 21; + RosCompileMessage published(std::move(accumulator)); + + accumulator = RosCompileMessage::CreateDynamicMutable(8192); + accumulator.x = 22; + + EXPECT_EQ(published.x.Get(), 21); + EXPECT_EQ(accumulator.x.Get(), 22); } TEST(RosCompileTest, MessageCopyCtorDeepCopies) { diff --git a/phaser/ros_metadata_protobuf_frontend_test.cc b/phaser/ros_metadata_protobuf_frontend_test.cc index 265d40b..9415172 100644 --- a/phaser/ros_metadata_protobuf_frontend_test.cc +++ b/phaser/ros_metadata_protobuf_frontend_test.cc @@ -24,22 +24,28 @@ TEST(RosMetadataProtobufFrontendTest, MatchesRosMetadata) { EXPECT_EQ(Wrapper::RosDefinition(), "uint8 READY=1\nexample_msgs/Bool child\nint32[3] samples\n" "example_msgs/Status status\nbool ready\ntime stamp\n" - "time[] stamps\n" + "time[] stamps\n\n" "==================================================================" "==============\n" "MSG: example_msgs/Bool\n" - "bool data\n" + "bool data\n\n" "==================================================================" "==============\n" "MSG: example_msgs/Status\n" "int32 OK=0\nint32 BAD=1\nint32 value\n"); - EXPECT_EQ( - Wrapper::RosMd5(), - ::phaser::Md5("uint8 READY=1\n" - "8b94c1b53db61fb6aed406028ad6332a child\n" - "int32[3] samples\n" + - Status::RosMd5() + - " status\nbool ready\ntime stamp\ntime[] stamps")); + EXPECT_EQ(Wrapper::RosMd5(), + ::phaser::Md5("uint8 READY=1\n" + "8b94c1b53db61fb6aed406028ad6332a child\n" + "int32[3] samples\n" + + Status::RosMd5() + + " status\nbool ready\ntime stamp\ntime[] stamps")); + + EXPECT_EQ(LegacyStatusWrapper::RosDefinition(), + "example_msgs/LegacyStatus status\n\n" + "==================================================================" + "==============\n" + "MSG: example_msgs/LegacyStatus\n" + "# Legacy status.\nuint8 OK = 0\n"); } } // namespace diff --git a/phaser/ros_metadata_ros_frontend_test.cc b/phaser/ros_metadata_ros_frontend_test.cc index 0e28e85..5fd4609 100644 --- a/phaser/ros_metadata_ros_frontend_test.cc +++ b/phaser/ros_metadata_ros_frontend_test.cc @@ -21,13 +21,19 @@ TEST(RosMetadataRosFrontendTest, MatchesRosMetadata) { ::phaser::Md5("int32 OK=0\nint32 BAD=1\nint32 value")); EXPECT_EQ(Wrapper::RosDataType(), "example_msgs/Wrapper"); - EXPECT_EQ( - Wrapper::RosMd5(), - ::phaser::Md5("uint8 READY=1\n" - "8b94c1b53db61fb6aed406028ad6332a child\n" - "int32[3] samples\n" + - Status::RosMd5() + - " status\nbool ready\ntime stamp\ntime[] stamps")); + EXPECT_EQ(Wrapper::RosMd5(), + ::phaser::Md5("uint8 READY=1\n" + "8b94c1b53db61fb6aed406028ad6332a child\n" + "int32[3] samples\n" + + Status::RosMd5() + + " status\nbool ready\ntime stamp\ntime[] stamps")); + + EXPECT_EQ(LegacyStatusWrapper::RosDefinition(), + "example_msgs/LegacyStatus status\n\n" + "==================================================================" + "==============\n" + "MSG: example_msgs/LegacyStatus\n" + "# Legacy status.\nuint8 OK = 0\n"); } } // namespace diff --git a/phaser/ros_wire_conversion_test.cc b/phaser/ros_wire_conversion_test.cc index 4ecfb95..b0e8f92 100644 --- a/phaser/ros_wire_conversion_test.cc +++ b/phaser/ros_wire_conversion_test.cc @@ -23,6 +23,7 @@ namespace { using RosCompileMessage = ::foo::bar::phaser::RosCompileMessage; using RosInner = ::foo::bar::phaser::RosInner; using RosIntrinsicMessage = ::foo::bar::phaser::RosIntrinsicMessage; +using RosNarrowScalars = ::foo::bar::phaser::RosNarrowScalars; using RosPackedFixedMessage = ::foo::bar::phaser::RosPackedFixedMessage; using ProtobufFrontendIntrinsicMessage = ::foo::bar::pb::protobuf_phaser::RosIntrinsicMessage; @@ -125,16 +126,16 @@ void PopulateProtobufMessage(::foo::bar::RosCompileMessage& message) { std::string ExpectedRosCompileBytes(bool include_oneof = true) { std::string bytes; - AppendIntegral(bytes, static_cast(-7)); + AppendIntegral(bytes, int32_t{-7}); AppendString(bytes, "robot"); AppendIntegral(bytes, static_cast(1)); AppendDouble(bytes, 1.5); AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_RED)); - AppendIntegral(bytes, static_cast(42)); + AppendIntegral(bytes, int32_t{42}); AppendIntegral(bytes, static_cast(2)); - AppendIntegral(bytes, static_cast(10)); - AppendIntegral(bytes, static_cast(-20)); + AppendIntegral(bytes, int32_t{10}); + AppendIntegral(bytes, int32_t{-20}); AppendIntegral(bytes, static_cast(2)); AppendString(bytes, "a"); AppendString(bytes, "beta"); @@ -142,20 +143,19 @@ std::string ExpectedRosCompileBytes(bool include_oneof = true) { AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_RED)); AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_BLUE)); AppendIntegral(bytes, static_cast(2)); - AppendIntegral(bytes, static_cast(100)); - AppendIntegral(bytes, static_cast(200)); + AppendIntegral(bytes, int32_t{100}); + AppendIntegral(bytes, int32_t{200}); for (int32_t value : {1, 2, 3, 4}) { AppendIntegral(bytes, value); } AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_RED)); AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_BLUE)); - AppendIntegral(bytes, - static_cast(RosColor::ROS_COLOR_UNSPECIFIED)); + AppendIntegral(bytes, static_cast(RosColor::ROS_COLOR_UNSPECIFIED)); AppendString(bytes, "left"); AppendString(bytes, "right"); - AppendIntegral(bytes, static_cast(300)); - AppendIntegral(bytes, static_cast(400)); + AppendIntegral(bytes, int32_t{300}); + AppendIntegral(bytes, int32_t{400}); if (include_oneof) { AppendIntegral(bytes, static_cast(17)); @@ -170,22 +170,22 @@ std::string ExpectedIntrinsicBytes() { std::string bytes; AppendIntegral(bytes, static_cast(12)); AppendIntegral(bytes, static_cast(345)); - AppendIntegral(bytes, static_cast(-4)); - AppendIntegral(bytes, static_cast(500)); + AppendIntegral(bytes, int32_t{-4}); + AppendIntegral(bytes, int32_t{500}); AppendIntegral(bytes, static_cast(9)); AppendIntegral(bytes, static_cast(21)); AppendIntegral(bytes, static_cast(654)); AppendString(bytes, "map"); - AppendIntegral(bytes, static_cast(0)); // count - AppendString(bytes, ""); // name + AppendIntegral(bytes, int32_t{0}); // count + AppendString(bytes, ""); // name AppendIntegral(bytes, static_cast(0)); // samples AppendIntegral(bytes, static_cast(0)); // tags AppendIntegral(bytes, static_cast(0)); // children AppendString(bytes, ""); // fixed_names[0] AppendString(bytes, ""); // fixed_names[1] for (int i = 0; i < 2; ++i) { - AppendIntegral(bytes, static_cast(0)); // child id + AppendIntegral(bytes, int32_t{0}); // child id AppendString(bytes, ""); // child label } AppendIntegral(bytes, static_cast(0)); // choice unset @@ -198,16 +198,16 @@ std::string ExpectedIntrinsicBytes() { AppendIntegral(bytes, static_cast(3)); AppendIntegral(bytes, static_cast(4)); AppendIntegral(bytes, static_cast(1)); // timeouts count - AppendIntegral(bytes, static_cast(-5)); - AppendIntegral(bytes, static_cast(6)); + AppendIntegral(bytes, int32_t{-5}); + AppendIntegral(bytes, int32_t{6}); AppendIntegral(bytes, static_cast(7)); // fixed_stamps[0] AppendIntegral(bytes, static_cast(8)); AppendIntegral(bytes, static_cast(9)); // fixed_stamps[1] AppendIntegral(bytes, static_cast(10)); - AppendIntegral(bytes, static_cast(-11)); // fixed_timeouts[0] - AppendIntegral(bytes, static_cast(12)); - AppendIntegral(bytes, static_cast(13)); // fixed_timeouts[1] - AppendIntegral(bytes, static_cast(14)); + AppendIntegral(bytes, int32_t{-11}); // fixed_timeouts[0] + AppendIntegral(bytes, int32_t{12}); + AppendIntegral(bytes, int32_t{13}); // fixed_timeouts[1] + AppendIntegral(bytes, int32_t{14}); return bytes; } @@ -231,12 +231,12 @@ TEST(ROSWireConversionTest, LiveProtobufAndNativePathsMatchKnownBytes) { EXPECT_EQ(::phaser::InferMessageWireFormat(protobuf_wire), ::phaser::MessageWireFormat::kProtobuf); ::phaser::ROSBuffer protobuf_output; - ASSERT_TRUE(RosCompileMessage::ProtobufToROS(protobuf_wire, protobuf_output) - .ok()); + ASSERT_TRUE( + RosCompileMessage::ProtobufToROS(protobuf_wire, protobuf_output).ok()); EXPECT_EQ(protobuf_output.AsString(), expected); std::vector exact_output(expected.size()); ::phaser::ROSBuffer fixed_protobuf_output(exact_output.data(), - exact_output.size()); + exact_output.size()); ASSERT_TRUE( RosCompileMessage::ProtobufToROS(protobuf_wire, fixed_protobuf_output) .ok()); @@ -246,21 +246,18 @@ TEST(ROSWireConversionTest, LiveProtobufAndNativePathsMatchKnownBytes) { ::phaser::ROSBuffer native_output; const auto* native_data = reinterpret_cast(phaser_message.Data()); - const absl::Span native_bytes(native_data, - phaser_message.Size()); + const absl::Span native_bytes(native_data, phaser_message.Size()); EXPECT_EQ(::phaser::InferMessageWireFormat(native_bytes), ::phaser::MessageWireFormat::kPhaser); - ASSERT_TRUE(RosCompileMessage::PhaserToROS( - native_bytes, native_output) - .ok()); + ASSERT_TRUE(RosCompileMessage::PhaserToROS(native_bytes, native_output).ok()); EXPECT_EQ(native_output.AsString(), expected); ::phaser::ROSBuffer inferred_protobuf_output; - ASSERT_TRUE(RosCompileMessage::ConvertToROS( - absl::Span(protobuf_wire.data(), - protobuf_wire.size()), - inferred_protobuf_output) - .ok()); + ASSERT_TRUE( + RosCompileMessage::ConvertToROS( + absl::Span(protobuf_wire.data(), protobuf_wire.size()), + inferred_protobuf_output) + .ok()); EXPECT_EQ(inferred_protobuf_output.AsString(), expected); ::phaser::ROSBuffer inferred_native_output; @@ -308,8 +305,8 @@ TEST(ROSWireConversionTest, DirectPackedFixedFieldsUseCompatibleRawLayout) { AppendIntegral(expected, uint64_t{0x1122334455667788}); ::phaser::ROSBuffer ros_output; - ASSERT_TRUE(RosPackedFixedMessage::ProtobufToROS( - protobuf.SerializeAsString(), ros_output) + ASSERT_TRUE(RosPackedFixedMessage::ProtobufToROS(protobuf.SerializeAsString(), + ros_output) .ok()); EXPECT_EQ(ros_output.AsString(), expected); @@ -322,7 +319,7 @@ TEST(ROSWireConversionTest, DirectPackedFixedFieldsUseCompatibleRawLayout) { .ok()); ::foo::bar::RosPackedFixedMessage reparsed; ASSERT_TRUE(reparsed.ParseFromArray(protobuf_storage.data(), - protobuf_output.Size())); + int(protobuf_output.Size()))); EXPECT_EQ(reparsed.SerializeAsString(), protobuf.SerializeAsString()); } @@ -333,8 +330,7 @@ TEST(ROSWireConversionTest, DirectPackedFixedFieldRejectsPartialElement) { malformed.append("\x01\x02\x03", 3); ::phaser::ROSBuffer output; - EXPECT_FALSE( - RosPackedFixedMessage::ProtobufToROS(malformed, output).ok()); + EXPECT_FALSE(RosPackedFixedMessage::ProtobufToROS(malformed, output).ok()); } TEST(ROSWireConversionTest, FixedOutputAndErrorsAreReported) { @@ -356,9 +352,9 @@ TEST(ROSWireConversionTest, FixedOutputAndErrorsAreReported) { RosCompileMessage::ProtobufToROS(std::string(1, '\x80'), output).ok()); EXPECT_TRUE(output.empty()); EXPECT_FALSE(RosCompileMessage::PhaserToROS({}, output).ok()); - EXPECT_FALSE(RosCompileMessage::ConvertToROS( - absl::Span("\0", 1), output) - .ok()); + EXPECT_FALSE( + RosCompileMessage::ConvertToROS(absl::Span("\0", 1), output) + .ok()); } TEST(ROSWireConversionTest, OneofWritesFieldNumberDiscriminator) { @@ -394,16 +390,16 @@ TEST(ROSWireConversionTest, ROS1IntrinsicsUseNativeLayoutsAndFlushCaches) { ::phaser::ROSBuffer protobuf_output; ASSERT_TRUE(RosIntrinsicMessage::ProtobufToROS(message.SerializeAsString(), - protobuf_output) + protobuf_output) .ok()); EXPECT_EQ(protobuf_output.AsString(), expected); ::phaser::ROSBuffer native_output; const auto* native_data = reinterpret_cast(message.Data()); - ASSERT_TRUE(RosIntrinsicMessage::PhaserToROS( - absl::Span(native_data, message.Size()), - native_output) - .ok()); + ASSERT_TRUE( + RosIntrinsicMessage::PhaserToROS( + absl::Span(native_data, message.Size()), native_output) + .ok()); EXPECT_EQ(native_output.AsString(), expected); ::phaser::ROSBuffer protobuf_frontend_native_output; @@ -414,12 +410,40 @@ TEST(ROSWireConversionTest, ROS1IntrinsicsUseNativeLayoutsAndFlushCaches) { EXPECT_EQ(protobuf_frontend_native_output.AsString(), expected); } +TEST(ROSWireConversionTest, ROSNarrowScalarsUseDeclaredWireWidths) { + RosNarrowScalars message; + message.uint8_value = 0xa1; + message.int8_value = -2; + message.uint16_value = 0xb2c3; + message.int16_value = -4; + + std::string expected; + AppendIntegral(expected, static_cast(0xa1)); + AppendIntegral(expected, static_cast(-2)); + AppendIntegral(expected, static_cast(0xb2c3)); + AppendIntegral(expected, static_cast(-4)); + + std::string output; + ASSERT_TRUE(message.SerializeToROSString(&output).ok()); + EXPECT_EQ(output, expected); + EXPECT_EQ(message.ROSSerializedSize(), expected.size()); + + RosNarrowScalars decoded; + ASSERT_TRUE( + decoded.ParseFromROS(absl::Span(output.data(), output.size())) + .ok()); + EXPECT_EQ(decoded.uint8_value.Get(), 0xa1); + EXPECT_EQ(decoded.int8_value.Get(), -2); + EXPECT_EQ(decoded.uint16_value.Get(), 0xb2c3); + EXPECT_EQ(decoded.int16_value.Get(), -4); +} + TEST(ROSWireConversionTest, ParsesKnownROSBytesIntoNativePayload) { RosCompileMessage message; const std::string input = ExpectedRosCompileBytes(); - ASSERT_TRUE(message.ParseFromROS( - absl::Span(input.data(), input.size())) - .ok()); + ASSERT_TRUE( + message.ParseFromROS(absl::Span(input.data(), input.size())) + .ok()); EXPECT_EQ(message.x.Get(), -7); EXPECT_EQ(message.name.Get(), "robot"); @@ -456,23 +480,23 @@ TEST(ROSWireConversionTest, ParsesKnownROSBytesIntoNativePayload) { std::vector direct_wire(4096); ::phaser::ProtoBuffer direct_output(direct_wire.data(), direct_wire.size()); - ASSERT_TRUE(RosCompileMessage::ROSToProtobuf( - absl::Span(input.data(), input.size()), - direct_output) - .ok()); + ASSERT_TRUE( + RosCompileMessage::ROSToProtobuf( + absl::Span(input.data(), input.size()), direct_output) + .ok()); ::foo::bar::RosCompileMessage direct_protobuf; ASSERT_TRUE(direct_protobuf.ParseFromArray(direct_wire.data(), - direct_output.Size())); + int(direct_output.Size()))); EXPECT_EQ(direct_protobuf.SerializeAsString(), protobuf.SerializeAsString()); } TEST(ROSWireConversionTest, ParsedROSPayloadUsesEitherFrontend) { RosIntrinsicMessage ros_message; const std::string input = ExpectedIntrinsicBytes(); - ASSERT_TRUE(ros_message - .ParseFromROS( - absl::Span(input.data(), input.size())) - .ok()); + ASSERT_TRUE( + ros_message + .ParseFromROS(absl::Span(input.data(), input.size())) + .ok()); EXPECT_EQ(ros_message.stamp->sec, 12u); EXPECT_EQ(ros_message.stamp->nsec, 345u); @@ -517,10 +541,10 @@ TEST(ROSWireConversionTest, ParsedROSPayloadUsesEitherFrontend) { EXPECT_EQ(protobuf_view.fixed_timeouts(1).nanos(), 14); ProtobufFrontendIntrinsicMessage parsed_protobuf_frontend; - ASSERT_TRUE(parsed_protobuf_frontend - .ParseFromROS( - absl::Span(input.data(), input.size())) - .ok()); + ASSERT_TRUE( + parsed_protobuf_frontend + .ParseFromROS(absl::Span(input.data(), input.size())) + .ok()); EXPECT_EQ(parsed_protobuf_frontend.stamp().seconds(), 12); EXPECT_EQ(parsed_protobuf_frontend.timeout().nanos(), 500); EXPECT_EQ(parsed_protobuf_frontend.header().stamp().nanos(), 654); @@ -528,13 +552,13 @@ TEST(ROSWireConversionTest, ParsedROSPayloadUsesEitherFrontend) { std::vector direct_wire(4096); ::phaser::ProtoBuffer direct_output(direct_wire.data(), direct_wire.size()); - ASSERT_TRUE(RosIntrinsicMessage::ROSToProtobuf( - absl::Span(input.data(), input.size()), - direct_output) - .ok()); + ASSERT_TRUE( + RosIntrinsicMessage::ROSToProtobuf( + absl::Span(input.data(), input.size()), direct_output) + .ok()); ProtobufFrontendIntrinsicMessage direct_protobuf; - ASSERT_TRUE(direct_protobuf.ParseFromArray(direct_wire.data(), - direct_output.Size())); + ASSERT_TRUE( + direct_protobuf.ParseFromArray(direct_wire.data(), direct_output.Size())); EXPECT_EQ(direct_protobuf.stamp().seconds(), 12); EXPECT_EQ(direct_protobuf.timeout().nanos(), 500); EXPECT_EQ(direct_protobuf.header().frame_id(), "map"); @@ -544,14 +568,13 @@ TEST(ROSWireConversionTest, ParsesScalarAndMessageOneofArms) { std::string scalar_input = ExpectedRosCompileBytes(false); scalar_input.resize(scalar_input.size() - sizeof(uint32_t)); AppendIntegral(scalar_input, static_cast(15)); - AppendIntegral(scalar_input, static_cast(123)); + AppendIntegral(scalar_input, int32_t{123}); RosCompileMessage scalar_message; - ASSERT_TRUE( - scalar_message - .ParseFromROS( - absl::Span(scalar_input.data(), scalar_input.size())) - .ok()); + ASSERT_TRUE(scalar_message + .ParseFromROS(absl::Span(scalar_input.data(), + scalar_input.size())) + .ok()); using ChoiceCount = RosCompileMessage::ChoiceCountAlternative; ASSERT_TRUE(scalar_message.choice.holds_alternative()); EXPECT_EQ(scalar_message.choice.get(), 123); @@ -559,14 +582,13 @@ TEST(ROSWireConversionTest, ParsesScalarAndMessageOneofArms) { std::string message_input = ExpectedRosCompileBytes(false); message_input.resize(message_input.size() - sizeof(uint32_t)); AppendIntegral(message_input, static_cast(18)); - AppendIntegral(message_input, static_cast(456)); + AppendIntegral(message_input, int32_t{456}); RosCompileMessage message; - ASSERT_TRUE( - message - .ParseFromROS( - absl::Span(message_input.data(), message_input.size())) - .ok()); + ASSERT_TRUE(message + .ParseFromROS(absl::Span(message_input.data(), + message_input.size())) + .ok()); using ChoiceInner = RosCompileMessage::ChoiceInnerAlternative; ASSERT_TRUE(message.choice.holds_alternative()); EXPECT_EQ(message.choice.get().id.Get(), 456); @@ -586,11 +608,10 @@ TEST(ROSWireConversionTest, RejectsMalformedROSInput) { std::string trailing = valid; trailing.push_back('\0'); RosCompileMessage trailing_message; - EXPECT_FALSE( - trailing_message - .ParseFromROS( - absl::Span(trailing.data(), trailing.size())) - .ok()); + EXPECT_FALSE(trailing_message + .ParseFromROS( + absl::Span(trailing.data(), trailing.size())) + .ok()); std::string invalid_length = valid; invalid_length[4] = static_cast(0xff); @@ -598,11 +619,10 @@ TEST(ROSWireConversionTest, RejectsMalformedROSInput) { invalid_length[6] = static_cast(0xff); invalid_length[7] = static_cast(0x7f); RosCompileMessage invalid_length_message; - EXPECT_FALSE( - invalid_length_message - .ParseFromROS(absl::Span(invalid_length.data(), - invalid_length.size())) - .ok()); + EXPECT_FALSE(invalid_length_message + .ParseFromROS(absl::Span(invalid_length.data(), + invalid_length.size())) + .ok()); std::string invalid_discriminator = valid; const size_t discriminator_offset = @@ -616,19 +636,18 @@ TEST(ROSWireConversionTest, RejectsMalformedROSInput) { .ok()); std::string oversized_sequence; - AppendIntegral(oversized_sequence, static_cast(0)); + AppendIntegral(oversized_sequence, int32_t{0}); AppendString(oversized_sequence, ""); AppendIntegral(oversized_sequence, static_cast(0)); AppendDouble(oversized_sequence, 0); - AppendIntegral(oversized_sequence, static_cast(0)); - AppendIntegral(oversized_sequence, static_cast(0)); + AppendIntegral(oversized_sequence, int32_t{0}); + AppendIntegral(oversized_sequence, int32_t{0}); AppendIntegral(oversized_sequence, static_cast(100)); RosCompileMessage oversized_sequence_message; - EXPECT_FALSE( - oversized_sequence_message - .ParseFromROS(absl::Span(oversized_sequence.data(), - oversized_sequence.size())) - .ok()); + EXPECT_FALSE(oversized_sequence_message + .ParseFromROS(absl::Span( + oversized_sequence.data(), oversized_sequence.size())) + .ok()); EXPECT_TRUE(oversized_sequence_message.xs.empty()); } diff --git a/phaser/runtime/BUILD.bazel b/phaser/runtime/BUILD.bazel index aaceb53..0df8a40 100644 --- a/phaser/runtime/BUILD.bazel +++ b/phaser/runtime/BUILD.bazel @@ -1,5 +1,9 @@ load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") -load("//phaser:copts.bzl", "PHASER_COPTS") +load( + "//phaser:copts.bzl", + "PHASER_COPTS", + "PHASER_PROTOBUF_GENERATED_COPTS", +) package(default_visibility = ["//visibility:public"]) @@ -53,7 +57,7 @@ cc_test( srcs = [ "message_test.cc", ], - copts = PHASER_COPTS, + copts = PHASER_COPTS + PHASER_PROTOBUF_GENERATED_COPTS, data = ["//phaser:valgrind.supp"], deps = [ ":phaser_runtime", diff --git a/phaser/runtime/any.h b/phaser/runtime/any.h index 5ec0f45..0e57c11 100644 --- a/phaser/runtime/any.h +++ b/phaser/runtime/any.h @@ -26,8 +26,13 @@ namespace phaser { +#if defined(__clang__) #pragma clang diagnostic push #pragma clang diagnostic ignored "-Winvalid-offsetof" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Winvalid-offsetof" +#endif // Hand-coded message class that represents a google.protobuf.Any message. class AnyMessage : public Message { @@ -374,7 +379,11 @@ class AnyMessage : public Message { phaser::StringField value_; }; +#if defined(__clang__) #pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#endif class AnyField : public IndirectMessageField { public: diff --git a/phaser/runtime/arrays.h b/phaser/runtime/arrays.h index 9f19db0..67a1ca5 100644 --- a/phaser/runtime/arrays.h +++ b/phaser/runtime/arrays.h @@ -35,8 +35,9 @@ inline bool HasMutablePayload( template struct FixedArrayConstIterator { - FixedArrayConstIterator(const Field* f, size_t idx, bool reverse = false) - : field(f), index(idx), reverse(reverse) {} + FixedArrayConstIterator(const Field* f, size_t idx, + bool reverse_iteration = false) + : field(f), index(idx), reverse(reverse_iteration) {} FixedArrayConstIterator& operator++() { if (reverse) { diff --git a/phaser/runtime/fields.h b/phaser/runtime/fields.h index d122b73..23647d0 100644 --- a/phaser/runtime/fields.h +++ b/phaser/runtime/fields.h @@ -9,8 +9,12 @@ #include #include +#include #include +#include +#include #include +#include #include #include #include @@ -118,6 +122,244 @@ class Field { mutable int indent_ = 0; }; +// Common std::string-like API for payload-backed string fields. Read operations +// remain zero-copy. Operations that can change the string length construct the +// new value outside the payload first so aliases into a relocating payload stay +// valid until the mutation is ready to commit. +template +class StringFieldFacade { + public: + using value_type = char; + using size_type = size_t; + using difference_type = ptrdiff_t; + using const_reference = const char&; + using const_pointer = const char*; + using const_iterator = std::string_view::const_iterator; + using const_reverse_iterator = std::string_view::const_reverse_iterator; + + static constexpr size_type npos = std::string_view::npos; + + bool empty() const { return View().empty(); } + size_type length() const { return View().length(); } + size_type max_size() const { return std::string{}.max_size(); } + size_type capacity() const { return View().size(); } + + char operator[](size_type pos) const { return View()[pos]; } + char at(size_type pos) const { return View().at(pos); } + char front() const { return View().front(); } + char back() const { return View().back(); } + + const_iterator begin() const { return View().begin(); } + const_iterator end() const { return View().end(); } + const_iterator cbegin() const { return View().cbegin(); } + const_iterator cend() const { return View().cend(); } + const_reverse_iterator rbegin() const { return View().rbegin(); } + const_reverse_iterator rend() const { return View().rend(); } + const_reverse_iterator crbegin() const { return View().crbegin(); } + const_reverse_iterator crend() const { return View().crend(); } + + int compare(std::string_view other) const { return View().compare(other); } + int compare(size_type pos, size_type count, std::string_view other) const { + return View().compare(pos, count, other); + } + int compare(size_type pos, size_type count, std::string_view other, + size_type other_pos, size_type other_count = npos) const { + return View() + .substr(pos, count) + .compare(other.substr(other_pos, other_count)); + } + + bool starts_with(std::string_view prefix) const { + const std::string_view value = View(); + return value.size() >= prefix.size() && + value.substr(0, prefix.size()) == prefix; + } + bool starts_with(char prefix) const { + const std::string_view value = View(); + return !value.empty() && value.front() == prefix; + } + bool ends_with(std::string_view suffix) const { + const std::string_view value = View(); + return value.size() >= suffix.size() && + value.substr(value.size() - suffix.size()) == suffix; + } + bool ends_with(char suffix) const { + const std::string_view value = View(); + return !value.empty() && value.back() == suffix; + } + bool contains(std::string_view value) const { return find(value) != npos; } + bool contains(char value) const { return find(value) != npos; } + + size_type find(std::string_view value, size_type pos = 0) const { + return View().find(value, pos); + } + size_type find(char value, size_type pos = 0) const { + return View().find(value, pos); + } + size_type find(const char* value, size_type pos, size_type count) const { + return View().find(std::string_view(value, count), pos); + } + size_type rfind(std::string_view value, size_type pos = npos) const { + return View().rfind(value, pos); + } + size_type rfind(char value, size_type pos = npos) const { + return View().rfind(value, pos); + } + size_type rfind(const char* value, size_type pos, size_type count) const { + return View().rfind(std::string_view(value, count), pos); + } + size_type find_first_of(std::string_view value, size_type pos = 0) const { + return View().find_first_of(value, pos); + } + size_type find_first_of(char value, size_type pos = 0) const { + return View().find_first_of(value, pos); + } + size_type find_last_of(std::string_view value, size_type pos = npos) const { + return View().find_last_of(value, pos); + } + size_type find_last_of(char value, size_type pos = npos) const { + return View().find_last_of(value, pos); + } + size_type find_first_not_of(std::string_view value, size_type pos = 0) const { + return View().find_first_not_of(value, pos); + } + size_type find_first_not_of(char value, size_type pos = 0) const { + return View().find_first_not_of(value, pos); + } + size_type find_last_not_of(std::string_view value, + size_type pos = npos) const { + return View().find_last_not_of(value, pos); + } + size_type find_last_not_of(char value, size_type pos = npos) const { + return View().find_last_not_of(value, pos); + } + + std::string substr(size_type pos = 0, size_type count = npos) const { + return std::string(View().substr(pos, count)); + } + size_type copy(char* destination, size_type count, size_type pos = 0) const { + return View().copy(destination, count, pos); + } + + Derived& assign(std::string_view value) { + const std::string value_copy(value); + Mutable().Set(value_copy); + return Mutable(); + } + Derived& assign(const char* value, size_type count) { + return assign(std::string_view(value, count)); + } + Derived& assign(size_type count, char value) { + Mutable().Set(std::string(count, value)); + return Mutable(); + } + Derived& append(std::string_view value) { + return Mutate([value](std::string& current) { current.append(value); }); + } + Derived& append(const char* value, size_type count) { + return append(std::string_view(value, count)); + } + Derived& append(size_type count, char value) { + return Mutate( + [count, value](std::string& current) { current.append(count, value); }); + } + Derived& operator+=(std::string_view value) { return append(value); } + Derived& operator+=(const char* value) { return append(value); } + Derived& operator+=(char value) { + push_back(value); + return Mutable(); + } + void push_back(char value) { + Mutate([value](std::string& current) { current.push_back(value); }); + } + void pop_back() { + Mutate([](std::string& current) { current.pop_back(); }); + } + void resize(size_type count, char value = char()) { + Mutate( + [count, value](std::string& current) { current.resize(count, value); }); + } + Derived& erase(size_type pos = 0, size_type count = npos) { + return Mutate( + [pos, count](std::string& current) { current.erase(pos, count); }); + } + Derived& insert(size_type pos, std::string_view value) { + return Mutate( + [pos, value](std::string& current) { current.insert(pos, value); }); + } + Derived& insert(size_type pos, size_type count, char value) { + return Mutate([pos, count, value](std::string& current) { + current.insert(pos, count, value); + }); + } + Derived& replace(size_type pos, size_type count, std::string_view value) { + return Mutate([pos, count, value](std::string& current) { + current.replace(pos, count, value); + }); + } + void clear() { Mutable().Clear(); } + void swap(Derived& other) { + if (&Mutable() == &other) { + return; + } + const std::string mine(View()); + const std::string theirs(other.Get()); + Mutable().Set(theirs); + other.Set(mine); + } + + bool operator==(const Derived& other) const { return View() == other.Get(); } + bool operator!=(const Derived& other) const { return View() != other.Get(); } + bool operator==(std::string_view other) const { return View() == other; } + bool operator!=(std::string_view other) const { return View() != other; } + bool operator<(std::string_view other) const { return View() < other; } + bool operator<=(std::string_view other) const { return View() <= other; } + bool operator>(std::string_view other) const { return View() > other; } + bool operator>=(std::string_view other) const { return View() >= other; } + + friend bool operator==(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs == rhs.View(); + } + friend bool operator!=(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs != rhs.View(); + } + friend bool operator<(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs < rhs.View(); + } + friend bool operator<=(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs <= rhs.View(); + } + friend bool operator>(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs > rhs.View(); + } + friend bool operator>=(std::string_view lhs, const StringFieldFacade& rhs) { + return lhs >= rhs.View(); + } + friend std::string operator+(const StringFieldFacade& lhs, + std::string_view rhs) { + std::string result(lhs.View()); + result.append(rhs); + return result; + } + friend std::string operator+(std::string lhs, const StringFieldFacade& rhs) { + lhs.append(rhs.View()); + return lhs; + } + + private: + const Derived& Const() const { return static_cast(*this); } + Derived& Mutable() { return static_cast(*this); } + std::string_view View() const { return Const().Get(); } + + template + Derived& Mutate(Mutator mutator) { + std::string value(View()); + mutator(value); + Mutable().Set(value); + return Mutable(); + } +}; + #define DEFINE_PRIMITIVE_FIELD(cname, type) \ template \ class cname##Field : public Field { \ @@ -368,7 +610,7 @@ class EnumField : public Field { }; // String field with an offset inline in the message. -class StringField : public Field { +class StringField : public Field, public StringFieldFacade { public: StringField() = default; explicit StringField(uint32_t source_offset, uint32_t relative_binary_offset, @@ -463,11 +705,6 @@ class StringField : public Field { GetMessageBinaryStart() + relative_binary_offset_, clear); } - bool operator==(const StringField& other) const { - return Get() == other.Get(); - } - bool operator!=(const StringField& other) const { return !(*this == other); } - size_t size() const { int32_t offset = FindFieldOffset(source_offset_); if (offset < 0) { @@ -534,7 +771,8 @@ class StringField : public Field { // This is a string field that is not embedded inside a message. They // store the std::shared_ptr to the phaser::Runtime pointer instead of // an offset to the start of the message. -class NonEmbeddedStringField { +class NonEmbeddedStringField + : public StringFieldFacade { public: NonEmbeddedStringField() = default; explicit NonEmbeddedStringField(const Message* msg, @@ -596,9 +834,9 @@ class NonEmbeddedStringField { return *this; } NonEmbeddedStringField& operator=(const char* s) { - ::toolbelt::PayloadBuffer::SetString( - GetBufferAddr(), std::string_view(s, std::strlen(s)), - absolute_binary_offset_); + ::toolbelt::PayloadBuffer::SetString(GetBufferAddr(), + std::string_view(s, std::strlen(s)), + absolute_binary_offset_); return *this; } @@ -626,13 +864,6 @@ class NonEmbeddedStringField { absolute_binary_offset_); } - bool operator==(const NonEmbeddedStringField& other) const { - return Get() == other.Get(); - } - bool operator!=(const NonEmbeddedStringField& other) const { - return !(*this == other); - } - size_t size() const { if (IsPlaceholder()) { return 0; @@ -646,8 +877,6 @@ class NonEmbeddedStringField { } return GetBuffer()->StringData(absolute_binary_offset_); } - bool empty() const { return size() == 0; } - bool IsPlaceholder() const { return msg_ == nullptr; } // Number of bytes the raw string occupies on the wire (not including any @@ -667,10 +896,10 @@ class NonEmbeddedStringField { } const Message* msg_ = nullptr; - ::toolbelt::BufferOffset - absolute_binary_offset_ = 0; // Offset into - // ::toolbelt::PayloadBuffer of - // toolbelt::StringHeader + ::toolbelt::BufferOffset absolute_binary_offset_ = + 0; // Offset into + // ::toolbelt::PayloadBuffer of + // toolbelt::StringHeader }; // This is a buffer offset containing the absolute offset of a message in the diff --git a/phaser/runtime/iterators.h b/phaser/runtime/iterators.h index 715907f..a84d4c3 100644 --- a/phaser/runtime/iterators.h +++ b/phaser/runtime/iterators.h @@ -9,8 +9,10 @@ #include #include +#include #include #include +#include #include #include "absl/status/status.h" @@ -22,6 +24,12 @@ namespace phaser { template struct FieldIterator { + using iterator_category = std::bidirectional_iterator_tag; + using value_type = std::remove_const_t; + using difference_type = ptrdiff_t; + using pointer = T*; + using reference = T&; + FieldIterator(const Field* f, ::toolbelt::BufferOffset o, bool r = false) : field(f), offset(o), reverse(r) {} @@ -41,17 +49,31 @@ struct FieldIterator { } return *this; } - FieldIterator operator+(size_t i) { + FieldIterator operator++(int) { + FieldIterator result = *this; + ++*this; + return result; + } + FieldIterator operator--(int) { + FieldIterator result = *this; + --*this; + return result; + } + FieldIterator operator+(size_t i) const { + const auto byte_offset = + static_cast<::toolbelt::BufferOffset>(i * sizeof(T)); if (reverse) { - return FieldIterator(field, field->BaseOffset() - i * sizeof(T), true); + return FieldIterator(field, offset - byte_offset, true); } - return FieldIterator(field, field->BaseOffset() + i * sizeof(T)); + return FieldIterator(field, offset + byte_offset); } - FieldIterator operator-(size_t i) { + FieldIterator operator-(size_t i) const { + const auto byte_offset = + static_cast<::toolbelt::BufferOffset>(i * sizeof(T)); if (reverse) { - return FieldIterator(field, field->BaseOffset() + i * sizeof(T), true); + return FieldIterator(field, offset + byte_offset, true); } - return FieldIterator(field, field->BaseOffset() - i * sizeof(T)); + return FieldIterator(field, offset - byte_offset); } T& operator*() const { T* addr = field->GetBuffer()->template ToAddress(offset); @@ -70,6 +92,12 @@ struct FieldIterator { template struct StringFieldIterator { + using iterator_category = std::bidirectional_iterator_tag; + using value_type = std::string_view; + using difference_type = ptrdiff_t; + using pointer = void; + using reference = std::string_view; + StringFieldIterator(const Field* f, ::toolbelt::BufferOffset o, bool r = false) : field(f), offset(o), reverse(r) {} @@ -90,23 +118,31 @@ struct StringFieldIterator { } return *this; } - StringFieldIterator operator+(size_t i) { + StringFieldIterator operator++(int) { + StringFieldIterator result = *this; + ++*this; + return result; + } + StringFieldIterator operator--(int) { + StringFieldIterator result = *this; + --*this; + return result; + } + StringFieldIterator operator+(size_t i) const { if (reverse) { return StringFieldIterator( - field, field->BaseOffset() - i * sizeof(::toolbelt::BufferOffset), - true); + field, offset - i * sizeof(::toolbelt::BufferOffset), true); } - return StringFieldIterator( - field, field->BaseOffset() + i * sizeof(::toolbelt::BufferOffset)); + return StringFieldIterator(field, + offset + i * sizeof(::toolbelt::BufferOffset)); } - StringFieldIterator operator-(size_t i) { + StringFieldIterator operator-(size_t i) const { if (reverse) { return StringFieldIterator( - field, field->BaseOffset() + i * sizeof(::toolbelt::BufferOffset), - true); + field, offset + i * sizeof(::toolbelt::BufferOffset), true); } - return StringFieldIterator( - field, field->BaseOffset() - i * sizeof(::toolbelt::BufferOffset)); + return StringFieldIterator(field, + offset - i * sizeof(::toolbelt::BufferOffset)); } std::string_view operator*() const { return field->GetBuffer()->GetStringView(field->BaseOffset() + offset); @@ -126,6 +162,12 @@ struct StringFieldIterator { template struct EnumFieldIterator { + using iterator_category = std::bidirectional_iterator_tag; + using value_type = std::remove_const_t; + using difference_type = ptrdiff_t; + using pointer = T*; + using reference = T&; + EnumFieldIterator(const Field* f, ::toolbelt::BufferOffset o, bool r = false) : field(f), offset(o), reverse(r) {} @@ -145,29 +187,37 @@ struct EnumFieldIterator { } return *this; } - EnumFieldIterator operator+(size_t i) { + EnumFieldIterator operator++(int) { + EnumFieldIterator result = *this; + ++*this; + return result; + } + EnumFieldIterator operator--(int) { + EnumFieldIterator result = *this; + --*this; + return result; + } + EnumFieldIterator operator+(size_t i) const { + using Value = std::remove_const_t; + const auto byte_offset = static_cast<::toolbelt::BufferOffset>( + i * sizeof(typename std::underlying_type::type)); if (reverse) { - return EnumFieldIterator( - field, - field->BaseOffset() - i * sizeof(std::underlying_type::type), - true); + return EnumFieldIterator(field, offset - byte_offset, true); } - return EnumFieldIterator( - field, field->BaseOffset() + i * sizeof(std::underlying_type::type)); + return EnumFieldIterator(field, offset + byte_offset); } - EnumFieldIterator operator-(size_t i) { + EnumFieldIterator operator-(size_t i) const { + using Value = std::remove_const_t; + const auto byte_offset = static_cast<::toolbelt::BufferOffset>( + i * sizeof(typename std::underlying_type::type)); if (reverse) { - return EnumFieldIterator( - field, - field->BaseOffset() + i * sizeof(std::underlying_type::type), - true); + return EnumFieldIterator(field, offset + byte_offset, true); } - return EnumFieldIterator( - field, field->BaseOffset() - i * sizeof(std::underlying_type::type)); + return EnumFieldIterator(field, offset - byte_offset); } T& operator*() const { - using U = typename std::underlying_type::type; + using U = typename std::underlying_type>::type; U* addr = field->GetBuffer()->template ToAddress(offset); // An enum and its fixed underlying type share representation; route the // cast through void* so it is not flagged as a dereference of an unrelated diff --git a/phaser/runtime/message.h b/phaser/runtime/message.h index 8b20d72..8dfa833 100644 --- a/phaser/runtime/message.h +++ b/phaser/runtime/message.h @@ -33,8 +33,10 @@ namespace phaser { // FieldData is a structure that contains the field numbers and offsets for a // message. It is stored in the payload buffer. +#if defined(__clang__) #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wc99-extensions" +#endif struct FieldData { uint32_t num; struct { @@ -43,7 +45,9 @@ struct FieldData { uint32_t id : 8; // Field id for presence bit mask. } fields[]; // Flexible array member; data lives in the payload buffer. }; +#if defined(__clang__) #pragma clang diagnostic pop +#endif // Hybrid field metadata stores a compact direct-indexed range followed by // sorted sparse entries. The arrays immediately following HybridFieldData are: diff --git a/phaser/runtime/message_test.cc b/phaser/runtime/message_test.cc index 5a03024..de905de 100644 --- a/phaser/runtime/message_test.cc +++ b/phaser/runtime/message_test.cc @@ -510,6 +510,9 @@ static phaser::BankInfo innerMessageBackInfo{ .serialize_to_buffer = InnerMessageSerializeToBuffer, .deserialize_from_buffer = InnerMessageDeserializeFromBuffer, .serialized_size = InnerMessageSerializedSize, + .serialize_at_offset = nullptr, + .deserialize_at_offset = nullptr, + .serialized_size_at_offset = nullptr, .allocate_at_offset = InnerMessageAllocateAtOffset, .allocate = InnerMessageAllocate, .clear = InnerMessageClear, @@ -1272,6 +1275,9 @@ static phaser::BankInfo kTestMessageBackInfo{ .serialize_to_buffer = TestMessageSerializeToBuffer, .deserialize_from_buffer = TestMessageDeserializeFromBuffer, .serialized_size = TestMessageSerializedSize, + .serialize_at_offset = nullptr, + .deserialize_at_offset = nullptr, + .serialized_size_at_offset = nullptr, .allocate_at_offset = TestMessageAllocateAtOffset, .allocate = TestMessageAllocate, .clear = TestMessageClear, diff --git a/phaser/runtime/ros_wireformat_test.cc b/phaser/runtime/ros_wireformat_test.cc index 16ba61d..93e9027 100644 --- a/phaser/runtime/ros_wireformat_test.cc +++ b/phaser/runtime/ros_wireformat_test.cc @@ -69,7 +69,7 @@ TEST(ROSWireformatTest, RejectsInvalidRawWrite) { TEST(ROSWireformatTest, ReadsCanonicalLittleEndianBytes) { ROSBuffer buffer; ASSERT_TRUE(buffer.Write(static_cast(-2)).ok()); - ASSERT_TRUE(buffer.Write(static_cast(0xf2345678)).ok()); + ASSERT_TRUE(buffer.Write(0xf2345678U).ok()); ASSERT_TRUE(buffer.Write(true).ok()); ASSERT_TRUE(buffer.Write(1.5F).ok()); ASSERT_TRUE(buffer.Write(-2.25).ok()); diff --git a/phaser/runtime/vectors.h b/phaser/runtime/vectors.h index 2b3ddad..d3a8f25 100644 --- a/phaser/runtime/vectors.h +++ b/phaser/runtime/vectors.h @@ -9,8 +9,11 @@ #include #include +#include +#include #include #include +#include #include #include @@ -29,7 +32,7 @@ class ProtoBuffer; #define DECLARE_ZERO_COPY_VECTOR_BITS(vtype, itype, ctype, utype) \ using value_type = vtype; \ using reference = value_type&; \ - using const_reference = value_type&; \ + using const_reference = const value_type&; \ using pointer = value_type*; \ using const_pointer = const value_type*; \ using size_type = size_t; \ @@ -37,8 +40,8 @@ class ProtoBuffer; \ using iterator = itype; \ using const_iterator = ctype; \ - using reverse_iterator = itype; \ - using const_reverse_iterator = ctype; \ + using reverse_iterator = std::reverse_iterator; \ + using const_reverse_iterator = std::reverse_iterator; \ \ iterator begin() { return iterator(this, BaseOffset()); } \ iterator end() { \ @@ -59,35 +62,19 @@ class ProtoBuffer; NumElements() * sizeof(value_type))); \ } \ \ - reverse_iterator rbegin() { \ - return reverse_iterator(this, BaseOffset(), true); \ - } \ - reverse_iterator rend() { \ - return reverse_iterator( \ - this, \ - BaseOffset() + static_cast<::toolbelt::BufferOffset>( \ - NumElements() * sizeof(value_type)), \ - true); \ - } \ + reverse_iterator rbegin() { return reverse_iterator(end()); } \ + reverse_iterator rend() { return reverse_iterator(begin()); } \ const_reverse_iterator rbegin() const { \ - return const_reverse_iterator(this, BaseOffset(), true); \ + return const_reverse_iterator(end()); \ } \ const_reverse_iterator rend() const { \ - return const_reverse_iterator( \ - this, \ - BaseOffset() + static_cast<::toolbelt::BufferOffset>( \ - NumElements() * sizeof(value_type)), \ - true); \ + return const_reverse_iterator(begin()); \ } \ const_reverse_iterator crbegin() const { \ - return const_reverse_iterator(this, BaseOffset(), true); \ + return const_reverse_iterator(cend()); \ } \ const_reverse_iterator crend() const { \ - return const_reverse_iterator( \ - this, \ - BaseOffset() + static_cast<::toolbelt::BufferOffset>( \ - NumElements() * sizeof(value_type)), \ - true); \ + return const_reverse_iterator(cbegin()); \ } // vtype: value type @@ -155,9 +142,22 @@ class PrimitiveVectorField : public Field { return base[index]; } - T front() { return (*this)[0]; } + T& at(size_t index) { + if (index >= size()) { + throw std::out_of_range("PrimitiveVectorField::at"); + } + return (*this)[static_cast(index)]; + } + T at(size_t index) const { + if (index >= size()) { + throw std::out_of_range("PrimitiveVectorField::at"); + } + return (*this)[static_cast(index)]; + } + + T& front() { return (*this)[0]; } const T front() const { return (*this)[0]; } - T back() { return (*this)[size() - 1]; } + T& back() { return (*this)[static_cast(size() - 1)]; } const T back() const { return (*this)[size() - 1]; } T Get(size_t index) const { return (*this)[static_cast(index)]; } @@ -174,7 +174,7 @@ class PrimitiveVectorField : public Field { std::vector v; size_t n = size(); for (size_t i = 0; i < n; i++) { - v.push_back((*this)[i]); + v.push_back((*this)[static_cast(i)]); } return v; } @@ -192,6 +192,18 @@ class PrimitiveVectorField : public Field { GetBufferAddr(), Header(relative_binary_offset_), v); } + template + T& emplace_back(Args&&... args) { + push_back(T(std::forward(args)...)); + return (*this)[static_cast(size() - 1)]; + } + + void pop_back() { + if (!empty()) { + resize(size() - 1); + } + } + void reserve(size_t n) { ::toolbelt::PayloadBuffer::VectorReserve( GetBufferAddr(), Header(relative_binary_offset_), n); @@ -201,6 +213,70 @@ class PrimitiveVectorField : public Field { ::toolbelt::PayloadBuffer::VectorResize( GetBufferAddr(), Header(relative_binary_offset_), n); } + void resize(size_t n, const T& value) { + const T value_copy = value; + const size_t old_size = size(); + resize(n); + for (size_t i = old_size; i < n; ++i) { + Set(i, value_copy); + } + } + + void assign(size_t count, const T& value) { + const T value_copy = value; + Clear(); + resize(count, value_copy); + } + template , int> = 0> + void assign(InputIterator first, InputIterator last) { + Clear(); + for (; first != last; ++first) { + push_back(*first); + } + } + void assign(std::initializer_list values) { + assign(values.begin(), values.end()); + } + + template + iterator insert(Iterator pos, const T& value) { + const size_t index = static_cast((pos.offset - BaseOffset()) / + static_cast(sizeof(T))); + std::vector values = Get(); + values.insert(values.begin() + static_cast(index), value); + assign(values.begin(), values.end()); + return begin() + index; + } + template + iterator emplace(Iterator pos, Args&&... args) { + return insert(pos, T(std::forward(args)...)); + } + template + iterator erase(Iterator pos) { + return erase(pos, pos + 1); + } + template + iterator erase(Iterator first, Iterator last) { + const size_t first_index = static_cast( + (first.offset - BaseOffset()) / static_cast(sizeof(T))); + const size_t last_index = static_cast( + (last.offset - BaseOffset()) / static_cast(sizeof(T))); + std::vector values = Get(); + values.erase(values.begin() + static_cast(first_index), + values.begin() + static_cast(last_index)); + assign(values.begin(), values.end()); + return begin() + first_index; + } + void swap(PrimitiveVectorField& other) { + if (this == &other) { + return; + } + const std::vector mine = Get(); + const std::vector theirs = other.Get(); + *this = theirs; + other = mine; + } void Clear() { ::toolbelt::PayloadBuffer::VectorClear(GetBufferAddr(), @@ -215,7 +291,7 @@ class PrimitiveVectorField : public Field { Clear(); reserve(other.size()); for (size_t i = 0; i < other.size(); i++) { - push_back(other[i]); + push_back(other[static_cast(i)]); } ResetFieldCache(); return *this; @@ -223,6 +299,15 @@ class PrimitiveVectorField : public Field { PrimitiveVectorField& operator=(PrimitiveVectorField&& other) noexcept { return operator=(static_cast(other)); } + PrimitiveVectorField& operator=(std::initializer_list values) { + assign(values); + return *this; + } + template + PrimitiveVectorField& operator=(const std::vector& values) { + assign(values.begin(), values.end()); + return *this; + } size_t size() const { return NumElements(); } T* data() { return GetRuntime()->template ToAddress(BaseOffset()); } @@ -278,9 +363,11 @@ class PrimitiveVectorField : public Field { return relative_binary_offset_; } - bool operator==( - const PrimitiveVectorField& other) const { - size_t n = size(); + bool operator==(const PrimitiveVectorField& other) const { + const size_t n = size(); + if (n != other.size()) { + return false; + } for (size_t i = 0; i < n; i++) { if ((*this)[i] != other[i]) { return false; @@ -288,10 +375,27 @@ class PrimitiveVectorField : public Field { } return true; } - bool operator!=( - const PrimitiveVectorField& other) const { + bool operator!=(const PrimitiveVectorField& other) const { return !(*this == other); } + template + bool operator==(const std::vector& other) const { + return size() == other.size() && std::equal(begin(), end(), other.begin()); + } + template + bool operator!=(const std::vector& other) const { + return !(*this == other); + } + template + friend bool operator==(const std::vector& lhs, + const PrimitiveVectorField& rhs) { + return rhs == lhs; + } + template + friend bool operator!=(const std::vector& lhs, + const PrimitiveVectorField& rhs) { + return !(rhs == lhs); + } size_t SerializedSize() const { size_t sz = size(); @@ -518,16 +622,30 @@ class EnumVectorField : public Field { return *reinterpret_cast(&base[index]); } - Enum front() { return (*this)[0]; } + Enum& at(size_t index) { + if (index >= size()) { + throw std::out_of_range("EnumVectorField::at"); + } + return (*this)[static_cast(index)]; + } + Enum at(size_t index) const { + if (index >= size()) { + throw std::out_of_range("EnumVectorField::at"); + } + return (*this)[static_cast(index)]; + } + + Enum& front() { return (*this)[0]; } const Enum front() const { return (*this)[0]; } - Enum back() { return (*this)[size() - 1]; } - const Enum back() const { return (*this)[size() - 1]; } + Enum& back() { return (*this)[static_cast(size() - 1)]; } + const Enum back() const { return (*this)[static_cast(size() - 1)]; } - const std::vector Get() const { - size_t n = size(); + std::vector Get() const { + const size_t n = size(); std::vector r; + r.reserve(n); for (size_t i = 0; i < n; i++) { - r[i] = (*this)[i]; + r.push_back((*this)[static_cast(i)]); } return r; } @@ -576,6 +694,18 @@ class EnumVectorField : public Field { GetBufferAddr(), Header(relative_binary_offset_), static_cast(v)); } + template + Enum& emplace_back(Args&&... args) { + push_back(Enum(std::forward(args)...)); + return (*this)[static_cast(size() - 1)]; + } + + void pop_back() { + if (!empty()) { + resize(size() - 1); + } + } + void reserve(size_t n) { ::toolbelt::PayloadBuffer::VectorReserve( GetBufferAddr(), Header(relative_binary_offset_), n); @@ -585,9 +715,71 @@ class EnumVectorField : public Field { ::toolbelt::PayloadBuffer::VectorResize( GetBufferAddr(), Header(relative_binary_offset_), n); } + void resize(size_t n, Enum value) { + const size_t old_size = size(); + resize(n); + for (size_t i = old_size; i < n; ++i) { + Set(i, value); + } + } void Add(Enum v) { push_back(v); } + void assign(size_t count, Enum value) { + Clear(); + resize(count, value); + } + template , int> = 0> + void assign(InputIterator first, InputIterator last) { + Clear(); + for (; first != last; ++first) { + push_back(*first); + } + } + void assign(std::initializer_list values) { + assign(values.begin(), values.end()); + } + + template + iterator insert(Iterator pos, Enum value) { + const size_t index = static_cast((pos.offset - BaseOffset()) / + static_cast(sizeof(T))); + std::vector values = Get(); + values.insert(values.begin() + static_cast(index), value); + assign(values.begin(), values.end()); + return begin() + index; + } + template + iterator emplace(Iterator pos, Args&&... args) { + return insert(pos, Enum(std::forward(args)...)); + } + template + iterator erase(Iterator pos) { + return erase(pos, pos + 1); + } + template + iterator erase(Iterator first, Iterator last) { + const size_t first_index = static_cast( + (first.offset - BaseOffset()) / static_cast(sizeof(T))); + const size_t last_index = static_cast( + (last.offset - BaseOffset()) / static_cast(sizeof(T))); + std::vector values = Get(); + values.erase(values.begin() + static_cast(first_index), + values.begin() + static_cast(last_index)); + assign(values.begin(), values.end()); + return begin() + first_index; + } + void swap(EnumVectorField& other) { + if (this == &other) { + return; + } + const std::vector mine = Get(); + const std::vector theirs = other.Get(); + *this = theirs; + other = mine; + } + void Clear() { ::toolbelt::PayloadBuffer::VectorClear(GetBufferAddr(), Header(relative_binary_offset_)); @@ -609,6 +801,15 @@ class EnumVectorField : public Field { EnumVectorField& operator=(EnumVectorField&& other) noexcept { return operator=(static_cast(other)); } + EnumVectorField& operator=(std::initializer_list values) { + assign(values); + return *this; + } + template + EnumVectorField& operator=(const std::vector& values) { + assign(values.begin(), values.end()); + return *this; + } size_t size() const { return NumElements(); } Enum* data() { return GetRuntime()->template ToAddress(BaseOffset()); } @@ -638,7 +839,10 @@ class EnumVectorField : public Field { bool operator==( const EnumVectorField& other) const { - size_t n = size(); + const size_t n = size(); + if (n != other.size()) { + return false; + } for (size_t i = 0; i < n; i++) { if ((*this)[i] != other[i]) { return false; @@ -650,6 +854,24 @@ class EnumVectorField : public Field { const EnumVectorField& other) const { return !(*this == other); } + template + bool operator==(const std::vector& other) const { + return size() == other.size() && std::equal(begin(), end(), other.begin()); + } + template + bool operator!=(const std::vector& other) const { + return !(*this == other); + } + template + friend bool operator==(const std::vector& lhs, + const EnumVectorField& rhs) { + return rhs == lhs; + } + template + friend bool operator!=(const std::vector& lhs, + const EnumVectorField& rhs) { + return !(rhs == lhs); + } size_t SerializedSize() const { size_t sz = size(); @@ -831,9 +1053,25 @@ class MessageVectorField : public Field { } T operator[](int index) { + if (GetRuntime() != nullptr && GetRuntime()->IsMutable()) { + return Mutable(static_cast(index)); + } return static_cast(this)->operator[](index); } + T at(size_t index) { + if (index >= size()) { + throw std::out_of_range("MessageVectorField::at"); + } + return (*this)[static_cast(index)]; + } + T at(size_t index) const { + if (index >= size()) { + throw std::out_of_range("MessageVectorField::at"); + } + return (*this)[static_cast(index)]; + } + T front() { return (*this)[0]; } T front() const { return (*this)[0]; } T back() { return (*this)[static_cast(size() - 1)]; } @@ -882,6 +1120,7 @@ class MessageVectorField : public Field { bool operator!=(const const_iterator& other) const { return !(*this == other); } + size_t Index() const { return index_; } private: const MessageVectorField* field_ = nullptr; @@ -895,42 +1134,24 @@ class MessageVectorField : public Field { iterator end() { return iterator(this, size()); } reverse_iterator rbegin() { return reverse_iterator(end()); } reverse_iterator rend() { return reverse_iterator(begin()); } - const_iterator begin() const { - return const_iterator(this, 0); - } - const_iterator end() const { - return const_iterator(this, size()); - } - const_iterator cbegin() const { - return begin(); - } - const_iterator cend() const { - return end(); - } + const_iterator begin() const { return const_iterator(this, 0); } + const_iterator end() const { return const_iterator(this, size()); } + const_iterator cbegin() const { return begin(); } + const_iterator cend() const { return end(); } const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } - const_reverse_iterator crbegin() const { - return rbegin(); - } - const_reverse_iterator crend() const { - return rend(); - } + const_reverse_iterator crbegin() const { return rbegin(); } + const_reverse_iterator crend() const { return rend(); } - void push_back(const T& v) { - ::toolbelt::BufferOffset offset = v.absolute_binary_offset; - ::toolbelt::PayloadBuffer::VectorPush<::toolbelt::BufferOffset>( - GetBufferAddr(), Header(), offset); - } + void push_back(const T& v) { (void)Add().CloneFrom(v); } - void push_back(T&& v) { - ::toolbelt::BufferOffset offset = v.absolute_binary_offset; - ::toolbelt::PayloadBuffer::VectorPush<::toolbelt::BufferOffset>( - GetBufferAddr(), Header(), offset); - } + void push_back(T&& v) { (void)Add().CloneFrom(v); } + + T emplace_back() { return Add(); } T Add() { // Allocate a new message. @@ -993,7 +1214,9 @@ class MessageVectorField : public Field { std::vector Allocate(size_t n) { std::vector result; result.reserve(n); - this->resize(n); + Clear(); + ::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>( + GetBufferAddr(), Header(), n); // Allocate memory for n messages in the payload buffer. std::vector addrs = ::toolbelt::PayloadBuffer::AllocateMany( GetBufferAddr(), T::BinarySize(), static_cast(n), true); @@ -1030,9 +1253,90 @@ class MessageVectorField : public Field { } void resize(size_t n) { - // Resize the vector data in the binary. This contains BufferOffets. + const size_t old_size = size(); + if (n < old_size) { + for (size_t i = n; i < old_size; ++i) { + auto hdr = Header(); + auto data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>( + hdr->data); + const ::toolbelt::BufferOffset offset = data[i]; + if (offset != 0) { + T(GetRuntime(), offset).Clear(); + GetBuffer()->Free(GetRuntime()->ToAddress(offset)); + } + } + } ::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>( GetBufferAddr(), Header(), n); + for (size_t i = old_size; i < n; ++i) { + (void)Mutable(i); + } + } + void resize(size_t n, const T& value) { + const size_t old_size = size(); + resize(n); + for (size_t i = old_size; i < n; ++i) { + (void)Mutable(i).CloneFrom(value); + } + } + + void pop_back() { + if (!empty()) { + resize(size() - 1); + } + } + + void assign(size_t count, const T& value) { + Clear(); + for (size_t i = 0; i < count; ++i) { + push_back(value); + } + } + template + void assign(InputIterator first, InputIterator last) { + Clear(); + for (; first != last; ++first) { + push_back(*first); + } + } + void assign(std::initializer_list values) { + assign(values.begin(), values.end()); + } + + iterator insert(const_iterator pos, const T& value) { + const size_t index = pos.Index(); + std::vector values = CopyValues(); + values.insert(values.begin() + static_cast(index), value); + assign(values.begin(), values.end()); + return iterator(this, index); + } + template + iterator emplace(const_iterator pos, Args&&... args) { + return insert(pos, T(std::forward(args)...)); + } + iterator erase(const_iterator pos) { + const size_t index = pos.Index(); + std::vector values = CopyValues(); + values.erase(values.begin() + static_cast(index)); + assign(values.begin(), values.end()); + return iterator(this, index); + } + iterator erase(const_iterator first, const_iterator last) { + const size_t first_index = first.Index(); + std::vector values = CopyValues(); + values.erase(values.begin() + static_cast(first_index), + values.begin() + static_cast(last.Index())); + assign(values.begin(), values.end()); + return iterator(this, first_index); + } + void swap(MessageVectorField& other) { + if (this == &other) { + return; + } + const std::vector mine = CopyValues(); + const std::vector theirs = other.CopyValues(); + *this = theirs; + other = mine; } void Clear() { @@ -1049,6 +1353,7 @@ class MessageVectorField : public Field { ::toolbelt::PayloadBuffer::VectorClear<::toolbelt::BufferOffset>( GetBufferAddr(), Header()); } + void clear() { Clear(); } size_t size() const { return NumElements(); } bool empty() const { return size() == 0; } @@ -1072,6 +1377,15 @@ class MessageVectorField : public Field { MessageVectorField& operator=(MessageVectorField&& other) noexcept { return operator=(static_cast(other)); } + MessageVectorField& operator=(std::initializer_list values) { + assign(values); + return *this; + } + template + MessageVectorField& operator=(const std::vector& values) { + assign(values.begin(), values.end()); + return *this; + } ::toolbelt::BufferOffset BinaryEndOffset() const { return relative_binary_offset_ + sizeof(toolbelt::VectorHeader); @@ -1094,6 +1408,32 @@ class MessageVectorField : public Field { bool operator!=(const MessageVectorField& other) const { return !operator==(other); } + template + bool operator==(const std::vector& other) const { + if (size() != other.size()) { + return false; + } + for (size_t i = 0; i < size(); ++i) { + if (Get(i) != other[i]) { + return false; + } + } + return true; + } + template + bool operator!=(const std::vector& other) const { + return !(*this == other); + } + template + friend bool operator==(const std::vector& lhs, + const MessageVectorField& rhs) { + return rhs == lhs; + } + template + friend bool operator!=(const std::vector& lhs, + const MessageVectorField& rhs) { + return !(rhs == lhs); + } std::vector Get() const { std::vector result; @@ -1156,6 +1496,17 @@ class MessageVectorField : public Field { } private: + std::vector CopyValues() const { + std::vector result; + result.reserve(size()); + for (size_t i = 0; i < size(); ++i) { + T copy; + (void)copy.CloneFrom(Get(i)); + result.push_back(std::move(copy)); + } + return result; + } + friend FieldIterator; friend FieldIterator; toolbelt::VectorHeader* Header( @@ -1261,10 +1612,33 @@ class StringVectorField : public Field { } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); + if (data[index] == 0) { + void* str_hdr = ::toolbelt::PayloadBuffer::Allocate( + GetBufferAddr(), sizeof(toolbelt::StringHeader)); + const ::toolbelt::BufferOffset string_offset = + GetRuntime()->ToOffset(str_hdr); + hdr = Header(); + data = + GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); + data[index] = string_offset; + } return NonEmbeddedStringField(Message::GetMessage(this, source_offset_), data[index]); } + NonEmbeddedStringField at(size_t index) { + if (index >= size()) { + throw std::out_of_range("StringVectorField::at"); + } + return (*this)[static_cast(index)]; + } + std::string_view at(size_t index) const { + if (index >= size()) { + throw std::out_of_range("StringVectorField::at"); + } + return Get(index); + } + using value_type = std::string_view; using reference = std::string_view; using const_reference = std::string_view; @@ -1308,6 +1682,7 @@ class StringVectorField : public Field { bool operator!=(const const_iterator& other) const { return !(*this == other); } + size_t Index() const { return index_; } private: const StringVectorField* field_ = nullptr; @@ -1321,30 +1696,18 @@ class StringVectorField : public Field { iterator end() { return iterator(this, size()); } reverse_iterator rbegin() { return reverse_iterator(end()); } reverse_iterator rend() { return reverse_iterator(begin()); } - const_iterator begin() const { - return const_iterator(this, 0); - } - const_iterator end() const { - return const_iterator(this, size()); - } - const_iterator cbegin() const { - return begin(); - } - const_iterator cend() const { - return end(); - } + const_iterator begin() const { return const_iterator(this, 0); } + const_iterator end() const { return const_iterator(this, size()); } + const_iterator cbegin() const { return begin(); } + const_iterator cend() const { return end(); } const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } - const_reverse_iterator crbegin() const { - return rbegin(); - } - const_reverse_iterator crend() const { - return rend(); - } + const_reverse_iterator crbegin() const { return rbegin(); } + const_reverse_iterator crend() const { return rend(); } size_t size() const { return NumElements(); } NonEmbeddedStringField* data() = delete; @@ -1354,7 +1717,9 @@ class StringVectorField : public Field { NonEmbeddedStringField front() { return (*this)[0]; } std::string_view front() const { return Get(0); } - NonEmbeddedStringField back() { return (*this)[static_cast(size() - 1)]; } + NonEmbeddedStringField back() { + return (*this)[static_cast(size() - 1)]; + } std::string_view back() const { return Get(size() - 1); } StringVectorField& operator=(const StringVectorField& other) { @@ -1385,12 +1750,19 @@ class StringVectorField : public Field { // Add an offset for the new string to the binary. ::toolbelt::PayloadBuffer::VectorPush<::toolbelt::BufferOffset>( GetBufferAddr(), Header(), hdr_offset); - } - void Add(const char* s, size_t len) { - push_back(std::string_view(s, len)); + template + NonEmbeddedStringField emplace_back(Str s) { + push_back(s); + return back(); + } + NonEmbeddedStringField emplace_back() { + push_back(std::string_view()); + return back(); } + + void Add(const char* s, size_t len) { push_back(std::string_view(s, len)); } template void Add(Str s) { push_back(s); @@ -1444,16 +1816,105 @@ class StringVectorField : public Field { } void resize(size_t n) { - // Resize the vector data in the binary. This contains BufferOffets. + const size_t old_size = size(); + if (n < old_size) { + for (size_t i = n; i < old_size; ++i) { + auto hdr = Header(); + auto data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>( + hdr->data); + const ::toolbelt::BufferOffset offset = data[i]; + if (offset != 0) { + NonEmbeddedStringField(Message::GetMessage(this, source_offset_), + offset) + .Clear(); + } + } + } ::toolbelt::PayloadBuffer::VectorResize<::toolbelt::BufferOffset>( GetBufferAddr(), Header(), n); } + void resize(size_t n, std::string_view value) { + const std::string value_copy(value); + const size_t old_size = size(); + resize(n); + for (size_t i = old_size; i < n; ++i) { + Set(i, value_copy); + } + } - void Clear() { - const size_t count = size(); + void pop_back() { + if (!empty()) { + resize(size() - 1); + } + } + + void assign(size_t count, std::string_view value) { + const std::string value_copy(value); + Clear(); + reserve(count); for (size_t i = 0; i < count; ++i) { - NonEmbeddedStringField field = (*this)[static_cast(i)]; - field.Clear(); + push_back(value_copy); + } + } + template + void assign(InputIterator first, InputIterator last) { + Clear(); + for (; first != last; ++first) { + push_back(*first); + } + } + void assign(std::initializer_list values) { + assign(values.begin(), values.end()); + } + + iterator insert(const_iterator pos, std::string_view value) { + const size_t index = pos.Index(); + std::vector values = CopyValues(); + values.insert(values.begin() + static_cast(index), + std::string(value)); + assign(values.begin(), values.end()); + return iterator(this, index); + } + template + iterator emplace(const_iterator pos, Args&&... args) { + return insert(pos, std::string(std::forward(args)...)); + } + iterator erase(const_iterator pos) { + const size_t index = pos.Index(); + std::vector values = CopyValues(); + values.erase(values.begin() + static_cast(index)); + assign(values.begin(), values.end()); + return iterator(this, index); + } + iterator erase(const_iterator first, const_iterator last) { + const size_t first_index = first.Index(); + std::vector values = CopyValues(); + values.erase(values.begin() + static_cast(first_index), + values.begin() + static_cast(last.Index())); + assign(values.begin(), values.end()); + return iterator(this, first_index); + } + void swap(StringVectorField& other) { + if (this == &other) { + return; + } + const std::vector mine = CopyValues(); + const std::vector theirs = other.CopyValues(); + *this = theirs; + other = mine; + } + + void Clear() { + auto hdr = Header(); + auto data = + GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); + for (size_t i = 0; i < hdr->num_elements; ++i) { + const ::toolbelt::BufferOffset offset = data[i]; + if (offset == 0) { + continue; + } + NonEmbeddedStringField(Message::GetMessage(this, source_offset_), offset) + .Clear(); } ::toolbelt::PayloadBuffer::VectorClear<::toolbelt::BufferOffset>( GetBufferAddr(), Header()); @@ -1461,6 +1922,16 @@ class StringVectorField : public Field { void clear() { Clear(); } // STL compatibility. + StringVectorField& operator=(std::initializer_list values) { + assign(values); + return *this; + } + template + StringVectorField& operator=(const std::vector& values) { + assign(values.begin(), values.end()); + return *this; + } + ::toolbelt::BufferOffset BinaryEndOffset() const { return relative_binary_offset_ + sizeof(toolbelt::VectorHeader); } @@ -1482,6 +1953,32 @@ class StringVectorField : public Field { bool operator!=(const StringVectorField& other) const { return !(*this == other); } + template + bool operator==(const std::vector& other) const { + if (size() != other.size()) { + return false; + } + for (size_t i = 0; i < size(); ++i) { + if (Get(i) != std::string_view(other[i])) { + return false; + } + } + return true; + } + template + bool operator!=(const std::vector& other) const { + return !(*this == other); + } + template + friend bool operator==(const std::vector& lhs, + const StringVectorField& rhs) { + return rhs == lhs; + } + template + friend bool operator!=(const std::vector& lhs, + const StringVectorField& rhs) { + return !(rhs == lhs); + } void Populate() const {} @@ -1530,6 +2027,15 @@ class StringVectorField : public Field { } private: + std::vector CopyValues() const { + std::vector result; + result.reserve(size()); + for (size_t i = 0; i < size(); ++i) { + result.emplace_back(Get(i)); + } + return result; + } + toolbelt::VectorHeader* Header( ::toolbelt::BufferOffset relative_offset = 0) const { if (relative_offset == 0) { diff --git a/phaser/runtime/wireformat.h b/phaser/runtime/wireformat.h index 39ee98c..dabf717 100644 --- a/phaser/runtime/wireformat.h +++ b/phaser/runtime/wireformat.h @@ -47,7 +47,7 @@ inline bool ConsumeWireVarint(absl::Span data, size_t* offset, return false; } const uint8_t byte = - static_cast(static_cast(data[(*offset)++])); + static_cast(data[(*offset)++]); if (i + 1 == max_bytes && ((byte & 0x80U) != 0 || (byte & 0x7fU) > max_last_byte)) { return false; @@ -102,7 +102,7 @@ inline bool ConsumeProtobufFields(absl::Span data, size_t* offset, value > data.size() - *offset) { return false; } - *offset += static_cast(value); + *offset += value; break; case 3: if (!ConsumeProtobufFields(data, offset, field_number, depth + 1)) { diff --git a/phaser/stress_test.cc b/phaser/stress_test.cc index 8675351..9d2e47e 100644 --- a/phaser/stress_test.cc +++ b/phaser/stress_test.cc @@ -98,7 +98,8 @@ TEST(StressTest, AllocFailsAtStart) { TEST(StressTest, CustomAllocSucceeds) { TestMessage msg = TestMessage::CreateDynamicMutable( - 512, ::phaser::test::AllocUntilLimit(64 * 1024), [](void* p) { free(p); }, + 512, ::phaser::test::AllocUntilLimit(64 * 1024), + [](void* p) noexcept { free(p); }, [](void* p, size_t, size_t new_size) -> absl::StatusOr { void* r = realloc(p, new_size); if (r == nullptr) { @@ -112,7 +113,8 @@ TEST(StressTest, CustomAllocSucceeds) { TEST(StressTest, TryCreateCustomAllocSucceeds) { auto result = TestMessage::TryCreateDynamicMutable( - 512, ::phaser::test::AllocUntilLimit(64 * 1024), [](void* p) { free(p); }, + 512, ::phaser::test::AllocUntilLimit(64 * 1024), + [](void* p) noexcept { free(p); }, [](void* p, size_t, size_t new_size) -> absl::StatusOr { void* r = realloc(p, new_size); if (r == nullptr) { @@ -128,7 +130,8 @@ TEST(StressTest, TryCreateCustomAllocSucceeds) { TEST(StressTest, TryCreateReportsInitialAllocationFailure) { auto result = TestMessage::TryCreateDynamicMutable( - 512, ::phaser::test::AllocUntilLimit(0), [](void* p) { free(p); }, + 512, ::phaser::test::AllocUntilLimit(0), + [](void* p) noexcept { free(p); }, ::phaser::test::ReallocAlwaysFails()); EXPECT_FALSE(result.ok()); EXPECT_EQ(result.status().code(), absl::StatusCode::kResourceExhausted); @@ -139,7 +142,8 @@ TEST(StressTest, ReallocFailureAborts) { { TestMessage msg = TestMessage::CreateDynamicMutable( 256, ::phaser::test::AllocUntilLimit(1024 * 1024), - [](void* p) { free(p); }, ::phaser::test::ReallocAlwaysFails()); + [](void* p) noexcept { free(p); }, + ::phaser::test::ReallocAlwaysFails()); for (int i = 0; i < 2000; i++) { msg.add_vstr(::phaser::test::MakePatternString(256, 'x')); } diff --git a/phaser/testdata/BUILD b/phaser/testdata/BUILD index 0ead64a..56db1ed 100644 --- a/phaser/testdata/BUILD +++ b/phaser/testdata/BUILD @@ -87,6 +87,19 @@ phaser_library( deps = [":ros_compile_proto"], ) +proto_library( + name = "service_only_proto", + srcs = ["ServiceOnly.proto"], + deps = ["@com_google_protobuf//:empty_proto"], +) + +phaser_library( + name = "service_only_phaser", + add_namespace = "phaser", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":service_only_proto"], +) + proto_library( name = "ros_metadata_proto", srcs = ["RosMetadata.proto"], @@ -212,7 +225,8 @@ sh_test( "//phaser:options_proto", "//phaser/compiler:phaser", "@com_google_protobuf//:protoc", - "@com_google_protobuf//:well_known_protos", + "@com_google_protobuf//:descriptor_proto_srcs", + "@com_google_protobuf//:well_known_type_protos", ], ) @@ -232,7 +246,7 @@ sh_test( "//phaser:valgrind.supp", "//phaser/compiler:phaser", "@com_google_protobuf//:protoc", - "@com_google_protobuf//:well_known_protos", + "@com_google_protobuf//:well_known_type_protos", ], ) diff --git a/phaser/testdata/RosCompile.proto b/phaser/testdata/RosCompile.proto index 43d115f..753442a 100644 --- a/phaser/testdata/RosCompile.proto +++ b/phaser/testdata/RosCompile.proto @@ -10,10 +10,33 @@ enum RosColor { ROS_COLOR_BLUE = 2; } +enum RosConstantOnly { + ROS_CONSTANT_ONLY_UNSPECIFIED = 0; + ROS_CONSTANT_ONLY_FIRST = 1; + ROS_CONSTANT_ONLY_SECOND = 2; +} + +message RosConstantOnlyHolder { + RosConstantOnly marker = 1 [(phaser.ros_field) = { + type: "example_msgs/Marker" + nested_data_type: "example_msgs/Marker" + nested_md5_text: "uint8 FIRST=1\nuint8 SECOND=2" + }]; + uint32 value = 2; +} + message RosInner { int32 id = 1; } +message RosDefinitionMetadata { + option (phaser.ros_message) = { + data_type: "example_msgs/RosDefinitionMetadata" + source_definition: "# Original comment\n\nint32 value\n" + }; + int32 value = 1; +} + message RosCompileMessage { int32 x = 1; string name = 2; @@ -49,6 +72,13 @@ message RosPackedFixedMessage { repeated fixed64 fixed_array = 7 [(phaser.array_size) = 3]; } +message RosNarrowScalars { + uint32 uint8_value = 1 [(phaser.ros_field) = { type: "uint8" }]; + int32 int8_value = 2 [(phaser.ros_field) = { type: "int8" }]; + uint32 uint16_value = 3 [(phaser.ros_field) = { type: "uint16" }]; + int32 int16_value = 4 [(phaser.ros_field) = { type: "int16" }]; +} + // These names have all been used by generated method parameters or local // variables. They must remain usable as unmodified ROS frontend field names. message GeneratorNameCollisions { diff --git a/phaser/testdata/RosMetadata.proto b/phaser/testdata/RosMetadata.proto index 37edbeb..76fc45c 100644 --- a/phaser/testdata/RosMetadata.proto +++ b/phaser/testdata/RosMetadata.proto @@ -39,3 +39,20 @@ message Wrapper { google.protobuf.Timestamp stamp = 5; repeated google.protobuf.Timestamp stamps = 6; } + +enum LegacyStatus { + option (phaser.ros_enum) = { + type: "uint8" + }; + + LEGACY_STATUS_OK = 0 [(phaser.ros_enum_value) = { name: "OK" }]; +} + +message LegacyStatusWrapper { + LegacyStatus status = 1 [(phaser.ros_field) = { + type: "example_msgs/LegacyStatus" + nested_data_type: "example_msgs/LegacyStatus" + nested_md5_text: "uint8 OK=0" + nested_source_definition: "# Legacy status.\nuint8 OK = 0\n" + }]; +} diff --git a/phaser/testdata/ServiceOnly.proto b/phaser/testdata/ServiceOnly.proto new file mode 100644 index 0000000..166c064 --- /dev/null +++ b/phaser/testdata/ServiceOnly.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package phaser.testdata; + +import "google/protobuf/empty.proto"; + +service ServiceOnly { + rpc Ping(google.protobuf.Empty) returns (google.protobuf.Empty); +} From 867d314c8b34e081a5a68e129c8b4d1febd22b56 Mon Sep 17 00:00:00 2001 From: Dave Allison Date: Fri, 28 Aug 2026 16:42:07 -0700 Subject: [PATCH 2/3] Fix build issue --- phaser/phaser_library.bzl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/phaser/phaser_library.bzl b/phaser/phaser_library.bzl index 2043035..c18aa6d 100644 --- a/phaser/phaser_library.bzl +++ b/phaser/phaser_library.bzl @@ -45,6 +45,13 @@ def _phaser_action( import_paths = {} for s in transitive_sources: for f in s.to_list(): + if f.is_source and f.short_path.startswith("../"): + # External repository sources have short paths of the form + # ..//. Add that + # repository's execroot directory so imports retain their + # canonical proto paths. + repository = f.short_path.split("/", 2)[1] + import_paths["-Iexternal/" + repository] = None if not f.is_source: index = f.path.find("_virtual_imports") if index != -1: From 3869f6d59a000d734f39b243ec23436d478c6ebf Mon Sep 17 00:00:00 2001 From: Dave Allison Date: Wed, 2 Sep 2026 12:41:21 -0700 Subject: [PATCH 3/3] Fix some warnings --- phaser/compiler/message_gen.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/phaser/compiler/message_gen.cc b/phaser/compiler/message_gen.cc index 923bfee..e15210a 100644 --- a/phaser/compiler/message_gen.cc +++ b/phaser/compiler/message_gen.cc @@ -3148,7 +3148,9 @@ void MessageGenerator::GenerateDirectProtobufField( "!status.ok()) return status;\n"; } - os << indent << " size_t ros_emitted = 0;\n"; + if (fixed_extent > 0) { + os << indent << " size_t ros_emitted = 0;\n"; + } os << indent << " ::phaser::ProtoBuffer ros_scan(protobuf);\n"; os << indent << " while (!ros_scan.Eof()) {\n"; os << indent @@ -3174,7 +3176,9 @@ void MessageGenerator::GenerateDirectProtobufField( os << emit_indent << "}\n"; } GenerateDirectROSWriteValue(os, field, "ros_value", emit_indent); - os << emit_indent << "++ros_emitted;\n"; + if (fixed_extent > 0) { + os << emit_indent << "++ros_emitted;\n"; + } }; if (field->is_packable()) { os << indent @@ -3192,9 +3196,9 @@ void MessageGenerator::GenerateDirectProtobufField( << " return absl::InvalidArgumentError(" "\"packed fixed-width field has a partial element\");\n"; os << indent << " }\n"; - os << indent << " const size_t ros_packed_count = " - << "ros_packed->size() / sizeof(" << type << ");\n"; if (fixed_extent > 0) { + os << indent << " const size_t ros_packed_count = " + << "ros_packed->size() / sizeof(" << type << ");\n"; os << indent << " if (ros_packed_count > " << fixed_extent << " - ros_emitted) {\n"; os << indent @@ -3206,7 +3210,9 @@ void MessageGenerator::GenerateDirectProtobufField( << " if (absl::Status status = output.WriteRaw(" "ros_packed->data(), ros_packed->size()); !status.ok()) " "return status;\n"; - os << indent << " ros_emitted += ros_packed_count;\n"; + if (fixed_extent > 0) { + os << indent << " ros_emitted += ros_packed_count;\n"; + } } else { os << indent << " ::phaser::ProtoBuffer ros_values(*ros_packed);\n";