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

Migrate c modules to cpp #321

Merged
merged 21 commits into from
Sep 24, 2024
Merged

Conversation

patkenneally
Copy link
Collaborator

@patkenneally patkenneally commented Sep 11, 2024

  • Tickets addressed:
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

The first three commits of this PR are preparation for the mammoth migration of all C modules to C++.

  • The first removes unnecessary virtual destructor from the BSKLogger and makes the class final.
  • The second rapidly aborts when reading an unsubscribed ReadFunctor, because its pointer is at best null and at worst garbage.
  • The third makes the log level std::map static. There was a strange and perhaps undiagnosable memory access fault seemingly involving copy/move constructors and we couldn't get to the root cause. Rather than spend excessive amounts of time, making the map static removed the potential for error.

The fourth commit, migrates all C modules to C++. To quote from the associated commit message,

This is the first step to removing the C messaging infrastructure. The C++ messaging system is better fit for purpose and requires little-to-no code generation. C can always be used from C++, so this drastically reduces our exposed API surface at no cost.

Subsequent commits remove all C messaging infrastructure from simulation modules, and remove all C messaging infrastructure.

Verification

All CI runs

Documentation

So much documentation was probably invalidated. This is a Future Pat task (seriously though, there is a ticket for updating this after the messaging migration).

Future work

None

@patkenneally patkenneally force-pushed the refactor/migrate-c-modules-to-cpp branch from c06972c to a219900 Compare September 11, 2024 06:42
@patkenneally patkenneally force-pushed the refactor/migrate-c-modules-to-cpp branch 6 times, most recently from 63daed9 to cd9c41b Compare September 16, 2024 21:27
@patkenneally patkenneally force-pushed the refactor/migrate-c-modules-to-cpp branch 3 times, most recently from 74571a0 to 4f868a3 Compare September 23, 2024 23:01
Twisol and others added 15 commits September 23, 2024 20:35
This is the first step to removing the C messaging infrastructure.
The C++ messaging system is better fit for purpose and requires
little-to-no code generation. C can always be used from C++, so
this drastically reduces our exposed API surface at no cost.

Co-authored-by: Jonathan Castello <[email protected]>
Co-authored-by: Jonathan Castello <[email protected]>
In addition it would seem that the recorder redirection test
was pretty busted. The test is ammended and now tests correct
behavior.

Co-authored-by: Jonathan Castello <[email protected]>
Co-authored-by: Jonathan Castello <[email protected]>
@patkenneally patkenneally force-pushed the refactor/migrate-c-modules-to-cpp branch from 9b27093 to 22891c8 Compare September 24, 2024 02:35
@patkenneally patkenneally force-pushed the refactor/migrate-c-modules-to-cpp branch from 22891c8 to ac1ab13 Compare September 24, 2024 04:11
@patkenneally patkenneally merged commit f32aa0e into develop Sep 24, 2024
0 of 3 checks passed
@patkenneally patkenneally deleted the refactor/migrate-c-modules-to-cpp branch September 24, 2024 04:13
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.

2 participants