[Base] remove __COUNTER__ from unique_name macro (clang-22 c2y extensions)#1783
[Base] remove __COUNTER__ from unique_name macro (clang-22 c2y extensions)#1783tdavidcl merged 1 commit intoShamrock-code:mainfrom
Conversation
|
Thanks @tdavidcl for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
There was a problem hiding this comment.
Code Review
This pull request simplifies the __shamrock_unique_name macro by removing the conditional check for __COUNTER__ and defaulting to the use of __LINE__. A review comment suggests renaming the macro to SHAMROCK_UNIQUE_NAME to avoid using double underscores, which are reserved for the implementation in C++ and could lead to name collisions.
| #else | ||
| #define __shamrock_unique_name(base_name) internal_macro_sham_EXPAND2(base_name, __LINE__) | ||
| #endif | ||
| #define __shamrock_unique_name(base_name) internal_macro_sham_EXPAND2(base_name, __LINE__) |
There was a problem hiding this comment.
The macro name __shamrock_unique_name starts with a double underscore, which is reserved for the implementation (compiler and standard library) according to the C++ standard (and the C standard). Using reserved identifiers can lead to name collisions with compiler-defined macros or future standard library additions, potentially causing undefined behavior. It is recommended to use a prefix that does not start with a double underscore, such as SHAMROCK_UNIQUE_NAME.
| #define __shamrock_unique_name(base_name) internal_macro_sham_EXPAND2(base_name, __LINE__) | |
| #define SHAMROCK_UNIQUE_NAME(base_name) internal_macro_sham_EXPAND2(base_name, __LINE__) |
Workflow reportworkflow report corresponding to commit 8d05018 Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportNo relevant changes found. You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review. Doxygen diff with
|
see catchorg/Catch2#3076