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

Update compiler warning levels #4558

Merged
merged 1 commit into from Apr 22, 2024
Merged

Conversation

jrw972
Copy link
Contributor

@jrw972 jrw972 commented Apr 3, 2024

See #4550.

@jrw972 jrw972 self-assigned this Apr 3, 2024
cmake/opendds_build_helpers.cmake Outdated Show resolved Hide resolved
@jrw972 jrw972 force-pushed the compiler-flags branch 2 times, most recently from c6f6a78 to 4bbbc58 Compare April 4, 2024 21:43
@jrw972 jrw972 marked this pull request as ready for review April 8, 2024 15:41
@jrw972 jrw972 requested a review from mitza-oci April 8, 2024 15:41
Copy link
Member

@mitza-oci mitza-oci left a comment

Choose a reason for hiding this comment

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

  • add new CMake option to documentation
  • update MPC build(s)

@iguessthislldo
Copy link
Member

update MPC build(s)

How do we want to do this? In the configure script?

cmake/opendds_build_helpers.cmake Outdated Show resolved Hide resolved
.github/workflows/cmake.yml Show resolved Hide resolved
@@ -672,6 +672,13 @@ These are all the variables that are exclusive to building OpenDDS with CMake:
See :ref:`cmake-running-tests` for how to run them.
The default for this is ``TRUE``.

.. cmake:var:: OPENDDS_COMPILE_WARNINGS

If not empty, enables compiler warnings when compiling OpenDDS.
Copy link
Member

Choose a reason for hiding this comment

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

"enables more compiler warnings" ? Aren't some enabled by default either in CMake, its specific Generators, or the compilers themselves?

@mitza-oci
Copy link
Member

update MPC build(s)

How do we want to do this? In the configure script?

I think that's a good option

@jrw972 jrw972 force-pushed the compiler-flags branch 3 times, most recently from 38dcccd to fc9f335 Compare April 10, 2024 16:21
@jrw972 jrw972 added this to the 3.29 milestone Apr 17, 2024
@jrw972 jrw972 force-pushed the compiler-flags branch 3 times, most recently from 53faf7b to 18e5a88 Compare April 18, 2024 18:59
Comment on lines 25 to 27
-Wall -Wpedantic -Wno-unused -Wextra -Wcast-align -Wcast-qual -Wctor-dtor-privacy
-Wdisabled-optimization -Wformat=2 -Winit-self -Wmissing-include-dirs -Woverloaded-virtual
-Wredundant-decls -Wshadow -Wsign-conversion -Wstrict-overflow=5 -Wundef
Copy link
Member

Choose a reason for hiding this comment

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

Instead of duplicating these, what about putting them in a separate file which is read by both CMake and configure?

Copy link
Member

Choose a reason for hiding this comment

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

Like acetao.ini does now? I don't know if we want to put them there or not though.

@@ -18,6 +31,13 @@ function(_opendds_alias target)
add_executable("${name}" ALIAS "${target}")
endif()
set_target_properties(${target} PROPERTIES EXPORT_NAME "${name}")

if(OPENDDS_COMPILE_WARNINGS STREQUAL "WARNING" OR OPENDDS_COMPILE_WARNINGS STREQUAL "ERROR")
target_compile_options(${target} PRIVATE ${CMAKE_CXX_COMPILER_ID}-warning)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iguessthislldo What is the necessary syntax here?

Copy link
Member

Choose a reason for hiding this comment

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

I think what you want is:

Suggested change
target_compile_options(${target} PRIVATE ${CMAKE_CXX_COMPILER_ID}-warning)
target_compile_options(${target} PRIVATE ${${CMAKE_CXX_COMPILER_ID}-warning}})

Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming a space delimited list will be interpreted the same as a normal CMake ;-delimited list though.

@jrw972 jrw972 force-pushed the compiler-flags branch 2 times, most recently from 2a96e13 to b218f22 Compare April 19, 2024 16:51
@jrw972 jrw972 merged commit a17f490 into OpenDDS:master Apr 22, 2024
131 of 132 checks passed
@jrw972 jrw972 deleted the compiler-flags branch April 22, 2024 18:40
@jrw972 jrw972 linked an issue Apr 22, 2024 that may be closed by this pull request
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.

Update compiler warning levels used for CI
3 participants