[23128] Support builtin annotations in IDL Parser #5908

Carlosespicur · 2025-06-30T10:17:15Z

Description

This PR add support for builtin annotations in IDL Parser:

Adds Annotation class, which provides an interface to simplify the annotations parsing.
Adds a method AnnotationsList::from_builtin(), that defines each supported builtin annotation
Fixes Cleanup guards for error handling
Adds a IDLParserUtils file, for generic std::string transformations
Add a IDLParserTags file to avoid hardcoding annotations values and names

@Mergifyio backport 3.3.x

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
Any new/modified methods have been properly documented using Doxygen.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
N/A Changes are backport compatible: they do NOT break ABI nor change library core behavior.
Changes are API compatible.
New feature has been added to the versions.md file (if applicable).
New feature has been documented/Current behavior is correctly described in the documentation.
Related documentation PR:
- [23128] Support builtin annotations in IDL Parser Fast-DDS-docs#1097
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
If this is a critical bug fix, backports to the critical-only supported branches have been requested.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

test/feature/idl_parser/idl_extra_cases/builtin_annotations.idl

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlParserTags.hpp

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlGrammar.hpp

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlAnnotations.hpp

MiguelCompany · 2025-07-04T10:42:29Z

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlAnnotations.hpp

+    using TypeDescriptorAnnotationHandler = std::function<bool (TypeDescriptor::_ref_type& /* descriptor */,
+                    const std::map<std::string, std::string>&) /* annotation's member values */>;
+    using MemberDescriptorAnnotationHandler = std::function<bool (MemberDescriptor::_ref_type& /* descriptor */,
+                    const std::map<std::string, std::string>& /* annotation's member values */)>;


Please refactor these into two virtual methods, and create a class for each built-in annotation:

Suggested change

using TypeDescriptorAnnotationHandler = std::function<bool (TypeDescriptor::_ref_type& /* descriptor */,

const std::map<std::string, std::string>&) /* annotation's member values */>;

using MemberDescriptorAnnotationHandler = std::function<bool (MemberDescriptor::_ref_type& /* descriptor */,

const std::map<std::string, std::string>& /* annotation's member values */)>;

virtual bool apply_to_type(

TypeDescriptor::_ref_type& /* descriptor */,

const std::map<std::string, std::string>& /* annotation_member_values */)

{

return true;

}

virtual bool apply_to_member(

MemberDescriptor::_ref_type& /* descriptor */,

const std::map<std::string, std::string>& /* annotation_member_values */)

{

return true;

}

MiguelCompany · 2025-07-04T11:01:36Z

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlAnnotations.hpp

+    /**
+     * @brief Check if the annotation is a builtin annotation
+     */
+    bool is_builtin() const


This can also be converted into a virtual method, that returns false by default.
We can then have a BuiltinAnnotation class that just overloads this method, and then make the builtin annotations inherit from that one to customize the apply_to_xxx methods.

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…ions Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…p as AnnotationsList's container Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

MiguelCompany · 2025-07-17T07:33:43Z

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlParserTags.hpp

+} // namespace dds
+} // namespace fastdds
+} // namespace eprosima
+/* Built-in annotations related tags */


Suggested change

/* Built-in annotations related tags */

MiguelCompany · 2025-07-17T07:54:32Z

test/feature/idl_parser/idl_extra_cases/id_annotation.idl

+struct id_ann_duplicated_ids
+{
+    @id(0) long first;
+    @id(0) string second;
+    @id(1) float third;
+};
+


Suggested change

struct id_ann_duplicated_ids

{

@id(0) long first;

@id(0) string second;

@id(1) float third;

};

struct id_ann_duplicated_ids

{

@id(0) long first;

@id(0) string second;

@id(1) float third;

};

struct id_ann_implicit_duplicated_ids

{

@id(3) long first;

string second;

@id(4) float third;

};

MiguelCompany · 2025-07-17T07:58:06Z

test/feature/idl_parser/idl_extra_cases/id_annotation.idl

+
+@id(1) union id_ann_on_union switch(long)


Suggested change

@id(1) union id_ann_on_union switch(long)

