raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.74k stars 929 forks source link

Include directories should be added as system include directories #924

Closed ooxi closed 3 months ago

ooxi commented 2 years ago

Currently all include directories are added as none system include directories. This causes warnings and errors when compiling my own source code

In file included from /pico-sdk/src/common/pico_stdlib/include/pico/stdlib.h:13,
                 from /blink.c++:2:
/pico-sdk/src/rp2_common/hardware_gpio/include/hardware/gpio.h:882:33: error: ISO C++ does not permit named variadic macros [-Werror=variadic-macros]
  882 | #define CU_REGISTER_DEBUG_PINS(p...) \
      |                                 ^~~
kilograham commented 2 years ago

This error seems unrelated to whether things are system includes or not. What compiler version are you using?

ooxi commented 2 years ago

@kilograham I'm using Fedora 36, this is the compiler identification according to CMake

-- The C compiler identification is GNU 11.1.0
-- The CXX compiler identification is GNU 11.1.0

I have created a minimal example on GitHub where you can see the full compiler output https://github.com/ooxi/pico-sdk-pedantic/runs/7435146674?check_suite_focus=true

These are the compile settings for my source file which includes pico/stdlib.h

    -std=c++20

    -Wall
    -Wextra
    -Werror
    -Wpedantic

    -Wnon-virtual-dtor
    -Wcast-align
    -Wunused
    -Woverloaded-virtual
    -Wdouble-promotion
    -Wformat=2
kilograham commented 2 years ago

The SDK will not compile with both -Wpedantic and -Werror ; the variadic macro thing is something that could be fixed easily enough, but it is hardly a warning of a possible bug

ooxi commented 2 years ago

@kilograham first of all, thank you very much for your fast reaction! Very appreciated :-)

The SDK will not compile with both -Wpedantic and -Werror

I do not want to compile the SDK with -Wpedantic and -Werror, just my own sources. Therefore I used set_source_files_properties to set those compile options only on my own sources, not all sources of the library.

Using target_include_directories with SYSTEM INTERFACE instead of INTERFACE will let the compiler know to not apply those settings on the include.

I will come back with a proof of concept :-)

ooxi commented 2 years ago

@kilograham I have created a proof of concept which is sufficient to compile the example with -Wpedantic and -Werror

I would be happy to clean up the patch and prepare a PR if you consider accepting it.

kilograham commented 2 years ago

Ah, ok i see the point... warnings are suppressed in system headers.

ooxi commented 2 years ago

@kilograham feel free to assign the issue to me, I will prepare a PR against develop

kilograham commented 1 year ago

duplicate of #82

kilograham commented 1 year ago

oh this has a PR ;-)