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

Simplify add_compile_options in CMake #11422

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions cmake/CableCompileOptions.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Cable: CMake Bootstrap Library <https://github.com/ethereum/cable>
# Copyright 2021 Pawel Bylica.
# Licensed under the Apache License, Version 2.0.

# Cable Compile Options, version 1.0.0
#
# This CMake module provides utilities to conditionally add compile options
# depending on language and compiler support.
#
# CHANGELOG
#
# 1.0.0 - 2021-03-24

include_guard(GLOBAL)

include(CheckCXXCompilerFlag)
include(CheckCCompilerFlag)

function(cable_add_compile_options)
cmake_parse_arguments(ARG "" "" "IF_SUPPORTED" ${ARGN})

# Init options list with all arguments before IF_SUPPORTED keyword.
set(options ${ARG_UNPARSED_ARGUMENTS})

# Get list of languages to check.
# Currently only C and CXX is supported here, but with check_compiler_flag()
# from CMake 3.19 all languages can be checked.
get_property(languages GLOBAL PROPERTY ENABLED_LANGUAGES)
list(FILTER languages INCLUDE REGEX "C|CXX")

# Some flags causes compiler warning instead of error. Still such flags must
# be considered unsupported. To make check_X_compiler_flag() fail before
# CMake 3.19 additional -Werror flag must be added.
list(GET languages 0 example_lang)
set(compiler ${CMAKE_${example_lang}_COMPILER_ID})
if(compiler MATCHES GNU OR compiler MATCHES Clang)
set(CMAKE_REQUIRED_FLAGS -Werror)
endif()

foreach(flag ${ARG_IF_SUPPORTED})
string(MAKE_C_IDENTIFIER ${flag} flag_id)
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 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.

set(supported_in_all_languages TRUE)

foreach(lang ${languages})
set(result_var "${lang}${flag_id}")

# Check if the flag works in the lang's compiler.
# In CMake 3.18+ cmake_language(CALL ...) can be used.
if(lang STREQUAL CXX)
check_cxx_compiler_flag(${flag} ${result_var})
else()
check_c_compiler_flag(${flag} ${result_var})
endif()

if(NOT "${${result_var}}")
set(supported_in_all_languages FALSE)
endif()
endforeach()

if(supported_in_all_languages)
list(APPEND options ${flag})
else()
foreach(lang ${languages})
set(result_var "${lang}${flag_id}")
if(${${result_var}})
list(APPEND options "$<$<COMPILE_LANGUAGE:${lang}>:${flag}>")
endif()
endforeach()
endif()
endforeach()

add_compile_options(${options})
endfunction()
25 changes: 0 additions & 25 deletions cmake/EthCheckCXXCompilerFlag.cmake

This file was deleted.

65 changes: 25 additions & 40 deletions cmake/EthCompilerSettings.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,56 +14,41 @@
#
# These settings then end up spanning all POSIX platforms (Linux, OS X, BSD, etc)

include(EthCheckCXXCompilerFlag)
include(CableCompileOptions)
axic marked this conversation as resolved.
Show resolved Hide resolved

if(NOT EMSCRIPTEN)
eth_add_cxx_compiler_flag_if_supported(-fstack-protector-strong have_stack_protector_strong_support)
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)
Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently this can be simplified greatly, see wasmx/fizzy#770 (comment) and wasmx/fizzy#770 (comment)

endif()

eth_add_cxx_compiler_flag_if_supported(-Wimplicit-fallthrough)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually duplicate even today (added below).


# Prevent the path of the source directory from ending up in the binary via __FILE__ macros.
eth_add_cxx_compiler_flag_if_supported("-fmacro-prefix-map=${CMAKE_SOURCE_DIR}=/solidity")

# -Wpessimizing-move warns when a call to std::move would prevent copy elision
# if the argument was not wrapped in a call. This happens when moving a local
# variable in a return statement when the variable is the same type as the
# return type or using a move to create a new object from a temporary object.
eth_add_cxx_compiler_flag_if_supported(-Wpessimizing-move)

# -Wredundant-move warns when an implicit move would already be made, so the
# std::move call is not needed, such as when moving a local variable in a return
# that is different from the return type.
eth_add_cxx_compiler_flag_if_supported(-Wredundant-move)

if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang"))
# Enables all the warnings about constructions that some users consider questionable,
# and that are easy to avoid. Also enable some extra warning flags that are not
# enabled by -Wall. Finally, treat at warnings-as-errors, which forces developers
# to fix warnings as they arise, so they don't accumulate "to be fixed later".
add_compile_options(-Wall)
add_compile_options(-Wextra)
add_compile_options(-Werror)
add_compile_options(-pedantic)
add_compile_options(-Wmissing-declarations)
add_compile_options(-Wno-unknown-pragmas)
add_compile_options(-Wimplicit-fallthrough)
add_compile_options(-Wsign-conversion)
add_compile_options(-Wconversion)

eth_add_cxx_compiler_flag_if_supported(
$<$<COMPILE_LANGUAGE:CXX>:-Wextra-semi>
cable_add_compile_options(
-Wall
-Wextra
-Werror
-pedantic
-Wmissing-declarations
-Wno-unknown-pragmas
-Wimplicit-fallthrough
-Wsign-conversion
-Wconversion
IF_SUPPORTED
-Wextra-semi
-Wfinal-dtor-non-final-class
-Wnewline-eof
-Wsuggest-destructor-override
-Wduplicated-cond
-Wduplicate-enum
-Wlogical-op
-Wno-unknown-attributes
-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"
)
eth_add_cxx_compiler_flag_if_supported(-Wfinal-dtor-non-final-class)
eth_add_cxx_compiler_flag_if_supported(-Wnewline-eof)
eth_add_cxx_compiler_flag_if_supported(-Wsuggest-destructor-override)
eth_add_cxx_compiler_flag_if_supported(-Wduplicated-cond)
eth_add_cxx_compiler_flag_if_supported(-Wduplicate-enum)
eth_add_cxx_compiler_flag_if_supported(-Wlogical-op)
eth_add_cxx_compiler_flag_if_supported(-Wno-unknown-attributes)

# Configuration-specific compiler settings.
set(CMAKE_CXX_FLAGS_DEBUG "-O0 -g3 -DETH_DEBUG")
Expand Down
1 change: 1 addition & 0 deletions cmake/EthOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ macro(print_config NAME)
message("-- CMAKE_BUILD_TYPE Build type ${CMAKE_BUILD_TYPE}")
message("-- TARGET_PLATFORM Target platform ${CMAKE_SYSTEM_NAME}")
message("--------------------------------------------------------------- features")
message("-- SANITIZE Code sanitizer ${SANITIZE}")
message("-- COVERAGE Coverage support ${COVERAGE}")
message("------------------------------------------------------------- components")
if (SUPPORT_TESTS)
Expand Down