Simplify add_compile_options in CMake #11422
Conversation
| endif() | ||
|
|
||
| eth_add_cxx_compiler_flag_if_supported(-Wimplicit-fallthrough) |
There was a problem hiding this comment.
This is actually duplicate even today (added below).
| if(NOT have_stack_protector_strong_support) | ||
| eth_add_cxx_compiler_flag_if_supported(-fstack-protector) | ||
| endif() | ||
| cable_add_compile_options(IF_SUPPORTED -fstack-protector -fstack-protector-strong) |
There was a problem hiding this comment.
Apparently this can be simplified greatly, see wasmx/fizzy#770 (comment) and wasmx/fizzy#770 (comment)
cmake/EthCompilerSettings.cmake
Outdated
| -Wpessimizing-move | ||
| -Wredundant-move | ||
| # Prevent the path of the source directory from ending up in the binary via __FILE__ macros. | ||
| -fmacro-prefix-map=${CMAKE_SOURCE_DIR}=/solidity |
There was a problem hiding this comment.
What happens if CMAKE_SOURCE_DIR contains spaces or a semicolon?
There was a problem hiding this comment.
This might at least need to be quoted.
There was a problem hiding this comment.
Quoted it, but can you try it locally perhaps? The flag does not work on macos/clang.
There was a problem hiding this comment.
Lol, more than one thing broken in that setting apparently:
$ pwd
/home/daniel/weird test; dir/solidity/build
$ cmake ..
CMake Error at CMakeLists.txt:17 (include):
include could not find requested file:
EthToolchains
CMake Error at CMakeLists.txt:20 (include):
include could not find requested file:
EthPolicy
CMake Error at CMakeLists.txt:21 (eth_policy):
Unknown CMake command "eth_policy".
-- Configuring incomplete, errors occurred!
There was a problem hiding this comment.
Without the semicolon it works, though - and I would guess that cmake itself might be incompatible with paths containing a semicolon.
There was a problem hiding this comment.
$ clang -fmacro-prefix-map=""
clang: error: unknown argument: '-fmacro-prefix-map='
clang: error: no input files
$ clang -v
Apple clang version 11.0.0 (clang-1100.0.33.8)
Target: x86_64-apple-darwin19.6.0
Based on https://en.wikipedia.org/wiki/Xcode#Xcode_7.0_-_12.x_(since_Free_On-Device_Development) supposedly I have upstream clang 11.0.0.
There was a problem hiding this comment.
Quoting in CMake is fucked up. I usually just try different things until it works. But in most cases it will automatically quote a variable content.
There was a problem hiding this comment.
Yeah, stuff like that in cmake is indeed quite fucked up... but I haven't found any combination on the user side that would make this work properly - I haven't tried getting it to work by changing cable_add_compile_options myself yet, though.
There was a problem hiding this comment.
CMake even has a workaround helper separate_arguments. It is probably not worth the fight and better to handle -fmacro-prefix-map manually.
|
|
||
| foreach(flag ${ARG_IF_SUPPORTED}) | ||
| message("Checking ${flag}") | ||
| string(MAKE_C_IDENTIFIER ${flag} flag_id) |
There was a problem hiding this comment.
I think this here fixes the main issues we had earlier. Still we have to double-check the behaviour for the fmacro-prefix-map case and weird paths - but maybe it just works or at least quoting in the list (see https://github.com/ethereum/solidity/pull/11422/files#r642952157) will be enough.
|
Closing due to inactivity. |
This PR goes back to #10359 (comment).
The problem was described by @ekpyron at ethereum/cable#15, and an improved version/solution was added here wasmx/fizzy#770.
It seems to simplify our CMake configuration quite a bit, but it won't really make any difference, so it is not all that important to merge. The motivation was to resolve that old conversation in one way or another.