diff --git a/phaser/BUILD.bazel b/phaser/BUILD.bazel index 191547f..44ed55f 100644 --- a/phaser/BUILD.bazel +++ b/phaser/BUILD.bazel @@ -113,6 +113,55 @@ cc_test( ], ) +cc_test( + name = "self_named_field_test", + srcs = ["self_named_field_test.cc"], + copts = PHASER_COPTS, + data = ["valgrind.supp"], + deps = [ + "//phaser/runtime:phaser_runtime", + "//phaser/testdata:self_named_field_phaser", + "//phaser/testdata:self_named_field_ros_phaser", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "aliased_enum_test", + srcs = ["aliased_enum_test.cc"], + copts = PHASER_COPTS, + data = ["valgrind.supp"], + deps = [ + "//phaser/runtime:phaser_runtime", + "//phaser/testdata:aliased_enum_phaser", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "cross_package_enum_test", + srcs = ["cross_package_enum_test.cc"], + copts = PHASER_COPTS, + data = ["valgrind.supp"], + deps = [ + "//phaser/runtime:phaser_runtime", + "//phaser/testdata:cross_package_enum_phaser", + "@com_google_googletest//:gtest_main", + ], +) + +cc_test( + name = "cross_package_enum_ros_test", + srcs = ["cross_package_enum_ros_test.cc"], + copts = PHASER_COPTS, + data = ["valgrind.supp"], + deps = [ + "//phaser/runtime:phaser_runtime", + "//phaser/testdata:cross_package_enum_ros_phaser", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "ros_native_frontend_compatibility_test", srcs = ["ros_native_frontend_compatibility_test.cc"], diff --git a/phaser/aliased_enum_test.cc b/phaser/aliased_enum_test.cc new file mode 100644 index 0000000..1e1b644 --- /dev/null +++ b/phaser/aliased_enum_test.cc @@ -0,0 +1,48 @@ +#include + +#include "gtest/gtest.h" +#include "phaser/testdata/AliasedEnum.phaser.h" + +namespace alias::phaser { +namespace { + +TEST(AliasedEnumTest, AliasesCompareEqualToTheirPrimaryName) { + EXPECT_EQ(SIGNAL_UNSPECIFIED, SIGNAL_IDLE); + EXPECT_EQ(SIGNAL_BUSY, SIGNAL_WORKING); +} + +// Protobuf's own _Name reports the first name declared for a number, so the +// stringizer keeps that case and drops the aliases. +TEST(AliasedEnumTest, StringizerReportsTheFirstNameForANumber) { + EXPECT_EQ("SIGNAL_UNSPECIFIED", Signal_Name(SIGNAL_IDLE)); + EXPECT_EQ("SIGNAL_BUSY", Signal_Name(SIGNAL_WORKING)); + EXPECT_EQ("SIGNAL_DONE", Signal_Name(SIGNAL_DONE)); +} + +TEST(AliasedEnumTest, ParserAcceptsEveryAlias) { + Signal parsed; + Signal_Parse("SIGNAL_IDLE", &parsed); + EXPECT_EQ(SIGNAL_IDLE, parsed); + + Signal_Parse("SIGNAL_WORKING", &parsed); + EXPECT_EQ(SIGNAL_BUSY, parsed); +} + +TEST(AliasedEnumTest, FieldsRoundTrip) { + Job job; + job.set_signal(SIGNAL_WORKING); + job.add_history(SIGNAL_IDLE); + job.add_history(SIGNAL_DONE); + + const std::string wire = job.SerializeAsString(); + + Job parsed; + ASSERT_TRUE(parsed.ParseFromString(wire)); + EXPECT_EQ(SIGNAL_BUSY, parsed.signal()); + ASSERT_EQ(2u, parsed.history_size()); + EXPECT_EQ(SIGNAL_UNSPECIFIED, parsed.history(0)); + EXPECT_EQ(SIGNAL_DONE, parsed.history(1)); +} + +} // namespace +} // namespace alias::phaser diff --git a/phaser/compiler/enum_gen.cc b/phaser/compiler/enum_gen.cc index e229ce9..d87020c 100644 --- a/phaser/compiler/enum_gen.cc +++ b/phaser/compiler/enum_gen.cc @@ -5,6 +5,7 @@ #include "phaser/compiler/enum_gen.h" #include +#include namespace phaser { @@ -24,12 +25,19 @@ void EnumGenerator::GenerateHeader(std::ostream& os) { } os << "};\n\n"; - // Stringizer + // Stringizer. With `option allow_alias`, several names share one number, so + // only the first is given a case label; a switch cannot repeat one. That + // matches protobuf's own _Name, which also reports the first name declared + // for a number. os << "struct " << name << "Stringizer {\n"; os << " std::string operator()(" << name << " e) {\n"; os << " switch (e) {\n"; + std::set stringized_numbers; for (int i = 0; i < enum_->value_count(); i++) { const google::protobuf::EnumValueDescriptor* value = enum_->value(i); + if (!stringized_numbers.insert(value->number()).second) { + continue; + } std::string const_name(value->name()); if (enum_->containing_type() != nullptr) { const_name = name + "_" + const_name; diff --git a/phaser/compiler/message_gen.cc b/phaser/compiler/message_gen.cc index 59233d7..a2221fc 100644 --- a/phaser/compiler/message_gen.cc +++ b/phaser/compiler/message_gen.cc @@ -361,12 +361,39 @@ std::string MessageGenerator::SanitizedIdentifier( return name; } +std::string MessageGenerator::GeneratedClassName() const { + std::string name(message_->name()); + if (message_->containing_type() != nullptr) { + name = std::string(message_->containing_type()->name()) + "_" + name; + } + return name; +} + +// A field is free to carry a name C++ will not take verbatim: a reserved word, +// or the name of the message holding it, which would turn the accessor into a +// constructor. Both get trailing underscores, as many as it takes to keep the +// accessor clear of the class name. protoc never meets the second case because +// it lowercases accessors, and roscpp never meets it because it names the +// struct `Foo_` and typedefs `Foo` to that. +std::string MessageGenerator::FieldAccessorName( + const std::string& proto_name) const { + std::string name = SanitizedIdentifier(proto_name); + while (name == GeneratedClassName()) { + name += "_"; + } + return name; +} + std::string MessageGenerator::MemberVariableName( const std::string& proto_name) const { + // The ROS frontend exposes the member itself, so there is no accessor for it + // to collide with. The protobuf frontend keeps the accessor at the field name + // and pushes the member one underscore past it. + std::string name = FieldAccessorName(proto_name); if (IsRosFrontend()) { - return SanitizedIdentifier(proto_name); + return name; } - return proto_name + "_"; + return name + "_"; } std::string MessageGenerator::OneofVariantTypeName( @@ -449,10 +476,20 @@ absl::Status MessageGenerator::ValidateFieldOptions() const { return status; } if (IsRosFrontend() && IsRosIntrinsic(field) && - (field->is_repeated() || field->containing_oneof() != nullptr)) { + field->containing_oneof() != nullptr) { return absl::InvalidArgumentError(absl::StrFormat( - "ROS intrinsic field %s.%s must be singular and cannot be in a " - "oneof", + "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, + // but RosHeaderField hands out a view whose lifetime is tied to the field + // rather than a value, so it has no element type to repeat. + if (IsRosFrontend() && IsRosIntrinsic(field) && field->is_repeated() && + IsRosHeader(field->message_type())) { + return absl::InvalidArgumentError(absl::StrFormat( + "ROS intrinsic field %s.%s must be singular: the ROS frontend does " + "not support a repeated Header", message_->full_name(), field->name())); } } @@ -559,7 +596,19 @@ std::string MessageGenerator::EnumName( if (desc->containing_type() != nullptr) { name = std::string(desc->containing_type()->name()) + "_" + name; } - return name; + // Enums, along with their stringizer and parser, are emitted at namespace + // scope, so the short name only resolves inside the package that declared + // them. A field referring to an enum from another package has to spell out + // that package's namespace and the added namespace, if any. + std::string enum_package(desc->file()->package()); + if (enum_package == package_name_) { + return name; + } + std::string scope = absl::StrReplaceAll(enum_package, {{".", "::"}}); + if (!added_namespace_.empty()) { + scope += "::" + added_namespace_; + } + return scope + "::" + name; } std::string MessageGenerator::MessageName( @@ -775,6 +824,9 @@ std::string MessageGenerator::FieldRepeatedVectorCType( case google::protobuf::FieldDescriptor::TYPE_BYTES: return "StringVectorField"; case google::protobuf::FieldDescriptor::TYPE_MESSAGE: + if (IsRosFrontend() && IsRosIntrinsic(field)) { + return RosIntrinsicVectorFieldType(field); + } return "MessageVectorField<" + MessageName(field->message_type(), true) + ">"; case google::protobuf::FieldDescriptor::TYPE_GROUP: @@ -834,6 +886,9 @@ std::string MessageGenerator::FieldRepeatedArrayCType( case google::protobuf::FieldDescriptor::TYPE_BYTES: return "StringArrayField<" + extent + ">"; case google::protobuf::FieldDescriptor::TYPE_MESSAGE: + if (IsRosFrontend() && IsRosIntrinsic(field)) { + return RosIntrinsicArrayFieldType(field, extent); + } return "MessageArrayField<" + MessageName(field->message_type(), true) + ", " + extent + ">"; case google::protobuf::FieldDescriptor::TYPE_GROUP: @@ -970,6 +1025,28 @@ std::string MessageGenerator::RosIntrinsicFieldType( return "RosHeaderField<" + backend + ">"; } +std::string MessageGenerator::RosIntrinsicVectorFieldType( + const google::protobuf::FieldDescriptor* field) { + const std::string backend = MessageName(field->message_type(), true); + if (IsRosTime(field->message_type())) { + return "RosTimeVectorField<" + backend + ">"; + } + // A repeated Header is rejected in ValidateFieldOptions. + assert(IsRosDuration(field->message_type())); + return "RosDurationVectorField<" + backend + ">"; +} + +std::string MessageGenerator::RosIntrinsicArrayFieldType( + const google::protobuf::FieldDescriptor* field, const std::string& extent) { + const std::string backend = MessageName(field->message_type(), true); + if (IsRosTime(field->message_type())) { + return "RosTimeArrayField<" + backend + ", " + extent + ">"; + } + // A repeated Header is rejected in ValidateFieldOptions. + assert(IsRosDuration(field->message_type())); + return "RosDurationArrayField<" + backend + ", " + extent + ">"; +} + std::string MessageGenerator::RosIntrinsicCType( const google::protobuf::FieldDescriptor* field) { if (IsRosTime(field->message_type())) { @@ -1914,8 +1991,7 @@ void MessageGenerator::GenerateFieldProtobufAccessors( std::shared_ptr field, std::shared_ptr union_field, int union_index, std::ostream& os) { std::string field_name(field->field->name()); - std::string sanitized_field_name = - field_name + +(IsCppReservedWord(field_name) ? "_" : ""); + std::string sanitized_field_name = FieldAccessorName(field_name); std::string member_name = field->member_name; if (union_field != nullptr) { @@ -3934,6 +4010,11 @@ void MessageGenerator::GenerateStreamer(std::ostream& os) { google::protobuf::FieldDescriptor::TYPE_BYTES) { os << " os << \"" << field->field->name() << ": \\\"\" << v << \"\\\"\" << std::endl;\n"; + } else if (IsRosFrontend() && IsRosIntrinsic(field->field)) { + // Like a singular message field, no colon after the name. + os << " os << \"" << field->field->name() << " \";\n"; + os << " msg." << field->member_name << ".PrintElement(os, v);\n"; + os << " os << std::endl;\n"; } else { os << " os << \"" << field->field->name() << ": \" << v << std::endl;\n"; @@ -3961,8 +4042,11 @@ void MessageGenerator::GenerateStreamer(std::ostream& os) { void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { if (decl) { - os << " template \n"; - os << " absl::Status CloneFrom(const T& _phaser_other);\n\n"; + // The template parameter carries the _phaser_ prefix for the same reason + // the locals below do: a field is free to be named T, and inside the + // template the parameter would win the name lookup. + os << " template \n"; + os << " absl::Status CloneFrom(const _phaser_Source& _phaser_other);\n\n"; os << " void CopyFrom(" "const ::phaser::Message& _phaser_other) override {\n"; os << " const " << MessageName(message_) @@ -3974,17 +4058,21 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { } // CloneFrom. - os << "template \n"; + os << "template \n"; os << "inline absl::Status " << MessageName(message_) - << "::CloneFrom([[maybe_unused]] const T& _phaser_other) {\n"; + << "::CloneFrom([[maybe_unused]] const _phaser_Source& _phaser_other) " + "{\n"; if (IsRosFrontend()) { for (auto& field : fields_) { if (field->field->is_repeated()) { os << " " << field->member_name << ".Clear();\n"; if (UsesArrayFacade(field->field)) { const int 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() == - google::protobuf::FieldDescriptor::TYPE_MESSAGE) { + google::protobuf::FieldDescriptor::TYPE_MESSAGE && + !IsRosIntrinsic(field->field)) { os << " for (size_t _phaser_index = 0; " "_phaser_index < static_cast(" << array_size << "); ++_phaser_index) {\n"; @@ -4019,7 +4107,8 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { os << " }\n"; } } else if (field->field->type() == - google::protobuf::FieldDescriptor::TYPE_MESSAGE) { + google::protobuf::FieldDescriptor::TYPE_MESSAGE && + !IsRosIntrinsic(field->field)) { os << " for (auto _phaser_value : _phaser_other." << field->member_name << ") {\n"; os << " auto _phaser_message = " << field->member_name @@ -4103,18 +4192,21 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { } } else { for (auto& field : fields_) { + // Only the bare accessor carries the sanitizing; a prefixed one such as + // `set_x` is already a legal name whatever the field is called. + const std::string name(field->field->name()); + const std::string accessor = FieldAccessorName(name); if (field->field->is_repeated()) { - os << " for (auto _phaser_value : _phaser_other." - << field->field->name() << "()) {\n"; + os << " for (auto _phaser_value : _phaser_other." << accessor + << "()) {\n"; if (field->field->type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE) { - os << " auto _phaser_message = add_" << field->field->name() - << "();\n"; + os << " auto _phaser_message = add_" << name << "();\n"; os << " if (absl::Status _phaser_status = " "_phaser_message.CloneFrom(_phaser_value); " "!_phaser_status.ok()) return _phaser_status;\n"; } else { - os << " add_" << field->field->name() << "(_phaser_value);\n"; + os << " add_" << name << "(_phaser_value);\n"; } os << " }\n"; @@ -4123,15 +4215,14 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { << ".IsPresent()) {\n"; if (field->field->type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE) { - os << " auto* _phaser_message = mutable_" << field->field->name() - << "();\n"; + os << " auto* _phaser_message = mutable_" << name << "();\n"; os << " if (absl::Status _phaser_status = " "_phaser_message->CloneFrom(_phaser_other." - << field->field->name() + << accessor << "()); !_phaser_status.ok()) return _phaser_status;\n"; } else { - os << " set_" << field->field->name() << "(_phaser_other." - << field->field->name() << "());\n"; + os << " set_" << name << "(_phaser_other." << accessor + << "());\n"; } os << " }\n"; } @@ -4145,7 +4236,7 @@ void MessageGenerator::GenerateCopy(std::ostream& os, bool decl) { os << " case " << field->field->number() << ":\n"; os << " if (absl::Status _phaser_status = " << u->member_name << ".template CloneFrom<" << i << ">(_phaser_other." - << field->field->name() + << FieldAccessorName(std::string(field->field->name())) << "()); !_phaser_status.ok()) return _phaser_status;\n"; os << " break;\n"; } diff --git a/phaser/compiler/message_gen.h b/phaser/compiler/message_gen.h index 3e74bed..5495fdd 100644 --- a/phaser/compiler/message_gen.h +++ b/phaser/compiler/message_gen.h @@ -193,6 +193,8 @@ class MessageGenerator { uint32_t FieldBinarySize(const google::protobuf::FieldDescriptor* field); std::string FieldInfoType(const google::protobuf::FieldDescriptor* field); std::string SanitizedIdentifier(const std::string& name) const; + std::string GeneratedClassName() const; + std::string FieldAccessorName(const std::string& name) const; std::string MemberVariableName(const std::string& proto_name) const; std::string OneofVariantTypeName( const google::protobuf::OneofDescriptor* oneof) const; @@ -206,6 +208,11 @@ class MessageGenerator { bool IsRosIntrinsic(const google::protobuf::FieldDescriptor* field) const; std::string RosIntrinsicFieldType( const google::protobuf::FieldDescriptor* field); + std::string RosIntrinsicVectorFieldType( + const google::protobuf::FieldDescriptor* field); + std::string RosIntrinsicArrayFieldType( + const google::protobuf::FieldDescriptor* field, + const std::string& extent); std::string RosIntrinsicCType( const google::protobuf::FieldDescriptor* field); absl::Status ValidateFieldOptions() const; diff --git a/phaser/cross_package_enum_ros_test.cc b/phaser/cross_package_enum_ros_test.cc new file mode 100644 index 0000000..684886c --- /dev/null +++ b/phaser/cross_package_enum_ros_test.cc @@ -0,0 +1,67 @@ +#include + +#include "gtest/gtest.h" +#include "phaser/testdata/cross_package_enum_ros_phaser/phaser/testdata/CrossPackageEnumUser.phaser.h" + +namespace canvas::phaser_ros { +namespace { + +using ::palette::phaser_ros::Color; +using ::palette::phaser_ros::COLOR_BLUE; +using ::palette::phaser_ros::COLOR_GREEN; +using ::palette::phaser_ros::COLOR_RED; +using ::palette::phaser_ros::Shade_Depth_DEPTH_DARK; + +TEST(CrossPackageEnumRosTest, SingularFieldRoundTrips) { + Drawing drawing; + drawing.background = COLOR_BLUE; + EXPECT_EQ(COLOR_BLUE, drawing.background); +} + +TEST(CrossPackageEnumRosTest, NestedEnumFieldRoundTrips) { + Drawing drawing; + drawing.depth = Shade_Depth_DEPTH_DARK; + EXPECT_EQ(Shade_Depth_DEPTH_DARK, drawing.depth); +} + +TEST(CrossPackageEnumRosTest, RepeatedFieldRoundTrips) { + Drawing drawing; + drawing.strokes.push_back(COLOR_RED); + drawing.strokes.push_back(COLOR_GREEN); + ASSERT_EQ(2u, drawing.strokes.size()); + EXPECT_EQ(COLOR_RED, drawing.strokes[0]); + EXPECT_EQ(COLOR_GREEN, drawing.strokes[1]); +} + +TEST(CrossPackageEnumRosTest, FixedArrayFieldRoundTrips) { + Drawing drawing; + drawing.corners[0] = COLOR_GREEN; + drawing.corners[3] = COLOR_BLUE; + EXPECT_EQ(COLOR_GREEN, drawing.corners[0]); + EXPECT_EQ(COLOR_BLUE, drawing.corners[3]); +} + +TEST(CrossPackageEnumRosTest, StringizerAndParserResolveAcrossPackages) { + EXPECT_EQ("COLOR_GREEN", ::palette::phaser_ros::Color_Name(COLOR_GREEN)); + + Color parsed; + ::palette::phaser_ros::Color_Parse("COLOR_BLUE", &parsed); + EXPECT_EQ(COLOR_BLUE, parsed); +} + +TEST(CrossPackageEnumRosTest, SerializesAndParses) { + Drawing drawing; + drawing.background = COLOR_BLUE; + drawing.strokes.push_back(COLOR_RED); + + const std::string wire = drawing.SerializeAsString(); + + Drawing parsed; + ASSERT_TRUE(parsed.ParseFromString(wire)); + EXPECT_EQ(COLOR_BLUE, parsed.background); + ASSERT_EQ(1u, parsed.strokes.size()); + EXPECT_EQ(COLOR_RED, parsed.strokes[0]); +} + +} // namespace +} // namespace canvas::phaser_ros diff --git a/phaser/cross_package_enum_test.cc b/phaser/cross_package_enum_test.cc new file mode 100644 index 0000000..823d76a --- /dev/null +++ b/phaser/cross_package_enum_test.cc @@ -0,0 +1,86 @@ +#include + +#include "gtest/gtest.h" +#include "phaser/testdata/CrossPackageEnumUser.phaser.h" + +namespace canvas::phaser { +namespace { + +using ::palette::phaser::Color; +using ::palette::phaser::COLOR_BLUE; +using ::palette::phaser::COLOR_GREEN; +using ::palette::phaser::COLOR_RED; +using ::palette::phaser::Shade_Depth; +using ::palette::phaser::Shade_Depth_DEPTH_DARK; + +TEST(CrossPackageEnumTest, SingularFieldRoundTrips) { + Drawing drawing; + drawing.set_background(COLOR_BLUE); + EXPECT_EQ(COLOR_BLUE, drawing.background()); +} + +TEST(CrossPackageEnumTest, NestedEnumFieldRoundTrips) { + Drawing drawing; + drawing.set_depth(Shade_Depth_DEPTH_DARK); + EXPECT_EQ(Shade_Depth_DEPTH_DARK, drawing.depth()); +} + +TEST(CrossPackageEnumTest, RepeatedFieldRoundTrips) { + Drawing drawing; + drawing.add_strokes(COLOR_RED); + drawing.add_strokes(COLOR_GREEN); + ASSERT_EQ(2u, drawing.strokes_size()); + EXPECT_EQ(COLOR_RED, drawing.strokes(0)); + EXPECT_EQ(COLOR_GREEN, drawing.strokes(1)); +} + +// The protobuf frontend ignores (phaser.array_size) and keeps the field a +// vector, so size it before indexing into it. +TEST(CrossPackageEnumTest, SizedArrayFieldRoundTrips) { + Drawing drawing; + drawing.resize_corners(4); + drawing.set_corners(0, COLOR_GREEN); + drawing.set_corners(3, COLOR_BLUE); + EXPECT_EQ(COLOR_GREEN, drawing.corners(0)); + EXPECT_EQ(COLOR_BLUE, drawing.corners(3)); +} + +TEST(CrossPackageEnumTest, UnionFieldRoundTrips) { + Drawing drawing; + drawing.set_accent_color(COLOR_BLUE); + EXPECT_EQ(COLOR_BLUE, drawing.accent_color()); + EXPECT_TRUE(drawing.has_accent_color()); +} + +TEST(CrossPackageEnumTest, StringizerAndParserResolveAcrossPackages) { + EXPECT_EQ("COLOR_GREEN", ::palette::phaser::Color_Name(COLOR_GREEN)); + + Color parsed; + ::palette::phaser::Color_Parse("COLOR_BLUE", &parsed); + EXPECT_EQ(COLOR_BLUE, parsed); +} + +TEST(CrossPackageEnumTest, DebugStringNamesTheEnumValue) { + Drawing drawing; + drawing.set_background(COLOR_GREEN); + EXPECT_NE(std::string::npos, drawing.DebugString().find("COLOR_GREEN")); +} + +TEST(CrossPackageEnumTest, SerializesAndParses) { + Drawing drawing; + drawing.set_background(COLOR_BLUE); + drawing.add_strokes(COLOR_RED); + drawing.set_depth(Shade_Depth_DEPTH_DARK); + + const std::string wire = drawing.SerializeAsString(); + + Drawing parsed; + ASSERT_TRUE(parsed.ParseFromString(wire)); + EXPECT_EQ(COLOR_BLUE, parsed.background()); + ASSERT_EQ(1u, parsed.strokes_size()); + EXPECT_EQ(COLOR_RED, parsed.strokes(0)); + EXPECT_EQ(Shade_Depth_DEPTH_DARK, parsed.depth()); +} + +} // namespace +} // namespace canvas::phaser diff --git a/phaser/ros_intrinsics_test.cc b/phaser/ros_intrinsics_test.cc index ab7c75f..fe98490 100644 --- a/phaser/ros_intrinsics_test.cc +++ b/phaser/ros_intrinsics_test.cc @@ -108,5 +108,101 @@ TEST(RosIntrinsicsTest, CopyAndMovePreserveDeferredValues) { EXPECT_EQ(ReadFrame(moved.header), "map"); } +void FillRepeated(RosIntrinsicMessage& message) { + message.stamps.Add(::ros::Time(1, 2)); + message.stamps.Add(::ros::Time(3, 4)); + message.timeouts.Add(::ros::Duration(-5, 6)); + message.fixed_stamps.Set(0, ::ros::Time(7, 8)); + message.fixed_stamps.Set(1, ::ros::Time(9, 10)); + message.fixed_timeouts.Set(0, ::ros::Duration(-11, 12)); + message.fixed_timeouts.Set(1, ::ros::Duration(13, 14)); +} + +void ExpectRepeated(const RosIntrinsicMessage& message) { + ASSERT_EQ(message.stamps.size(), 2u); + EXPECT_EQ(message.stamps.Get(0), ::ros::Time(1, 2)); + EXPECT_EQ(message.stamps.Get(1), ::ros::Time(3, 4)); + ASSERT_EQ(message.timeouts.size(), 1u); + EXPECT_EQ(message.timeouts.Get(0), ::ros::Duration(-5, 6)); + ASSERT_EQ(message.fixed_stamps.size(), 2u); + EXPECT_EQ(message.fixed_stamps.Get(0), ::ros::Time(7, 8)); + EXPECT_EQ(message.fixed_stamps.Get(1), ::ros::Time(9, 10)); + ASSERT_EQ(message.fixed_timeouts.size(), 2u); + EXPECT_EQ(message.fixed_timeouts.Get(0), ::ros::Duration(-11, 12)); + EXPECT_EQ(message.fixed_timeouts.Get(1), ::ros::Duration(13, 14)); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicsReadAsRosValues) { + RosIntrinsicMessage message; + FillRepeated(message); + ExpectRepeated(message); + + // The element type is the ROS value, not the Timestamp backing it. + std::vector<::ros::Time> collected; + for (::ros::Time value : message.stamps) { + collected.push_back(value); + } + EXPECT_EQ(collected, + (std::vector<::ros::Time>{::ros::Time(1, 2), ::ros::Time(3, 4)})); + EXPECT_EQ(ReadSecondsByValue(message.stamps[1]), 3u); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicElementsAreAssignable) { + RosIntrinsicMessage message; + FillRepeated(message); + + message.stamps[0] = ::ros::Time(100, 200); + message.fixed_timeouts[1] = ::ros::Duration(-300, 400); + + EXPECT_EQ(message.stamps.Get(0), ::ros::Time(100, 200)); + EXPECT_EQ(message.stamps.Get(1), ::ros::Time(3, 4)); + EXPECT_EQ(message.fixed_timeouts.Get(1), ::ros::Duration(-300, 400)); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicsSurviveProtobufWireRoundtrip) { + RosIntrinsicMessage message; + FillRepeated(message); + + RosIntrinsicMessage parsed; + ASSERT_TRUE(parsed.ParseFromString(message.SerializeAsString())); + ExpectRepeated(parsed); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicsSurviveRosWireRoundtrip) { + RosIntrinsicMessage message; + FillRepeated(message); + + std::string ros_wire; + ASSERT_TRUE(message.SerializeToROSString(&ros_wire).ok()); + EXPECT_EQ(message.ROSSerializedSize(), ros_wire.size()); + + RosIntrinsicMessage parsed; + ASSERT_TRUE( + parsed.ParseFromROS(absl::Span(ros_wire.data(), + ros_wire.size())) + .ok()); + ExpectRepeated(parsed); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicsUseEightRosBytesPerElement) { + RosIntrinsicMessage empty; + RosIntrinsicMessage populated; + populated.stamps.Add(::ros::Time(1, 2)); + populated.stamps.Add(::ros::Time(3, 4)); + populated.timeouts.Add(::ros::Duration(-5, 6)); + + // A fixed extent is always serialized in full, so only the unbounded fields + // move: sec and nsec, four bytes each, with the sequence length unchanged. + EXPECT_EQ(populated.ROSSerializedSize(), empty.ROSSerializedSize() + 3 * 8); +} + +TEST(RosIntrinsicsTest, RepeatedIntrinsicsClone) { + RosIntrinsicMessage source; + FillRepeated(source); + + RosIntrinsicMessage copy(source); + ExpectRepeated(copy); +} + } // namespace } // namespace foo::bar::phaser diff --git a/phaser/ros_metadata_protobuf_frontend_test.cc b/phaser/ros_metadata_protobuf_frontend_test.cc index 9fe2a29..265d40b 100644 --- a/phaser/ros_metadata_protobuf_frontend_test.cc +++ b/phaser/ros_metadata_protobuf_frontend_test.cc @@ -24,6 +24,7 @@ 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" "==================================================================" "==============\n" "MSG: example_msgs/Bool\n" @@ -37,7 +38,8 @@ TEST(RosMetadataProtobufFrontendTest, MatchesRosMetadata) { ::phaser::Md5("uint8 READY=1\n" "8b94c1b53db61fb6aed406028ad6332a child\n" "int32[3] samples\n" + - Status::RosMd5() + " status\nbool ready\ntime stamp")); + Status::RosMd5() + + " status\nbool ready\ntime stamp\ntime[] stamps")); } } // namespace diff --git a/phaser/ros_metadata_ros_frontend_test.cc b/phaser/ros_metadata_ros_frontend_test.cc index 27b5549..0e28e85 100644 --- a/phaser/ros_metadata_ros_frontend_test.cc +++ b/phaser/ros_metadata_ros_frontend_test.cc @@ -26,7 +26,8 @@ TEST(RosMetadataRosFrontendTest, MatchesRosMetadata) { ::phaser::Md5("uint8 READY=1\n" "8b94c1b53db61fb6aed406028ad6332a child\n" "int32[3] samples\n" + - Status::RosMd5() + " status\nbool ready\ntime stamp")); + Status::RosMd5() + + " status\nbool ready\ntime stamp\ntime[] stamps")); } } // namespace diff --git a/phaser/ros_wire_conversion_test.cc b/phaser/ros_wire_conversion_test.cc index 7ff9df0..4ecfb95 100644 --- a/phaser/ros_wire_conversion_test.cc +++ b/phaser/ros_wire_conversion_test.cc @@ -189,6 +189,25 @@ std::string ExpectedIntrinsicBytes() { AppendString(bytes, ""); // child label } AppendIntegral(bytes, static_cast(0)); // choice unset + + // Repeated ROS intrinsics: a sequence length only when the extent is not + // fixed, then sec and nsec per element, exactly as a singular one. + AppendIntegral(bytes, static_cast(2)); // stamps count + AppendIntegral(bytes, static_cast(1)); + AppendIntegral(bytes, static_cast(2)); + 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, 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)); return bytes; } @@ -359,6 +378,13 @@ TEST(ROSWireConversionTest, ROS1IntrinsicsUseNativeLayoutsAndFlushCaches) { message.header->seq = 9; message.header->stamp = ::ros::Time(21, 654); message.header->frame_id = "map"; + message.stamps.Add(::ros::Time(1, 2)); + message.stamps.Add(::ros::Time(3, 4)); + message.timeouts.Add(::ros::Duration(-5, 6)); + message.fixed_stamps.Set(0, ::ros::Time(7, 8)); + message.fixed_stamps.Set(1, ::ros::Time(9, 10)); + message.fixed_timeouts.Set(0, ::ros::Duration(-11, 12)); + message.fixed_timeouts.Set(1, ::ros::Duration(13, 14)); const std::string expected = ExpectedIntrinsicBytes(); ::phaser::ROSBuffer live_output; @@ -457,6 +483,13 @@ TEST(ROSWireConversionTest, ParsedROSPayloadUsesEitherFrontend) { EXPECT_EQ(ros_message.header->stamp.nsec, 654u); EXPECT_EQ(ros_message.header->frame_id, "map"); EXPECT_EQ(ros_message.choice.index(), std::variant_npos); + ASSERT_EQ(ros_message.stamps.size(), 2u); + EXPECT_EQ(ros_message.stamps.Get(0), ::ros::Time(1, 2)); + EXPECT_EQ(ros_message.stamps.Get(1), ::ros::Time(3, 4)); + ASSERT_EQ(ros_message.timeouts.size(), 1u); + EXPECT_EQ(ros_message.timeouts.Get(0), ::ros::Duration(-5, 6)); + EXPECT_EQ(ros_message.fixed_stamps.Get(1), ::ros::Time(9, 10)); + EXPECT_EQ(ros_message.fixed_timeouts.Get(0), ::ros::Duration(-11, 12)); const size_t native_size = ros_message.Size(); std::vector native_payload(native_size); @@ -473,6 +506,15 @@ TEST(ROSWireConversionTest, ParsedROSPayloadUsesEitherFrontend) { EXPECT_FALSE(protobuf_view.has_choice_number()); EXPECT_FALSE(protobuf_view.has_choice_text()); EXPECT_FALSE(protobuf_view.has_choice_child()); + // The ROS frontend wrote these as ROS values; the protobuf frontend reads + // the same payload back as ordinary repeated Timestamp and Duration. + ASSERT_EQ(protobuf_view.stamps_size(), 2); + EXPECT_EQ(protobuf_view.stamps(1).seconds(), 3); + EXPECT_EQ(protobuf_view.stamps(1).nanos(), 4); + ASSERT_EQ(protobuf_view.timeouts_size(), 1); + EXPECT_EQ(protobuf_view.timeouts(0).seconds(), -5); + EXPECT_EQ(protobuf_view.fixed_stamps(0).seconds(), 7); + EXPECT_EQ(protobuf_view.fixed_timeouts(1).nanos(), 14); ProtobufFrontendIntrinsicMessage parsed_protobuf_frontend; ASSERT_TRUE(parsed_protobuf_frontend diff --git a/phaser/runtime/ros.h b/phaser/runtime/ros.h index 506d3cc..b7cfe82 100644 --- a/phaser/runtime/ros.h +++ b/phaser/runtime/ros.h @@ -11,13 +11,18 @@ #include #include +#include #include +#include #include #include #include #include +#include +#include "phaser/runtime/arrays.h" #include "phaser/runtime/fields.h" +#include "phaser/runtime/vectors.h" namespace phaser { @@ -247,6 +252,210 @@ template using RosDurationField = RosMessageField; +// Presents a repeated Phaser message field as a sequence of ROS values, so a +// ROS `time[]` or `duration[]` reads and writes as `::ros::Time` rather than as +// the google.protobuf.Timestamp backing it. `Sequence` is +// MessageVectorField for an unbounded field or +// MessageArrayField for a fixed extent. +// +// Unlike the singular RosMessageField there is no cache: every element access +// converts through `Traits` against the payload, so a mutation is visible to +// the backing message immediately and SyncToPayload has nothing to reconcile. +template +class RosRepeatedMessageField : public Sequence { + public: + using RosType = typename Traits::RosType; + using Sequence::Sequence; + + using value_type = RosType; + using reference = RosType; + using const_reference = RosType; + using size_type = size_t; + using difference_type = ptrdiff_t; + + // Lets `field[i] = value` work without handing out a reference into a + // payload the element does not own. + class Proxy { + public: + Proxy(RosRepeatedMessageField* field, size_t index) + : field_(field), index_(index) {} + + operator RosType() const { return field_->Get(index_); } + RosType Get() const { return field_->Get(index_); } + + Proxy& operator=(const RosType& value) { + field_->Set(index_, value); + return *this; + } + Proxy& operator=(const Proxy& other) { + if (this != &other) { + field_->Set(index_, other.Get()); + } + return *this; + } + + friend bool operator==(const Proxy& lhs, const RosType& rhs) { + return lhs.Get() == rhs; + } + friend bool operator==(const RosType& lhs, const Proxy& rhs) { + return lhs == rhs.Get(); + } + friend bool operator!=(const Proxy& lhs, const RosType& rhs) { + return !(lhs == rhs); + } + friend bool operator!=(const RosType& lhs, const Proxy& rhs) { + return !(lhs == rhs); + } + friend std::ostream& operator<<(std::ostream& os, const Proxy& proxy) { + Traits::Print(os, proxy.Get()); + return os; + } + + private: + RosRepeatedMessageField* field_; + size_t index_; + }; + + class const_iterator { + public: + using iterator_category = std::bidirectional_iterator_tag; + using value_type = RosType; + using difference_type = ptrdiff_t; + using pointer = void; + using reference = RosType; + + const_iterator() = default; + const_iterator(const RosRepeatedMessageField* field, size_t index) + : field_(field), index_(index) {} + + RosType operator*() const { return field_->Get(index_); } + const_iterator& operator++() { + ++index_; + return *this; + } + const_iterator operator++(int) { + const_iterator result = *this; + ++*this; + return result; + } + const_iterator& operator--() { + --index_; + return *this; + } + const_iterator operator--(int) { + const_iterator result = *this; + --*this; + return result; + } + bool operator==(const const_iterator& other) const { + return field_ == other.field_ && index_ == other.index_; + } + bool operator!=(const const_iterator& other) const { + return !(*this == other); + } + + private: + const RosRepeatedMessageField* field_ = nullptr; + size_t index_ = 0; + }; + using iterator = const_iterator; + using reverse_iterator = std::reverse_iterator; + using const_reverse_iterator = std::reverse_iterator; + + const_iterator begin() const { return const_iterator(this, 0); } + const_iterator end() const { return const_iterator(this, 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(); } + + // An out-of-range index needs no guard here: the sequence hands back a + // default-constructed backend for one, which loads as a zero ROS value. + RosType Get(size_t index) const { + RosType value; + Traits::Load(Sequence::Get(index), value); + return value; + } + + void Set(size_t index, const RosType& value) { + auto backend = Sequence::Mutable(index); + Traits::Store(value, backend); + } + + void Add(const RosType& value) { + auto backend = Sequence::Add(); + Traits::Store(value, backend); + } + + void push_back(const RosType& value) { Add(value); } + + RosType operator[](size_t index) const { return Get(index); } + Proxy operator[](size_t index) { return Proxy(this, index); } + + RosType front() const { return Get(0); } + Proxy front() { return Proxy(this, 0); } + RosType back() const { return Get(this->size() - 1); } + Proxy back() { return Proxy(this, this->size() - 1); } + + std::vector Get() const { + std::vector result; + result.reserve(this->size()); + for (size_t i = 0; i < this->size(); ++i) { + result.push_back(Get(i)); + } + return result; + } + + // Formats one element the way the singular field formats its value. Without + // this a repeated element would stream through ROS's own operator<<, which + // prints a different shape than the rest of the message. + static std::ostream& PrintElement(std::ostream& os, const RosType& value) { + Traits::Print(os, value); + return os; + } + + bool operator==(const RosRepeatedMessageField& other) const { + if (this->size() != other.size()) { + return false; + } + for (size_t i = 0; i < this->size(); ++i) { + if (!(Get(i) == other.Get(i))) { + return false; + } + } + return true; + } + bool operator!=(const RosRepeatedMessageField& other) const { + return !(*this == other); + } +}; + +template +using RosTimeVectorField = + RosRepeatedMessageField, + internal::RosTimeTraits>; + +template +using RosDurationVectorField = + RosRepeatedMessageField, + internal::RosDurationTraits>; + +template +using RosTimeArrayField = + RosRepeatedMessageField, + internal::RosTimeTraits>; + +template +using RosDurationArrayField = + RosRepeatedMessageField, + internal::RosDurationTraits>; + template class RosHeaderMutableView { public: diff --git a/phaser/self_named_field_test.cc b/phaser/self_named_field_test.cc new file mode 100644 index 0000000..9bfab65 --- /dev/null +++ b/phaser/self_named_field_test.cc @@ -0,0 +1,99 @@ +#include + +#include "gtest/gtest.h" +#include "phaser/testdata/SelfNamedField.phaser.h" +#include "phaser/testdata/self_named_field_ros_phaser/phaser/testdata/SelfNamedField.phaser.h" + +namespace selfnamed { +namespace { + +// The protobuf frontend normally puts the accessor at the field name and the +// member one underscore past it. A self-named field pushes both along by one so +// the accessor stops being a constructor. +TEST(SelfNamedFieldTest, ProtobufFrontendShiftsTheAccessorPastTheClassName) { + phaser::Polygon polygon; + phaser::Point point = polygon.add_Polygon(); + point.set_x(1.5); + point.set_y(-2.5); + + ASSERT_EQ(1u, polygon.Polygon_size()); + EXPECT_EQ(1.5, polygon.Polygon_(0).x()); + EXPECT_EQ(-2.5, polygon.Polygon_(0).y()); +} + +TEST(SelfNamedFieldTest, ProtobufFrontendHandlesScalarSelfNamedField) { + phaser::Label label; + label.set_Label("stop"); + label.set_index(3); + EXPECT_EQ("stop", label.Label_()); + EXPECT_EQ(3, label.index()); +} + +// The ROS frontend exposes the member itself, so only the member has to move. +TEST(SelfNamedFieldTest, RosFrontendSuffixesTheCollidingMember) { + phaser_ros::Polygon polygon; + auto point = polygon.Polygon_.Add(); + point.x = 4.0; + point.y = 5.0; + + ASSERT_EQ(1u, polygon.Polygon_.size()); + EXPECT_EQ(4.0, polygon.Polygon_[0].x); + EXPECT_EQ(5.0, polygon.Polygon_[0].y); +} + +TEST(SelfNamedFieldTest, RosFrontendLeavesOtherMembersAlone) { + phaser_ros::Label label; + label.Label_ = "go"; + label.index = 7; + EXPECT_EQ("go", label.Label_.Get()); + EXPECT_EQ(7, label.index); +} + +// Nested messages generate as `Outer_Inner`, so that is the name a member has +// to stay clear of, not the proto-level `Inner`. +TEST(SelfNamedFieldTest, RosFrontendUsesTheFlattenedNestedClassName) { + phaser_ros::Outer_Inner inner; + inner.Outer_Inner_ = 11; + inner.keep = 12; + EXPECT_EQ(11, inner.Outer_Inner_); + EXPECT_EQ(12, inner.keep); +} + +TEST(SelfNamedFieldTest, SelfNamedFieldSurvivesAWireRoundtrip) { + phaser_ros::Label label; + label.Label_ = "yield"; + label.index = 2; + + const std::string wire = label.SerializeAsString(); + + phaser::Label parsed; + ASSERT_TRUE(parsed.ParseFromString(wire)); + EXPECT_EQ("yield", parsed.Label_()); + EXPECT_EQ(2, parsed.index()); +} + +TEST(SelfNamedFieldTest, FieldNamedTDoesNotCollideWithCloneFromsTemplate) { + phaser_ros::Horizon source; + source.T = 20; + source.steps.push_back(0.5); + + phaser_ros::Horizon copy; + ASSERT_TRUE(copy.CloneFrom(source).ok()); + EXPECT_EQ(20, copy.T); + ASSERT_EQ(1u, copy.steps.size()); + EXPECT_EQ(0.5, copy.steps[0]); +} + +TEST(SelfNamedFieldTest, CloneFromCopiesASelfNamedField) { + phaser::Label source; + source.set_Label("merge"); + source.set_index(5); + + phaser::Label copy; + ASSERT_TRUE(copy.CloneFrom(source).ok()); + EXPECT_EQ("merge", copy.Label_()); + EXPECT_EQ(5, copy.index()); +} + +} // namespace +} // namespace selfnamed diff --git a/phaser/testdata/AliasedEnum.proto b/phaser/testdata/AliasedEnum.proto new file mode 100644 index 0000000..9eef8c3 --- /dev/null +++ b/phaser/testdata/AliasedEnum.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package alias; + +// `allow_alias` lets several names share one number. Protobuf accepts it, so +// the generated stringizer has to pick one name per number rather than emit a +// switch case for each. +enum Signal { + option allow_alias = true; + + SIGNAL_UNSPECIFIED = 0; + SIGNAL_IDLE = 0; + SIGNAL_BUSY = 1; + SIGNAL_WORKING = 1; + SIGNAL_DONE = 2; +} + +message Job { + Signal signal = 1; + repeated Signal history = 2; +} diff --git a/phaser/testdata/BUILD b/phaser/testdata/BUILD index adc59dd..0ead64a 100644 --- a/phaser/testdata/BUILD +++ b/phaser/testdata/BUILD @@ -223,9 +223,12 @@ sh_test( "$(rootpath @com_google_protobuf//:protoc)", "$(rootpath //phaser/compiler:phaser)", "$(rootpath InvalidRosIntrinsic.proto)", + "$(rootpath InvalidRosIntrinsicOneof.proto)", ], data = [ "InvalidRosIntrinsic.proto", + "InvalidRosIntrinsicOneof.proto", + "RosHeader.proto", "//phaser:valgrind.supp", "//phaser/compiler:phaser", "@com_google_protobuf//:protoc", @@ -233,6 +236,72 @@ sh_test( ], ) +proto_library( + name = "self_named_field_proto", + srcs = ["SelfNamedField.proto"], +) + +phaser_library( + name = "self_named_field_phaser", + add_namespace = "phaser", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":self_named_field_proto"], +) + +phaser_library( + name = "self_named_field_ros_phaser", + add_namespace = "phaser_ros", + cc_deps = [":ros1_shim"], + direct_header_symlinks = False, + frontend = "ros", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":self_named_field_proto"], +) + +proto_library( + name = "aliased_enum_proto", + srcs = ["AliasedEnum.proto"], +) + +phaser_library( + name = "aliased_enum_phaser", + add_namespace = "phaser", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":aliased_enum_proto"], +) + +proto_library( + name = "cross_package_enum_proto", + srcs = ["CrossPackageEnum.proto"], + deps = ["//phaser:options_proto"], +) + +proto_library( + name = "cross_package_enum_user_proto", + srcs = ["CrossPackageEnumUser.proto"], + deps = [ + ":cross_package_enum_proto", + "//phaser:options_proto", + ], +) + +phaser_library( + name = "cross_package_enum_phaser", + add_namespace = "phaser", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":cross_package_enum_user_proto"], +) + +phaser_library( + name = "cross_package_enum_ros_phaser", + add_namespace = "phaser_ros", + cc_deps = [":ros1_shim"], + direct_header_symlinks = False, + frontend = "ros", + runtime = "//phaser/runtime:phaser_runtime", + deps = [":cross_package_enum_user_proto"], +) + proto_library( name = "coverage_proto", srcs = ["coverage.proto"], @@ -272,4 +341,4 @@ phaser_library( add_namespace = "phaser", runtime = "//phaser/runtime:phaser_runtime", deps = [":vision_proto"], -) +) \ No newline at end of file diff --git a/phaser/testdata/CrossPackageEnum.proto b/phaser/testdata/CrossPackageEnum.proto new file mode 100644 index 0000000..15c1336 --- /dev/null +++ b/phaser/testdata/CrossPackageEnum.proto @@ -0,0 +1,27 @@ +syntax = "proto3"; + +package palette; + +import "phaser/options.proto"; + +enum Color { + COLOR_RED = 0; + COLOR_GREEN = 1; + COLOR_BLUE = 2; +} + +message Shade { + enum Depth { + DEPTH_LIGHT = 0; + DEPTH_DARK = 1; + } + + Depth depth = 1; +} + +message Palette { + Color primary = 1; + repeated Color swatches = 2; + repeated Color triad = 3 [(phaser.array_size) = 3]; + Shade.Depth depth = 4; +} diff --git a/phaser/testdata/CrossPackageEnumUser.proto b/phaser/testdata/CrossPackageEnumUser.proto new file mode 100644 index 0000000..0cef233 --- /dev/null +++ b/phaser/testdata/CrossPackageEnumUser.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +package canvas; + +import "phaser/options.proto"; +import "phaser/testdata/CrossPackageEnum.proto"; + +// Every enum field here refers to an enum declared in another proto package, so +// the generated code has to name the enum, its stringizer, and its parser with +// that package's namespace rather than the short name. +message Drawing { + palette.Color background = 1; + repeated palette.Color strokes = 2; + repeated palette.Color corners = 3 [(phaser.array_size) = 4]; + palette.Shade.Depth depth = 4; + palette.Palette palette = 5; + + oneof accent { + palette.Color accent_color = 6; + int32 accent_index = 7; + } +} diff --git a/phaser/testdata/InvalidRosIntrinsic.proto b/phaser/testdata/InvalidRosIntrinsic.proto index a44740c..66050ba 100644 --- a/phaser/testdata/InvalidRosIntrinsic.proto +++ b/phaser/testdata/InvalidRosIntrinsic.proto @@ -2,8 +2,8 @@ syntax = "proto3"; package invalid; -import "google/protobuf/timestamp.proto"; +import "phaser/testdata/RosHeader.proto"; message InvalidRosIntrinsic { - repeated google.protobuf.Timestamp stamps = 1; + repeated std_msgs.Header headers = 1; } diff --git a/phaser/testdata/InvalidRosIntrinsicOneof.proto b/phaser/testdata/InvalidRosIntrinsicOneof.proto new file mode 100644 index 0000000..b1cf4a8 --- /dev/null +++ b/phaser/testdata/InvalidRosIntrinsicOneof.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package invalid; + +import "google/protobuf/timestamp.proto"; + +message InvalidRosIntrinsicOneof { + oneof choice { + google.protobuf.Timestamp stamp = 1; + int32 count = 2; + } +} diff --git a/phaser/testdata/RosIntrinsics.proto b/phaser/testdata/RosIntrinsics.proto index 5ef6d44..154d5d4 100644 --- a/phaser/testdata/RosIntrinsics.proto +++ b/phaser/testdata/RosIntrinsics.proto @@ -29,4 +29,11 @@ message RosIntrinsicMessage { string choice_text = 12; WireChild choice_child = 13; } + + repeated google.protobuf.Timestamp stamps = 14; + repeated google.protobuf.Duration timeouts = 15; + repeated google.protobuf.Timestamp fixed_stamps = 16 + [(phaser.array_size) = 2]; + repeated google.protobuf.Duration fixed_timeouts = 17 + [(phaser.array_size) = 2]; } diff --git a/phaser/testdata/RosIntrinsicsProtobufFrontend.proto b/phaser/testdata/RosIntrinsicsProtobufFrontend.proto index a2dd101..76a7693 100644 --- a/phaser/testdata/RosIntrinsicsProtobufFrontend.proto +++ b/phaser/testdata/RosIntrinsicsProtobufFrontend.proto @@ -29,4 +29,11 @@ message RosIntrinsicMessage { string choice_text = 12; WireChild choice_child = 13; } + + repeated google.protobuf.Timestamp stamps = 14; + repeated google.protobuf.Duration timeouts = 15; + repeated google.protobuf.Timestamp fixed_stamps = 16 + [(phaser.array_size) = 2]; + repeated google.protobuf.Duration fixed_timeouts = 17 + [(phaser.array_size) = 2]; } diff --git a/phaser/testdata/RosMetadata.proto b/phaser/testdata/RosMetadata.proto index cc62ec2..37edbeb 100644 --- a/phaser/testdata/RosMetadata.proto +++ b/phaser/testdata/RosMetadata.proto @@ -37,4 +37,5 @@ message Wrapper { Status status = 3; bool ready = 4; google.protobuf.Timestamp stamp = 5; + repeated google.protobuf.Timestamp stamps = 6; } diff --git a/phaser/testdata/SelfNamedField.proto b/phaser/testdata/SelfNamedField.proto new file mode 100644 index 0000000..4aaa638 --- /dev/null +++ b/phaser/testdata/SelfNamedField.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package selfnamed; + +message Point { + double x = 1; + double y = 2; +} + +// A ROS message is free to give a field the same name as the message itself, +// which C++ will not accept as a member of the generated class. +message Polygon { + repeated Point Polygon = 1; +} + +message Label { + string Label = 1; + int32 index = 2; +} + +message Outer { + message Inner { + int32 Outer_Inner = 1; + int32 keep = 2; + } + + Inner inner = 1; +} + +// `T` is a plausible .msg field name and also the obvious name for a template +// parameter on the generated CloneFrom. +message Horizon { + int32 T = 1; + repeated double steps = 2; +} diff --git a/phaser/testdata/invalid_ros_intrinsic_test.sh b/phaser/testdata/invalid_ros_intrinsic_test.sh index bf2a8e1..d771e85 100755 --- a/phaser/testdata/invalid_ros_intrinsic_test.sh +++ b/phaser/testdata/invalid_ros_intrinsic_test.sh @@ -3,21 +3,34 @@ set -euo pipefail protoc="$1" plugin="$2" -invalid_proto="$3" +repeated_header_proto="$3" +oneof_proto="$4" out_dir="$(mktemp -d)" trap 'rm -rf "${out_dir}"' EXIT root="${TEST_SRCDIR}/${TEST_WORKSPACE}" proto_include="${TEST_SRCDIR}/protobuf+/src" -if ! "${protoc}" \ - --plugin="protoc-gen-phaser=${plugin}" \ - -I"${root}" \ - -I"${proto_include}" \ - --phaser_out="frontend=ros,add_namespace=phaser:${out_dir}" \ - "${invalid_proto}" 2>"${out_dir}/err.txt"; then - grep -q "must be singular and cannot be in a oneof" "${out_dir}/err.txt" - exit 0 -fi -echo "expected phaser plugin to reject repeated ROS intrinsic" >&2 -exit 1 +# A repeated time or duration is supported; these two remain rejected. +check_rejected() { + local proto="$1" + local expected="$2" + local err="${out_dir}/err.txt" + if "${protoc}" \ + --plugin="protoc-gen-phaser=${plugin}" \ + -I"${root}" \ + -I"${proto_include}" \ + --phaser_out="frontend=ros,add_namespace=phaser:${out_dir}" \ + "${proto}" 2>"${err}"; then + echo "expected phaser plugin to reject ${proto}" >&2 + exit 1 + fi + if ! grep -q "${expected}" "${err}"; then + echo "expected ${proto} to be rejected with '${expected}', got:" >&2 + cat "${err}" >&2 + exit 1 + fi +} + +check_rejected "${repeated_header_proto}" "does not support a repeated Header" +check_rejected "${oneof_proto}" "cannot be in a oneof"