Skip to content

Commit

Permalink
cmake: enable -Wshadow partially for in-tree code
Browse files Browse the repository at this point in the history
This enables -Wshadow to warn about shadow variables on
in tree code under arch/, boards/, drivers/, kernel/,
lib/, soc/, and subsys/.

Note that this does not enable it globally because
out-of-tree modules will probably take some time to fix
(or not at all depending on the project), and it would be
great to avoid introduction of any new shadow variables
in the meantime.

Also note that this tries to be done in a minimally
invasive way so it is easy to revert when we enable
-Wshadow globally. Source files under modules/, samples/
and tests/ are currently excluded because there does not
seem to be a trivial way to add -Wshadow there without
going through all CMakeLists.txt to add the option
(as there are 1000+ files to change).

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
  • Loading branch information
dcpleung authored and carlescufi committed Aug 22, 2023
1 parent 1f7e0de commit e38fc6d
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 7 deletions.
7 changes: 1 addition & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -603,12 +603,7 @@ add_subdirectory(lib)
# property which is set implicitly for custom command outputs
include(misc/generated/CMakeLists.txt)

if(EXISTS ${SOC_DIR}/${ARCH}/CMakeLists.txt)
add_subdirectory(${SOC_DIR}/${ARCH} soc/${ARCH})
else()
add_subdirectory(${SOC_DIR}/${ARCH}/${SOC_PATH} soc/${ARCH}/${SOC_PATH})
endif()

add_subdirectory(soc)
add_subdirectory(boards)
add_subdirectory(subsys)
add_subdirectory(drivers)
Expand Down
3 changes: 3 additions & 0 deletions arch/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: Apache-2.0

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)

add_definitions(-D__ZEPHYR_SUPERVISOR__)

include_directories(
Expand Down
5 changes: 5 additions & 0 deletions boards/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ if(EXISTS ${BOARD_DIR}/CMakeLists.txt)
set(build_dir boards/${ARCH}/${BOARD})
else()
unset(build_dir)

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
#
# For now, only enable warning for shadow variables for in-tree boards.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)
endif()

add_subdirectory(${BOARD_DIR} ${build_dir})
Expand Down
3 changes: 3 additions & 0 deletions cmake/compiler/arcmwdt/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,6 @@ if(CONFIG_ARCMWDT_LIBC)
# to ASM builds (which may use 'stdbool.h').
set_property(TARGET asm APPEND PROPERTY required "-I${NOSTDINC}")
endif()

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)
3 changes: 3 additions & 0 deletions cmake/compiler/armclang/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ set_property(TARGET asm APPEND PROPERTY required "--target=${triple}")

# Only the ARM Compiler C library is currently supported.
set_compiler_property(PROPERTY nostdinc)

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)
3 changes: 3 additions & 0 deletions cmake/compiler/compiler_flags_template.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,6 @@ set_compiler_property(PROPERTY no_position_independent)
# gen_kobject_list.py is does not understand it and end up identifying objects as if
# they had the same address.
set_compiler_property(PROPERTY no_global_merge)

# Compiler flag for warning about shadow variables
set_compiler_property(PROPERTY warning_shadow_variables)
2 changes: 2 additions & 0 deletions cmake/compiler/gcc/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,5 @@ set_compiler_property(PROPERTY no_position_independent
)

set_compiler_property(PROPERTY no_global_merge "")

set_compiler_property(PROPERTY warning_shadow_variables -Wshadow)
3 changes: 3 additions & 0 deletions cmake/compiler/icx/compiler_flags.cmake
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@

include(${ZEPHYR_BASE}/cmake/compiler/clang/compiler_flags.cmake)

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)
3 changes: 3 additions & 0 deletions cmake/compiler/xcc/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ set_compiler_property(PROPERTY warning_error_misra_sane)

# XCC does not support -fno-pic and -fno-pie
set_compiler_property(PROPERTY no_position_independent "")

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)
3 changes: 3 additions & 0 deletions cmake/compiler/xt-clang/compiler_flags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ endif()

# Clang version used by Xtensa does not support -fno-pic and -fno-pie
set_compiler_property(PROPERTY no_position_independent "")

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)
3 changes: 3 additions & 0 deletions drivers/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: Apache-2.0

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)

add_definitions(-D__ZEPHYR_SUPERVISOR__)

add_subdirectory(disk)
Expand Down
3 changes: 3 additions & 0 deletions kernel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ target_link_libraries(kernel INTERFACE ${libkernel})

else()

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)

list(APPEND kernel_files
main_weak.c
banner.c
Expand Down
3 changes: 3 additions & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: Apache-2.0

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)

add_subdirectory(crc)
if(NOT CONFIG_EXTERNAL_LIBC)
add_subdirectory(libc)
Expand Down
16 changes: 16 additions & 0 deletions soc/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# SPDX-License-Identifier: Apache-2.0

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
#
# Limit warning of shadow variables to in-tree SoC files for now.
cmake_path(IS_PREFIX ZEPHYR_BASE "${SOC_DIR}" NORMALIZE _SOC_IS_IN_TREE)
if(_SOC_IS_IN_TREE)
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)
endif()
unset(_SOC_IS_IN_TREE)

if(EXISTS ${SOC_DIR}/${ARCH}/CMakeLists.txt)
add_subdirectory(${SOC_DIR}/${ARCH} soc/${ARCH})
else()
add_subdirectory(${SOC_DIR}/${ARCH}/${SOC_PATH} soc/${ARCH}/${SOC_PATH})
endif()
12 changes: 11 additions & 1 deletion subsys/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# SPDX-License-Identifier: Apache-2.0

# FIXME: SHADOW_VARS: move this before adding shadow variable warning below.
# This is because, in some build configurations, the external lorawan module
# is pulled in as though the source files are in main repo. This results in
# shadow variable warnings being active on these files. Until the module has
# fixed those shadow variables, keep this here before add_compile_options()
# below.
add_subdirectory_ifdef(CONFIG_LORAWAN lorawan)

# FIXME: SHADOW_VARS: Remove this once we have enabled -Wshadow globally.
add_compile_options($<TARGET_PROPERTY:compiler,warning_shadow_variables>)

add_subdirectory(canbus)
add_subdirectory(debug)
add_subdirectory(fb)
Expand Down Expand Up @@ -29,7 +40,6 @@ add_subdirectory_ifdef(CONFIG_EMUL emul)
add_subdirectory_ifdef(CONFIG_IMG_MANAGER dfu)
add_subdirectory_ifdef(CONFIG_INPUT input)
add_subdirectory_ifdef(CONFIG_JWT jwt)
add_subdirectory_ifdef(CONFIG_LORAWAN lorawan)
add_subdirectory_ifdef(CONFIG_NET_BUF net)
add_subdirectory_ifdef(CONFIG_RETENTION retention)
add_subdirectory_ifdef(CONFIG_SENSING sensing)
Expand Down

0 comments on commit e38fc6d

Please sign in to comment.