union id_ann_on_union_discriminator switch(@id(1) long)

{

case 0:

long first;

default:

string second;

};

@id(1) union id_ann_on_union switch(long)

As we discussed, for now modifying discriminator's descriptor properties using annotations will be kept as unsupported. I will update the documentation with this case.

Would be nice if we can also make the parser fail with an error log when applying annotations to a discriminator.

MiguelCompany · 2025-07-17T08:01:24Z

test/feature/idl_parser/idl_extra_cases/key_annotation.idl

+    @key(value = FALSE) string second;
+    @key(FALSE) float third;
+};
+


Suggested change

union key_ann_valid_union switch(@key long)

{

case 0:

long first;

default:

string second;

};

MiguelCompany · 2025-07-17T08:03:59Z

test/feature/idl_parser/idl_extra_cases/optional_annotation.idl

+  @optional(value = FALSE) string second;
+  @optional(FALSE) float third;
+};
+


Suggested change

union optional_ann_invalid_union switch(long)

{

case 0:

@optional long first;

default:

string second;

};

MiguelCompany · 2025-07-17T08:12:45Z

test/feature/idl_parser/idl_extra_cases/value_annotation.idl

@@ -0,0 +1,20 @@
+struct value_ann_valid_struct
+{
+    @value("foo") string first;


Annotation @value is only valid for enumeration members.

This case is difficult to treat, due to both @default and @value annotations can be used to configure MemberDescriptor::default_value property, and IDL/XTypes specifications impose restrictions over the parent kind of a @value annotated member, but not in the @default case, i.e:

struct valid_struct { @default(1) long valid_member; };

should be valid, but:

struct invalid_struct { @value(1) long invalid_member; };

not. Both cases should be handled when adding the member to the constructed type (before adding it, we don't know the member's parent kind), but I think there is not a way to address the problem calling MemberDescriptor::is_consistent() method. There are two possibilities:

Modify is_consistent() logic to allow configuring MemberDescriptor::default_value() if and only if MemberDescriptorImpl::parent_kind_ == TK_ENUM, making both described cases to fail, but restricting @value to enumerations.

Keep is_consistent() logic, which makes both described cases to be parsed as the same.

Is there a better approach for this? @MiguelCompany @richiware

MiguelCompany · 2025-07-17T08:20:00Z

test/feature/idl_parser/idl_extra_cases/value_annotation.idl

+@value(8) struct value_ann_on_struct
+{
+    long first;
+};


Missing files for:

non_serialized

hashid

default_literal

New is_default_literal member added in MemberDescriptor/DynamicTypeBuilder/DynamicType related classes and DefaultLiteralAnnotation updated to modify it.

TODO: non_serialized, hashid, autoid implementations in DynTypes API

MiguelCompany · 2025-07-17T08:24:56Z

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

@@ -160,7 +261,7 @@ class Context
            std::map<std::string, std::string>& state,
            const std::string& type);

