Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CETL VariableLengthArray and polymorphic_allocator #316

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

skeetsaz
Copy link
Contributor

@skeetsaz skeetsaz commented Aug 8, 2023

This change modifies the C++ code-gen to create message classes that can be used with allocators.

  • The message class becomes an "_Impl" template that takes an allocator, which gets passed down to any variable length arrays in the messages (or in contained messages.) The normal message name is then aliased to the "_Impl" template using the allocator specified in the yaml config. In the end the message is a regular struct as it was before.
  • The old variable_length_array.hpp is removed from the support library. CETL is added as a submodule and the new variable_length_array.hpp from there is used.
  • Added a verification test to exercise a message using CETL's polymorphic_allocator

@thirtytwobits
Copy link
Member

You need to run Black to get the CI past the formatting errors.

src/nunavut/lang/cpp/__init__.py Outdated Show resolved Hide resolved
src/nunavut/lang/cpp/__init__.py Outdated Show resolved Hide resolved
src/nunavut/lang/cpp/templates/_composite_type.j2 Outdated Show resolved Hide resolved
@skeetsaz
Copy link
Contributor Author

Updated my pull-request per the feedback. The ctor_convention approach and -std=cetl++ -std=c++17-pmr flags help simplify things and maintain better backward compatibility.

The verification tests I added will fail due to CETL VLA not implementing the trailing-allocator convention correctly (I filed OpenCyphal/CETL#57 for that.) But I want to get this out for more feedback.

Copy link
Member

@thirtytwobits thirtytwobits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to specify the default memory resource that I'm missing?

src/nunavut/cli/runners.py Outdated Show resolved Hide resolved
docs/languages.rst Outdated Show resolved Hide resolved
src/nunavut/lang/__init__.py Outdated Show resolved Hide resolved
src/nunavut/lang/cpp/__init__.py Outdated Show resolved Hide resolved
src/nunavut/lang/cpp/__init__.py Outdated Show resolved Hide resolved
src/nunavut/lang/cpp/templates/_composite_type.j2 Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Sep 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

86.4% 86.4% Coverage
0.0% 0.0% Duplication

@thirtytwobits thirtytwobits merged commit f78e3f7 into OpenCyphal:main Sep 6, 2023
182 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants