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

feat: Update default language standard to the latest standard + feat: Set cppcheck default std instead of defaulting standard based on it #271

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

FeignClaims
Copy link
Contributor

@FeignClaims FeignClaims commented Aug 13, 2024

Update the default CMAKE_CXX_STANDARD and CMAKE_C_STANDARD settings to the latest standard version if they're not specified by the user.

Additionally, the standard test code is generalized to a function _set_language_standard:

_set_language_standard(CXX_LATEST_STANDARD CXX 23 20 17 14 11)
_set_language_standard(C_LATEST_STANDARD C 23 20 17 11 99 90)

@FeignClaims FeignClaims force-pushed the feat/update_c_cxx_latest_standard branch 2 times, most recently from 2b0a70f to 303b6ea Compare August 13, 2024 13:16
@FeignClaims FeignClaims changed the title feat: Update C and C++ standard checking to latest standard feat: Update default language standard to the latest standard + feat: Set cppcheck default std instead of defaulting standard based on it Aug 13, 2024
@FeignClaims
Copy link
Contributor Author

FeignClaims commented Aug 13, 2024

C++ fails to use 23 and C fails to use 23,20,17 because cppcheck dosen't support them. Instead of implicitly using an old standard, I think it's fairly reasonable to just use the latest standard while setting cppcheck to use the default standard, and give a ${WARNING_MESSAGE} about the cppcheck behaviour to those who care.

Also, the corresponding cppcheck option of set(CMAKE_C_STANDARD 90) is --std=c89 according to cppcheck --help:

--std=<id>           Set standard.
                     The available options are:
                      * c89
                             C code is C89 compatible
                      * c99
                             C code is C99 compatible
                      * c11
                             C code is C11 compatible (default)
                      * c++03
                             C++ code is C++03 compatible
                      * c++11
                             C++ code is C++11 compatible
                      * c++14
                             C++ code is C++14 compatible
                      * c++17
                             C++ code is C++17 compatible
                      * c++20
                             C++ code is C++20 compatible (default)

Versions are hard-coded in the code, as cppcheck runs normally even if I run cppcheck --std=c++100 so I can't test whether the option is correct by running it.

if(CMAKE_CXX_STANDARD MATCHES [[03|11|14|17|20]])
  set(CMAKE_CXX_CPPCHECK ${CMAKE_CXX_CPPCHECK} --std=c++${CMAKE_CXX_STANDARD})
else()
  message(${WARNING_MESSAGE} "cppcheck dosen't support specified C++ standard ${CMAKE_CXX_STANDARD}. Using the cppcheck default C++ standard version.")
endif()

@FeignClaims FeignClaims force-pushed the feat/update_c_cxx_latest_standard branch from 303b6ea to d40ef51 Compare August 13, 2024 14:06
@FeignClaims FeignClaims force-pushed the feat/update_c_cxx_latest_standard branch from d40ef51 to ca70709 Compare August 13, 2024 14:13
@FeignClaims FeignClaims marked this pull request as ready for review August 13, 2024 14:31
src/StaticAnalyzers.cmake Show resolved Hide resolved
src/StaticAnalyzers.cmake Show resolved Hide resolved
src/StaticAnalyzers.cmake Show resolved Hide resolved
@aminya
Copy link
Owner

aminya commented Aug 28, 2024

Is this ready to get merged? Can we address the essential part of my comments, or do you think this will not cause issues?

@FeignClaims
Copy link
Contributor Author

FeignClaims commented Aug 29, 2024

Is this ready to get merged? Can we address the essential part of my comments, or do you think this will not cause issues?

@aminya Sorry for the delay. According to my recent attempts, cppcheck itself is unable to detect the version issue, so this is the best result we can achieve at the moment.

I believe the current changes are sufficient and ready to merge, and I can manually maintain the version changes afterwards - after all, we were manually adjusting to the latest C++ version before.

Copy link
Owner

@aminya aminya left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution

@aminya aminya merged commit 588c32b into aminya:main Aug 29, 2024
18 checks passed
@FeignClaims FeignClaims deleted the feat/update_c_cxx_latest_standard branch August 31, 2024 16:21
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