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

#376: rename virtual serialize macros #378

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

cz4rs
Copy link
Contributor

@cz4rs cz4rs commented Nov 20, 2024

fixes #376

Adds magistrate_ version of the macros and deprecates the old checkpoint_ ones.

Since it's impossible to deprecate a macro on its own, I am using a helper (constexpr) function + a static_assert combo to generate warnings. You can see gtest doing a similiar thing here.

@cz4rs cz4rs force-pushed the 376-rename-virtual-serialize-macros branch 3 times, most recently from 379493e to 2499860 Compare November 20, 2024 12:45
@cz4rs
Copy link
Contributor Author

cz4rs commented Nov 20, 2024

Example deprecation message from PR tests with vt job:

/vt/src/vt/runtime/component/diagnostic_value_base.h:72:3: warning: 'checkpoint_virtual_serialize_root_is_deprecated' is deprecated: checkpoint_virtual_serialize_root is deprecated, please use magistrate_virtual_serialize_root [-Wdeprecated-declarations]
  checkpoint_virtual_serialize_root()
  ^
/build/checkpoint/install/include/checkpoint/dispatch/vrt/base.h:60:17: note: expanded from macro 'checkpoint_virtual_serialize_root'
  static_assert(checkpoint_virtual_serialize_root_is_deprecated()); \
                ^
/build/checkpoint/install/include/checkpoint/dispatch/vrt/base.h:53:3: note: 'checkpoint_virtual_serialize_root_is_deprecated' has been explicitly marked deprecated here
[[deprecated("checkpoint_virtual_serialize_root is deprecated,"
  ^

@cz4rs cz4rs force-pushed the 376-rename-virtual-serialize-macros branch from 2499860 to 297099a Compare November 22, 2024 17:26
Copy link
Contributor

@cwschilly cwschilly left a comment

Choose a reason for hiding this comment

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

Example deprecation message from PR tests with vt job:

/vt/src/vt/runtime/component/diagnostic_value_base.h:72:3: warning: 'checkpoint_virtual_serialize_root_is_deprecated' is deprecated: checkpoint_virtual_serialize_root is deprecated, please use magistrate_virtual_serialize_root [-Wdeprecated-declarations]
...

I don't fully understand this warning message. Why is checkpoint_virtual_serialize_root_is_deprecated deprecated?

@cz4rs
Copy link
Contributor Author

cz4rs commented Nov 25, 2024

I don't fully understand this warning message. Why is checkpoint_virtual_serialize_root_is_deprecated deprecated?

You cannot deprecate the macro itself, so I'm using this constexpr function to generate a warning. There's probably a couple of possibilities here, this one is inspired by gtest.

edit: Improved the PR description too.

@cz4rs cz4rs requested a review from cwschilly November 25, 2024 17:30
@lifflander lifflander merged commit 523d7ca into develop Dec 5, 2024
21 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.

rename checkpoint_virtual_serialize macros to magistrate_virtual_serialize
3 participants