Warnings from included headers files leak into build

[jferch]

Is your enhancement proposal related to a problem? Please describe. I'm compiling my project with a rather new version of the arm-none-eabi-c++ compiler and high warning levels enabled. This works fine for internal code but as a result I get a number of warnings/errors from external code such as the NXP SDK. The warnings originate only from header files as far as I can see, an example can be seen below. Ultimately, it would be nice to get rid of those compiler warnings if possible.

.../nxp/drivers/flash/./fsl_ftfx_controller.h:239:5: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
.../nxp/drivers/ftm/./fsl_ftm.h:437:73: warning: unused parameter 'base' [-Wunused-parameter]

Describe the solution you'd like That this project supports CMake is highly appreciated but I haven't found a solution that doesn't need to modify your code, but maybe you have a better solution for this. What seems to be working is declaring the header files as system include directories, therefore changing

target_include_directories(${MCUX_SDK_PROJECT_NAME} PUBLIC
    ${CMAKE_CURRENT_LIST_DIR}/.
)

to

target_include_directories(${MCUX_SDK_PROJECT_NAME} SYSTEM PUBLIC
    ${CMAKE_CURRENT_LIST_DIR}/.
)

Since hard-coding this into each include command is probably not be the desired behaviour for everyone, using a variable might be an option which is by default behaving like previously. The command could for example look like this:

target_include_directories(${MCUX_SDK_PROJECT_NAME} ${MCUX_SDK_PROJECT_SYSTEM_INCLUDE} PUBLIC
    ${CMAKE_CURRENT_LIST_DIR}/.
)

On the consumer side, the client can decide if they want to use a system include by setting the variable appropriately. Otherwise nothing additional needs to be done and the 2nd line can be omitted. The warnings/error disappear within my build after this.

A CMakeLists.txt could look like this:

set(MCUX_SDK_PROJECT_NAME MK64F12)
set(MCUX_SDK_PROJECT_SYSTEM_INCLUDE SYSTEM)

include(${SDK_DEVICE_DIR}/all_lib_device_MK64F12.cmake)  
include(driver_flash)
include(driver_ftm)  
...

Curious what your thoughts are on this, thanks in advance!

Cheers, Johannes

[mcuxsusan]

Hi @jferch, thanks for sharing the issue and proposal. I would appreciate if you could help share your arm-none-eabi-c++ compiler version and compile options for my side to reproduce the issue, thus we are on same page to investigate the resolution.

[jferch]

Hi @mcuxsusan, thanks for your reply! Sure no problem - I'm using arm-none-eabi-gcc v11.3.0 with the following compile options. The disabled errors are basically needed to get a successful build.

-std=c11 
-std=c++20
-Wall
-Wstrict-aliasing
-Wextra
-Werror
-Wpedantic
-Wshadow
-Wdouble-promotion
-Wnon-virtual-dtor
-Woverloaded-virtual
-Wmisleading-indentation
-Wnull-dereference
-Wduplicated-cond
-Wduplicated-branches
-Wlogical-op
-Wformat=2
-Wno-error=unused-parameter
-Wno-error=unused-variable
-Wno-error=unused-function
-Wno-error=conversion
-Wno-error=volatile
-Wno-error=pedantic
-Wno-volatile
-ffreestanding
-fno-builtin
-fno-common
-fdata-sections
-ffunction-sections
-fstack-usage
-finline-small-functions
-findirect-inlining
-fverbose-asm
-fno-use-cxa-atexit
-fno-rtti
-fno-exceptions
-fno-non-call-exceptions
-Wundef
-mthumb
-MD

[mcuxsusan]

Hi @jferch, thanks for the information, I am able to reproduce the warnings with above shared flags, but the build always failure because of below information:

cc1plus.exe: error: command-line option '-std=c11' is valid for C/ObjC but not for C++

Maybe because I am using gcc 10.2.1, after remove the -std=c11 flag, I could see the warnings.

Because the proposed change is to update all module cmake file to add SYSTEM attribute to the target_include_directories, which seems to be an architectural change to us, we will need more consideration for that, so currently we are not tend to apply the change. I investigated other possible resolution to suppress the warnings and I found below approach seems to be also OK, could you please try and check your side?

1. Build NXP SDK to a library, install the target and export the target, for example mcux-sdk-lib.
2. Application use find_package or directly include mcux-sdk-lib-config.cmake to import the information of the library, then link the library to the application.

I found in this way, without change to NXP SDK cmake components, the build warnings from NXP SDK headers could also be suppressed. I referenced the answer https://stackoverflow.com/questions/66321559/cmake-linked-static-librarys-header-files-not-found for library creation and application linking, provide below CMakeLists.txt file for your reference. lib_CMakeLists.txt app_CMakeLists.txt

[jferch]

Hello @mcuxsusan, thanks for your response. Sorry, yes you're right - the invalid flag was an error on my part and only appropriate when compiling actual .c files.

I appreciate your effort in finding an alternative approach which is more suitable for you as a maintainer, I'm aware and understand that the initial solution would have resulted in changes on your side. I've managed to incorporate your approach into our build system and can confirm that the warnings and errors have mostly disappeared (with the exception of the volatile and c++20 shown below). Although I would have preferred a solution which invokes cmake only once for simplicity's sake, this issue can therefore be resolved. The remaining error I'll probably suppress with '-Wno-volatile'.

.../lib/nxp_kl17/nxp/fsl_clock.h:427:37: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
  427 |     (*(volatile uint32_t *)regAddr) |= (1UL << CLK_GATE_ABSTRACT_BITS_SHIFT((uint32_t)name));

Thanks for your help!

[stefanct]

Why are all the header files declared PUBLIC in the first place? At least in the current implementation, where applications include the sdk components directly (i.e., without making it a library first) , this doesn't really make sense AFAICT. In its current form it also breaks creating/exporting customized libraries (where the add_library() target instead of the add_executable() target is including the sdk) because all the SDK's cmake files use CMAKE_CURRENT_LIST_DIR as a base which is not exportable. The correct way to keep them public would be something like this I think:

PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}>
       $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}...>

That way libraries could be exported/installed without making the header private I think. However, this also implies that all headers are to be provided to all library users although they don't necessarily need it. I found it easier to change the SDK to make everything private and only make selected headers/interface public to my library's users.