Details
Diff Detail
- Repository
- rT test-suite
Event Timeline
External/SPEC/CFP2017rate/507.cactuBSSN_r/CMakeLists.txt | ||
---|---|---|
14–49 | I would like to suggest a different approach: add_definitions($<$<COMPILE_LANGUAGE:Fortran>:FCODE>) add_definitions($<$<COMPILE_LANGUAGE:C>:CCODE>) add_definitions($<$<COMPILE_LANGUAGE:CXX>:CCODE CCTK_DISABLE_RESTRICT=1>) set_property(TARGET ${PROG} PROPERTY C_STANDARD 11) or set(CMAKE_C_STANDARD 11) (the last one sets the default for all upcoming targets; however, it is also a global variable but should only be changed within this add_subdirectory scope -- I think). At least the -std=c99 flag is something the cmake should determine. |
External/SPEC/CFP2017rate/507.cactuBSSN_r/CMakeLists.txt | ||
---|---|---|
14–49 | add_compile_definitions should be preferred over add_definitions |
LGTM
External/SPEC/CFP2017rate/507.cactuBSSN_r/CMakeLists.txt | ||
---|---|---|
51–53 | test-suite style usually defines compilation/linking options before adding the target (e.g. by setting LDFALGS, CFLAGS, CXXCLAGS), add_compile_definitions would be more consistent than target_compile_definitions. But I also don't think its important. |
External/SPEC/CFP2017rate/507.cactuBSSN_r/CMakeLists.txt | ||
---|---|---|
51–53 | At first I tried using add_compile_definitions, but the the generator expression was not being expanded correctly. It was being partially merged with the absolute path to the object file. I spent several hours trying to debug this but had no success: I then found this recommendation: and went with target_compile_definitions instead. |
I would like to suggest a different approach:
or
(the last one sets the default for all upcoming targets; however, it is also a global variable but should only be changed within this add_subdirectory scope -- I think). At least the -std=c99 flag is something the cmake should determine.