-    std::vector<std::string> split_string(
+    static std::vector<std::string> split_string(


Shall we move this to IdlParserUtils.hpp?

Yes, I think it's unrelated to the context

MiguelCompany · 2025-07-17T08:25:08Z

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

@@ -174,19 +275,38 @@ class Context
        return tokens;
    }

+    static std::string join_strings(


Shall we move this to IdlParserUtils.hpp?

MiguelCompany · 2025-07-17T08:51:35Z

src/cpp/fastdds/xtypes/dynamic_types/idl_parser/IdlAnnotations.hpp

+#include "../TypeValueConverter.hpp"
+#include "IdlParserTags.hpp"
+#include "IdlParserUtils.hpp"
+


Improve readability and maintainability by having each declared type in a separate file.
Then include all files here.

I will wait for this refactor to take place before reviewing this

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…otated without parameters Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…nd tests Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…eral type in type descriptor Signed-off-by: Carlosespicur <carlosespicur@proton.me>

…e member in MemberDescriptor Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur added this to the v3.3.0 milestone Jun 30, 2025

Carlosespicur force-pushed the feature/idl-parser-builtin-annotations branch 7 times, most recently from 71ea547 to 7c8a32c Compare July 4, 2025 07:10

Carlosespicur marked this pull request as ready for review July 4, 2025 07:39

MiguelCompany requested changes Jul 4, 2025

View reviewed changes

test/feature/idl_parser/idl_extra_cases/builtin_annotations.idl Outdated Show resolved Hide resolved

MiguelCompany requested changes Jul 4, 2025

View reviewed changes

rsanchez15 modified the milestones: v3.3.0 - Frozen ❄️, v3.3.1 Jul 7, 2025

Carlosespicur force-pushed the feature/idl-parser-builtin-annotations branch 3 times, most recently from c99f9a2 to 7effe6a Compare July 14, 2025 06:17

Carlosespicur added 14 commits July 14, 2025 08:18

Refs #23128: Add annotations interfaces

0dca6de

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Add annotation implementation and create builtin annotat…

b02b035

…ions Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Handle annotations in Parser

6586202

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Remove flags

03b90ab

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Uncrustify

fb57b04

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Add tests [WIP]

440f028

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Separate each annotation in tests

edfd828

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Fix identation

c62e3c3

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Add missing XTypes annotation tags

7f48a7c

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Fix annotation parameters grammar and resolution

9302298

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Refactor IdlAnnotations classes and use std::ma…

2bf1a7e

…p as AnnotationsList's container Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Remove escape characters in string values

4fac68e

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Fix @bit_bound annotation logic

e893440

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Fix enum members annotated using keyword notation

ecbd91d

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Update versions.md

b787c9f

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur force-pushed the feature/idl-parser-builtin-annotations branch from 94991d9 to b787c9f Compare July 14, 2025 06:19

Carlosespicur requested a review from MiguelCompany July 14, 2025 06:20

github-actions bot added the ci-pending PR which CI is running label Jul 14, 2025

Carlosespicur mentioned this pull request Jul 14, 2025

[23128] Support builtin annotations in IDL Parser eProsima/Fast-DDS-docs#1097

Open

6 tasks

Refs #23128: Fix unused exception variable

d433a19

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur requested review from MiguelCompany and removed request for MiguelCompany July 14, 2025 09:51

Refs #23128: Fix std::tolower warning when casting to char

b313fa5

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur requested review from MiguelCompany and removed request for MiguelCompany July 15, 2025 07:11

Refs #23128: Fix fixed point numbers grammar

d9e9a91

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur requested review from MiguelCompany and removed request for MiguelCompany July 15, 2025 08:00

MiguelCompany requested changes Jul 17, 2025

View reviewed changes

Refs #23128: Review - Remove invalid comment

dd9ecd4

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur modified the milestones: v3.3.1, v3.4.0 Jul 18, 2025

Carlosespicur added 6 commits July 18, 2025 12:52

Refs #23128: Review - Address changes related to current IDLParser tests

33dc4b8

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Move join/split string methods to utils

b72c07d

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Separate each builtin annotation implementation

7dd91ae

Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Fix trailing whitespaces consumption in enum members ann…

fa7af26

…otated without parameters Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Add default_literal annotation implementation a…

7986db4

…nd tests Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Refs #23128: Review - Fix bit_bound annotation and configure enum lit…

00e9f3d

…eral type in type descriptor Signed-off-by: Carlosespicur <carlosespicur@proton.me>

Carlosespicur force-pushed the feature/idl-parser-builtin-annotations branch from e6f0c92 to 00e9f3d Compare July 22, 2025 09:35

Refs #23128: Review - Fix value annotation and implement literal valu…

5fca3b9

…e member in MemberDescriptor Signed-off-by: Carlosespicur <carlosespicur@proton.me>

+union optional_ann_invalid_union switch(long)
+{
+  case 0:
+    @optional long first;
+  default:
+    string second;
+};

[23128] Support builtin annotations in IDL Parser #5908

Are you sure you want to change the base?

[23128] Support builtin annotations in IDL Parser #5908

Conversation

Carlosespicur commented Jun 30, 2025 • edited by rsanchez15 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Carlosespicur commented Jun 30, 2025 •

edited by rsanchez15

Loading