Skip to content

Commit

Permalink
Merge #1555: Fixed O3 replacement
Browse files Browse the repository at this point in the history
b8fe333 cmake: Fixed O3 replacement (Eduardo Menges Mattje)

Pull request description:

  Old replacement of `O3` in `CMAKE_C_FLAGS_RELEASE` skip spaces, which is problematic. For instance, if `CMAKE_C_FLAGS_RELEASE = "-O3 -DFOO"`, regex will replace it with `-O2-DFOO`, which causes a compile error.

  This patch changes this behavior, keeping whichever space exists between the flags.

  If I may question, what is the rationale behind replacing `O3` with `O2`? Changing the user's flags is a bad practice overall, and I don't see how this replacement is beneficial.

ACKs for top commit:
  hebasto:
    re-ACK b8fe333.

Tree-SHA512: d84091eb594af695805784e87f49d38ac08a7cf42978c844383882196389a0d83fc5f72e6394887b9961cc3f96538962f4ee86ef3eeca2c96509c64428046c70
  • Loading branch information
real-or-random committed Jun 29, 2024
2 parents 4af241b + b8fe333 commit a526937
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ else()
string(REGEX REPLACE "-DNDEBUG[ \t\r\n]*" "" CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE}")
string(REGEX REPLACE "-DNDEBUG[ \t\r\n]*" "" CMAKE_C_FLAGS_MINSIZEREL "${CMAKE_C_FLAGS_MINSIZEREL}")
# Prefer -O2 optimization level. (-O3 is CMake's default for Release for many compilers.)
string(REGEX REPLACE "-O3[ \t\r\n]*" "-O2" CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE}")
string(REGEX REPLACE "-O3( |$)" "-O2\\1" CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE}")
endif()

# Define custom "Coverage" build type.
Expand Down

0 comments on commit a526937

Please sign in to comment.