Skip to content

Commit 00e9f3d

Browse files
CarlosespicurCarlosespicur
committed
Refs #23128: Review - Fix bit_bound annotation and configure enum literal type in type descriptor
Signed-off-by: Carlosespicur <carlosespicur@proton.me>
1 parent 7986db4 commit 00e9f3d

File tree

10 files changed

+267
-78
lines changed

10 files changed

+267
-78
lines changed

‎include/fastdds/dds/xtypes/dynamic_types/TypeDescriptor.hpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,34 @@ class FASTDDS_EXPORTED_API TypeDescriptor
117117
virtual void discriminator_type(
118118
traits<DynamicType>::ref_type type) = 0;
119119

120+
/*!
121+
* Returns a reference to the literal type. The reference can be nil.
122+
*
123+
* @note This is only valid for @ref TK_ENUM types, and it represents the expected type of its literals.
124+
* @warning This method is an extension of the standard TypeDescriptor interface (see [standard] section \b 7.5.2.4 )
125+
* @return @ref DynamicType reference.
126+
*/
127+
virtual traits<DynamicType>::ref_type literal_type() const = 0;
128+
129+
/*!
130+
* Returns a reference to the literal type. The reference can be nil.
131+
*
132+
* @note This is only valid for @ref TK_ENUM types, and it represents the expected type of its literals.
133+
* @warning This method is an extension of the standard TypeDescriptor interface (see [standard] section \b 7.5.2.4 )
134+
* @return @ref DynamicType reference.
135+
*/
136+
virtual traits<DynamicType>::ref_type& literal_type() = 0;
137+
138+
/*!
139+
* Modifies the underlying literal type reference.
140+
*
141+
* @param [in] type @ref DynamicType reference.
142+
* @note This is only valid for @ref TK_ENUM types, and it represents the expected type of its literals.
143+
* @warning This method is an extension of the standard TypeDescriptor interface (see [standard] section \b 7.5.2.4 )
144+
*/
145+
virtual void literal_type(
146+
traits<DynamicType>::ref_type type) = 0;
147+
120148
/*!
121149
* Returns the bound.
122150
* @return @ref BoundSeq.

‎src/cpp/fastdds/xtypes/dynamic_types/DynamicTypeBuilderImpl.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,14 @@ ReturnCode_t DynamicTypeBuilderImpl::add_member(
526526
}
527527
}
528528

529+
// Member type and enum's literal type must be the same, if literal type is specified
530+
if (type_descriptor_.literal_type() &&
531+
descriptor->type()->get_kind() != type_descriptor_.literal_type()->get_kind())
532+
{
533+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Descriptor type kind differs from the literal type kind.");
534+
return RETCODE_BAD_PARAMETER;
535+
}
536+
529537
if (descriptor_impl->is_default_literal())
530538
{
531539
for (auto member : members_)

‎src/cpp/fastdds/xtypes/dynamic_types/TypeDescriptorImpl.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ ReturnCode_t TypeDescriptorImpl::copy_from(
125125
name_ = descriptor.name_;
126126
base_type_ = descriptor.base_type_;
127127
discriminator_type_ = descriptor.discriminator_type_;
128+
literal_type_ = descriptor.literal_type_;
128129
bound_ = descriptor.bound_;
129130
element_type_ = descriptor.element_type_;
130131
key_element_type_ = descriptor.key_element_type_;
@@ -149,6 +150,8 @@ bool TypeDescriptorImpl::equals(
149150
((!base_type_ && !descriptor.base_type_) || (base_type_ && base_type_->equals(descriptor.base_type_))) &&
150151
((!discriminator_type_ && !descriptor.discriminator_type_) ||
151152
(discriminator_type_ && discriminator_type_->equals(descriptor.discriminator_type_))) &&
153+
((!literal_type_ && !descriptor.literal_type_) ||
154+
(literal_type_ && literal_type_->equals(descriptor.literal_type_))) &&
152155
bound_ == descriptor.bound_ &&
153156
((!element_type_ && !descriptor.element_type_) ||
154157
(element_type_ && element_type_->equals(descriptor.element_type_))) &&
@@ -205,6 +208,28 @@ bool TypeDescriptorImpl::is_consistent() noexcept
205208
}
206209
}
207210

211+
if (literal_type_)
212+
{
213+
// Literal types are only used by enumerations.
214+
if (TK_ENUM != kind_)
215+
{
216+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Descriptor describes a type which not support literal types");
217+
return false;
218+
}
219+
220+
TypeKind literal_kind =
221+
traits<DynamicType>::narrow<DynamicTypeImpl>(literal_type_)->resolve_alias_enclosed_type()
222+
->get_kind();
223+
224+
if (TK_INT8 != literal_kind &&
225+
TK_INT16 != literal_kind &&
226+
TK_INT32 != literal_kind)
227+
{
228+
EPROSIMA_LOG_ERROR(DYN_TYPES, "Descriptor contains a literal type of an invalid kind");
229+
return false;
230+
}
231+
}
232+
208233
if (0 < bound_.size() && (
209234
TK_ARRAY != kind_ &&
210235
TK_BITMASK != kind_ &&

‎src/cpp/fastdds/xtypes/dynamic_types/TypeDescriptorImpl.hpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ class TypeDescriptorImpl : public virtual TypeDescriptor
3535
//! Discriminator type for a union.
3636
traits<DynamicType>::ref_type discriminator_type_;
3737

38+
//! Literal type for an enumeration.
39+
traits<DynamicType>::ref_type literal_type_;
40+
3841
//! Length for strings, arrays, sequences, maps and bitmasks.
3942
BoundSeq bound_;
4043

@@ -136,6 +139,22 @@ class TypeDescriptorImpl : public virtual TypeDescriptor
136139
discriminator_type_ = type;
137140
}
138141

142+
traits<DynamicType>::ref_type literal_type() const noexcept override
143+
{
144+
return literal_type_;
145+
}
146+
147+
traits<DynamicType>::ref_type& literal_type() noexcept override
148+
{
149+
return literal_type_;
150+
}
151+
152+
void literal_type(
153+
traits<DynamicType>::ref_type type) noexcept override
154+
{
155+
literal_type_ = type;
156+
}
157+
139158
const BoundSeq& bound() const noexcept override
140159
{
141160
return bound_;

‎src/cpp/fastdds/xtypes/dynamic_types/idl_parser/Annotations/BitBoundAnnotation.hpp

Lines changed: 50 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,19 @@ class BitBoundAnnotation final : public BuiltinAnnotation
7878
return false;
7979
}
8080

81-
if ((TK_BITSET != descriptor->kind()) && (TK_BITMASK != descriptor->kind()))
81+
if ((TK_ENUM != descriptor->kind()) && (TK_BITMASK != descriptor->kind()))
8282
{
8383
EPROSIMA_LOG_ERROR(IDL_PARSER,
8484
"TypeDescriptor can only be annotated with '" << IDL_BUILTIN_ANN_BIT_BOUND_TAG
85-
<< "' for bitset/bitmask types.");
85+
<< "' for enumeration/bitmask types.");
8686
return false;
8787
}
8888

89+
TypeForKind<TK_UINT16> value;
90+
8991
try
9092
{
91-
TypeForKind<TK_UINT16> value = TypeValueConverter::sto(parameters.at(IDL_VALUE_TAG));
92-
descriptor->bound({value});
93+
value = TypeValueConverter::sto(parameters.at(IDL_VALUE_TAG));
9394
}
9495
catch (const std::exception& e)
9596
{
@@ -101,65 +102,70 @@ class BitBoundAnnotation final : public BuiltinAnnotation
101102
return false;
102103
}
103104

105+
if (TK_BITMASK == descriptor->kind())
106+
{
107+
descriptor->bound({value});
108+
}
109+
else
110+
{
111+
TypeKind literal_kind;
112+
113+
try
114+
{
115+
literal_kind = get_literal_kind_from_bound(value);
116+
}
117+
catch (const std::exception& e)
118+
{
119+
EPROSIMA_LOG_ERROR(IDL_PARSER,
120+
"Failed to get literal kind from bound for annotation '" << IDL_BUILTIN_ANN_BIT_BOUND_TAG
121+
<< "': " << e.what());
122+
return false;
123+
}
124+
125+
descriptor->literal_type(DynamicTypeBuilderFactory::get_instance()->get_primitive_type(literal_kind));
126+
}
127+
104128
return true;
105129
}
106130

107131
bool apply_to_member(
108132
MemberDescriptor::_ref_type& descriptor,
109133
const std::map<std::string, std::string>& parameters) const override
110134
{
111-
assert(descriptor != nullptr);
135+
static_cast<void>(descriptor);
136+
static_cast<void>(parameters);
112137

113-
if (parameters.find(IDL_VALUE_TAG) == parameters.end())
114-
{
115-
EPROSIMA_LOG_ERROR(IDL_PARSER,
116-
"Missing required parameter '" << IDL_VALUE_TAG
117-
<< "' for annotation '" << IDL_BUILTIN_ANN_BIT_BOUND_TAG <<
118-
"'.");
119-
return false;
120-
}
121-
122-
TypeForKind<TK_UINT16> value;
138+
EPROSIMA_LOG_ERROR(IDL_PARSER, "Trying to apply @bit_bound annotation to a MemberDescriptor.");
139+
return false;
140+
}
123141

124-
try
142+
/**
143+
* @brief Get the TypeKind corresponding to the literal type of a @bit_bound annotated enumeration
144+
*
145+
* @param value The value of the @bit_bound annotation
146+
* @return The enumeration's literal type kind.
147+
* @throws std::invalid_argument if value is 0 or higher than 32.
148+
*/
149+
TypeKind get_literal_kind_from_bound(
150+
TypeForKind<TK_UINT16> value) const
151+
{
152+
if (value == 0 || value > 32)
125153
{
126-
value = TypeValueConverter::sto(parameters.at(IDL_VALUE_TAG));
154+
throw std::invalid_argument("Invalid bit bound value: " + std::to_string(value));
127155
}
128-
catch (const std::exception& e)
129-
{
130-
EPROSIMA_LOG_ERROR(IDL_PARSER,
131-
"Failed to convert value '" << parameters.at(
132-
IDL_VALUE_TAG)
133-
<< "' for annotation '" << IDL_BUILTIN_ANN_BIT_BOUND_TAG << "': " <<
134-
e.what());
135-
return false;
136-
}
137-
138-
DynamicType::_ref_type member_type;
139156

140-
if (value == 8)
141-
{
142-
member_type = DynamicTypeBuilderFactory::get_instance()->get_primitive_type(TK_INT8);
143-
}
144-
else if (value == 16)
157+
if (value <= 8)
145158
{
146-
member_type = DynamicTypeBuilderFactory::get_instance()->get_primitive_type(TK_INT16);
159+
return TK_INT8;
147160
}
148-
else if (value == 32)
161+
else if (value <= 16)
149162
{
150-
member_type = DynamicTypeBuilderFactory::get_instance()->get_primitive_type(TK_INT32);
163+
return TK_INT16;
151164
}
152165
else
153166
{
154-
EPROSIMA_LOG_ERROR(IDL_PARSER,
155-
"Invalid bit bound value '" << value << "' for annotation '"
156-
<< IDL_BUILTIN_ANN_BIT_BOUND_TAG << "'.");
157-
return false;
167+
return TK_INT32;
158168
}
159-
160-
descriptor->type(member_type);
161-
162-
return true;
163169
}
164170

