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

Component with Wpedantic flag causing warning on generated project_elf_src_esp32.c file (IDFGH-7292) #8881

Closed
Barabas5532 opened this issue May 2, 2022 · 4 comments
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@Barabas5532
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When there is a component in the project that adds Wpedantic to its public or interface compile options, then a warning appears on the generated project_elf_src_esp32.c file.

A project might have a component like this to provide compiler flags to be used by other component to enable compiler warnings, etc. An example is available in the JUCE project:

add_library(juce_recommended_warning_flags INTERFACE)
add_library(juce::juce_recommended_warning_flags ALIAS juce_recommended_warning_flags)

target_compile_options(juce_recommended_warning_flags INTERFACE
    -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wuninitialized
    -Wunused-parameter -Wsign-compare -Wsign-conversion -Wunreachable-code
    -Wcast-align -Wno-implicit-fallthrough -Wno-maybe-uninitialized
    -Wno-missing-field-initializers -Wno-ignored-qualifiers -Wswitch-enum
    -Wredundant-decls -Wno-strict-overflow -Wshadow
    $<$<COMPILE_LANGUAGE:CXX>:
        -Woverloaded-virtual -Wreorder -Wzero-as-null-pointer-constant>)

The warning generated is:

build/project_elf_src_esp32.c:1: warning: ISO C forbids an empty translation unit [-Wpedantic]

Describe the solution you'd like

I've found that adding target_link_libraries(${project_elf} "-Wno-pedantic") around here:

target_link_libraries(${project_elf} "-Wl,--cref" "-Wl,--Map=\"${mapfile}\"")
seems to do the trick. This does not feel like a real solution, other project might specify different flags that break in a different way, and need a different fix.

The warning appeared because the project_elf target links to all the components defined in the project, including the component defining the extra warning flags:

target_link_libraries(${project_elf} ${build_components})

Why does project_elf link to all other components? Could it just link to the main component and then all other components are pulled in transitively, avoiding linking these unintended flags?

Please correct me if I'm wrong, but I don't think linking the project_elf target to any INTERFACE library does anything useful? Linking only STATIC libraries might be another fix.

Additional context

I'm trying to implement the practices from this article into my project: https://www.foonathan.net/2018/10/cmake-warnings/

@Barabas5532 Barabas5532 added the Type: Feature Request Feature request for IDF label May 2, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label May 2, 2022
@github-actions github-actions bot changed the title Component with Wpedantic flag causing warning on generated project_elf_src_esp32.c file Component with Wpedantic flag causing warning on generated project_elf_src_esp32.c file (IDFGH-7292) May 2, 2022
@dobairoland
Copy link
Collaborator

Hi @Barabas5532. Please also consider reading https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html. We will consider your suggestions but I cannot promise it will be very soon.

The documentation (https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/build-system.html#controlling-component-compilation) recommends to set PRIVATE flags for components.

@DNedic
Copy link
Collaborator

DNedic commented May 2, 2022

@dobairoland I don't think you can set PRIVATE for interface libraries.
@Barabas5532 It is expected that linking publicly to an interface target with certain compile options will propagate those flags, infact it's a method that's often used to set sets of compile options and features for libraries. Take a look at: https://jeremimucha.com/2021/02/cmake-fundamentals-part5/

I will take a look at your suggestion, maybe filtering libraries that are being linked into the final app executable is an option. As Roland said, don't expect this to be very soon.

@igrr
Copy link
Member

igrr commented May 2, 2022

Another note, I think among various possible compiler flags, there are many such flags which, when added to global CFLAGS, will result in errors or warnings from ESP-IDF components.

For any such flag, IDF can make a choice:

  • either to "protect" its default components from such flag (e.g. add -Wno-pedantic as private flags where necessary)
  • or to be compatible with such flag being added.

The first approach is sometimes difficult as the ordering of private and public compiler flags doesn't always work the way one would expect.

The 2nd approach may in some cases require an impractical number of changes in ESP-IDF (as is the case for -Wpedantic, -Wundef).

Why does project_elf link to all other components? Could it just link to the main component and then all other components are pulled in transitively, avoiding linking these unintended flags?

I think you are right. However an application might not have a main component, or another application component (not main) may be the "root" of the dependency tree. So in general the safest option here is to link all the components.

@Barabas5532
Copy link
Contributor Author

It is expected that linking publicly to an interface target with certain compile options will propagate those flags, infact it's a method that's often used to set sets of compile options and features for libraries. Take a look at: https://jeremimucha.com/2021/02/cmake-fundamentals-part5/

Yes, that is what makes this compiler flags library useful. Other libraries in the project link to it using PRIVATE, so that the compiler flags are added to the build for the source files in that library. The flags can be changed for the whole project all at once in the compiler flags library.

Usually we can control which library links which other libraries. This is not the case with the project_elf library, which links to all other libraries, causing this issue.

I've made a slight mistake on the original post, adding target_link_libraries(${project_elf} "-Wno-pedantic") does not actually fix this. I've changed that to target_compile_options, and still the Wno-pedantic has no effect, since it appears before Wpedantic.

Changing this line:

COMMAND ${CMAKE_COMMAND} -E touch ${project_elf_src}

to

        COMMAND ${CMAKE_COMMAND} -E echo "typedef int make_iso_compilers_happy;" > ${project_elf_src}

does work.

https://stackoverflow.com/questions/26541150/warning-iso-c-forbids-an-empty-translation-unit

@espressif-bot espressif-bot added Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

5 participants