set_optimization_flags() adds semicolons instead of spaces to CMAKE_CXX_FLAGS

[Morwenn]

Bug category

• bug - compilation error
• bug - compilation warning
• bug - runtime error
• bug - runtime warning
• bug - logic error

Describe the bug
I was trying to integrate Matplot++ with Conan, and I stumbled upon an issue with set_optimization_flags(). The function currently adds optimization flags as follows:

list(APPEND CMAKE_CXX_FLAGS " /O2")

Which gives the following kind of value to CMAKE_CXX_FLAGS:

/DWIN32 /D_WINDOWS /W3 /GR /EHsc; /O2

I am unsure whether the semicolon makes a difference for MSVC, but when building a g++-compatible command line the stray semicolon in the flags causes a hard error. Conan happens to add the architecture flag (e.g. -m64) before those, which is how I found that issue. CMAKE_CXX_FLAGS apparently expects space-separated flags, and not semicolon-separated ones, but LIST(APPEND) uses semicolons to separate list elements.

I guess that the issue wasn't diagnosed previously because LIST(APPEND) works as expected when CMAKE_CXX_FLAGS is empty.

The ideal solution would be to use target_compile_options to add the optimization flags, but if you wish to apply them globally to the project, then changing the line above (and all the equivalent lines in set_optimization_flags()) to the following line should fix the issue:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2")

Steps to Reproduce

It's in the middle of a Conan wrapper, so the steps to reproduce would a bit complicated for the issue.

Output

Along those lines:

[  2%] Building CXX object source_subfolder/source/3rd_party/CMakeFiles/nodesoup.dir/nodesoup/src/fruchterman_reingold.cpp.obj
[  6%] Building CXX object source_subfolder/source/3rd_party/CMakeFiles/nodesoup.dir/nodesoup/src/algebra.cpp.obj
[  6%] Building CXX object source_subfolder/source/3rd_party/CMakeFiles/nodesoup.dir/nodesoup/src/kamada_kawai.cpp.obj
[  9%] Building CXX object source_subfolder/source/3rd_party/CMakeFiles/nodesoup.dir/nodesoup/src/layout.cpp.obj
[ 11%] Building CXX object source_subfolder/source/3rd_party/CMakeFiles/nodesoup.dir/nodesoup/src/nodesoup.cpp.obj
g++.exe: error: unrecognized command-line option '-m64;'; did you mean '-m64'?
g++.exe: error: unrecognized command-line option '-m64;'; did you mean '-m64'?
g++.exe: error: unrecognized command-line option '-m64;'; did you mean '-m64'?

Platform

• cross-platform issue - linux
• cross-platform issue - windows
• cross-platform issue - macos

Environment Details:

• OS: Windows
• OS Version: 10
• Compiler: g++
• Compiler version: (Rev3, Built by MSYS2 project) 10.1.0

[alandefreitas]

@Morwenn thanks for finding that issue. And thanks for the Conan recipe!

This is correct. Even this snippet needs to include the ';' to be able to iterate the existing flags.

matplotplusplus/cmake/functions/project_flags.cmake

Line 56 in 8c5f0c5

string(REPLACE " " ";" CMAKE_CXX_FLAGS_LIST ${CMAKE_CXX_FLAGS})

I imagine fixing this is just a matter of replacing the 6 list commands

matplotplusplus/cmake/functions/project_flags.cmake

Line 68 in 8c5f0c5

list(APPEND CMAKE_CXX_FLAGS " -O0 -g4")

with something like

set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} " -O0 -g4")

Right? Or is there a better command to use here?

I imagine that this has never been an issue because people might be providing no flags at all or already providing a flag.

Setting these flags directly is actually bad practice as these should be set per target, but the truth is even CMake itself does it. XD

https://gitlab.kitware.com/cmake/cmake/-/blob/master/CMakeLists.txt#L835

Would you like to open this PR?

[Morwenn]

Since the corresponding PR has been merge, can I close the issue?

[alandefreitas]

Sure!