165171
bool initialized_;

‎src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlModule.hpp

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class Module
139139
|| unions_.count(ident) > 0
140140
|| aliases_.count(ident) > 0
141141
|| constants_.count(ident) > 0
142-
|| enumerations_32_.count(ident) > 0
142+
|| enumerations_.count(ident) > 0
143143
|| inner_.count(ident) > 0;
144144

145145
if (has_it)
@@ -402,27 +402,27 @@ class Module
402402
return result.second;
403403
}
404404

405-
bool has_enum_32(
405+
bool has_enum(
406406
const std::string& name) const
407407
{
408-
return enumerations_32_.count(name) > 0;
408+
return enumerations_.count(name) > 0;
409409
}
410410

411-
bool enum_32(
411+
bool enumeration(
412412
const std::string& name,
413413
DynamicTypeBuilder::_ref_type builder,
414414
bool replace = false)
415415
{
416416
if (replace)
417417
{
418-
auto it = enumerations_32_.find(name);
419-
if (it != enumerations_32_.end())
418+
auto it = enumerations_.find(name);
419+
if (it != enumerations_.end())
420420
{
421-
enumerations_32_.erase(it);
421+
enumerations_.erase(it);
422422
}
423423
}
424424

425-
auto result = enumerations_32_.emplace(name, builder);
425+
auto result = enumerations_.emplace(name, builder);
426426

427427
return result.second;
428428
}
@@ -478,9 +478,9 @@ class Module
478478
}
479479

480480
// Check enums
481-
if (module.first->has_enum_32(module.second))
481+
if (module.first->has_enum(module.second))
482482
{
483-
builder = module.first->enumerations_32_.at(module.second);
483+
builder = module.first->enumerations_.at(module.second);
484484
}
485485

486486
// Check structs
@@ -525,10 +525,9 @@ class Module
525525
// std::map<std::string, Type> constants_types_;
526526
std::map<std::string, DynamicData::_ref_type> constants_;
527527
std::vector<std::string> from_enum_;
528-
std::map<std::string, DynamicTypeBuilder::_ref_type> enumerations_32_;
528+
std::map<std::string, DynamicTypeBuilder::_ref_type> enumerations_;
529529
std::map<std::string, DynamicTypeBuilder::_ref_type> structs_;
530530
std::map<std::string, DynamicTypeBuilder::_ref_type> unions_;
531-
//std::map<std::string, std::shared_ptr<AnnotationType>> annotations_;
532531
Module* outer_;
533532
std::map<std::string, std::shared_ptr<Module>> inner_;
534533
std::string name_;

‎src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlParser.hpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,7 +1869,15 @@ struct action<enum_dcl>
18691869
const std::string& member_name = tokens[i];
18701870
MemberDescriptor::_ref_type member_descriptor {traits<MemberDescriptor>::make_shared()};
18711871
member_descriptor->name(member_name);
1872-
member_descriptor->type(factory->get_primitive_type(TK_INT32));
1872+
1873+
if (type_descriptor->literal_type())
1874+
{
1875+
member_descriptor->type(type_descriptor->literal_type());
1876+
}
1877+
else
1878+
{
1879+
member_descriptor->type(factory->get_primitive_type(TK_INT32));
1880+
}
18731881

18741882
// Annotate the member descriptor with the annotations collected during parsing
18751883
const auto& member_annotations = ctx->annotations().pending_member_annotations();
@@ -1901,7 +1909,7 @@ struct action<enum_dcl>
19011909
}
19021910

19031911
EPROSIMA_LOG_INFO(IDLPARSER, "Found enum: " << scoped_enum_name);
1904-
module->enum_32(scoped_enum_name, builder);
1912+
module->enumeration(scoped_enum_name, builder);
19051913

19061914
if (scoped_enum_name == ctx->target_type_name)
19071915
{

‎src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlParserUtils.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
#define FASTDDS_XTYPES_DYNAMIC_TYPES_IDL_PARSER_IDLPARSERUTILS_HPP
1717

1818
#include <algorithm>
19-
#include <sstream>
20-
#include <vector>
2119
#include <cctype>
20+
#include <sstream>
2221
#include <string>
22+
#include <vector>
2323

2424
namespace eprosima {
2525
namespace fastdds {

0 commit comments

Comments
 (0)